From bfc2e12aa7500c4f00e4bee7d8d9e9b9721f0edc Mon Sep 17 00:00:00 2001 From: ladeak Date: Wed, 11 Sep 2024 20:25:21 +0200 Subject: [PATCH] Review comments --- .../Http2/FlowControl/InputFlowControl.cs | 10 +-- .../FlowControl/InputFlowControlTests.cs | 26 ++++--- .../Http2/FlowControl/FlowControlBenchmark.cs | 55 ++++++++++++++ .../Http2/FlowControl/InputFlowControl.cs | 74 ------------------- 4 files changed, 74 insertions(+), 91 deletions(-) create mode 100644 src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/FlowControlBenchmark.cs delete mode 100644 src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/InputFlowControl.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/FlowControl/InputFlowControl.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/FlowControl/InputFlowControl.cs index 6c5a1060736af..640af27e65c9d 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/FlowControl/InputFlowControl.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/FlowControl/InputFlowControl.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; /// internal sealed class InputFlowControl { - private record struct FlowControlState + private struct FlowControlState { private const long AbortedBitMask = 1L << 32; // uint MaxValue + 1 internal long _state; @@ -35,7 +35,7 @@ public FlowControlState(uint initialWindowSize, bool isAborted) public uint Available => (uint)_state; - public bool IsAborted => (_state & AbortedBitMask) > 0; + public bool IsAborted => _state > uint.MaxValue; } private readonly uint _initialWindowSize; @@ -84,7 +84,7 @@ public bool TryAdvance(int bytes) return false; } - computedFlow = new FlowControlState(currentFlow.Available - (uint)bytes, currentFlow.IsAborted); + computedFlow = new FlowControlState(currentFlow.Available - (uint)bytes, isAborted: false); } while (currentFlow._state != Interlocked.CompareExchange(ref _flow._state, computedFlow._state, currentFlow._state)); return true; @@ -110,7 +110,7 @@ public bool TryUpdateWindow(int bytes, out int updateSize) // It shouldn't be possible for the window size to ever exceed Http2PeerSettings.MaxWindowSize. Debug.Assert(false, $"{nameof(TryUpdateWindow)} attempted to grow window past max size."); } - computedFlow = new FlowControlState(currentFlow.Available + (uint)bytes, currentFlow.IsAborted); + computedFlow = new FlowControlState(currentFlow.Available + (uint)bytes, isAborted: false); } while (currentFlow._state != Interlocked.CompareExchange(ref _flow._state, computedFlow._state, currentFlow._state)); if (_windowUpdatesDisabled) @@ -156,7 +156,7 @@ public int Abort() return 0; } - computedFlow = new FlowControlState(currentFlow.Available, true); + computedFlow = new FlowControlState(currentFlow.Available, isAborted: true); } while (currentFlow._state != Interlocked.CompareExchange(ref _flow._state, computedFlow._state, currentFlow._state)); // Tell caller to return connection window space consumed by this stream. Even if window updates have diff --git a/src/Servers/Kestrel/Core/test/Http2/FlowControl/InputFlowControlTests.cs b/src/Servers/Kestrel/Core/test/Http2/FlowControl/InputFlowControlTests.cs index 52dba4b4e74ca..4c31ab6f0db39 100644 --- a/src/Servers/Kestrel/Core/test/Http2/FlowControl/InputFlowControlTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/FlowControl/InputFlowControlTests.cs @@ -8,10 +8,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests.Http2.FlowControl; public class InputFlowControlTests { + private const int IterationCount = 1; + [Fact] public async Task Threads1_Advance() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var sut = new InputFlowControl(5000, 1); await Task.Run(() => Advance(sut)); ; @@ -30,7 +32,7 @@ static void Advance(InputFlowControl sut) [Fact] public async Task Threads2_Advance() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var sut = new InputFlowControl(10000, 1); var t1 = Task.Run(() => Advance(sut)); @@ -51,7 +53,7 @@ static void Advance(InputFlowControl sut) [Fact] public async Task Threads3_Advance() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var sut = new InputFlowControl(15000, 1); var t1 = Task.Run(() => Advance(sut)); @@ -73,7 +75,7 @@ static void Advance(InputFlowControl sut) [Fact] public async Task Threads3_WindowUpdates() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var sut = new InputFlowControl(15000, 0); var t1 = Task.Run(() => WindowUpdate(sut)); @@ -96,7 +98,7 @@ static void WindowUpdate(InputFlowControl sut) [Fact] public async Task Threads4_Advance() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var sut = new InputFlowControl(20000, 1); var t1 = Task.Run(() => Advance(sut)); @@ -118,7 +120,7 @@ static void Advance(InputFlowControl sut) [Fact] public async Task Threads3_Abort() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var sut = new InputFlowControl(20000, 1); var t1 = Task.Run(() => Advance(sut)); @@ -165,7 +167,7 @@ public void Abort_Abort() [Fact] public async Task Threads3Abort_AssertAfter() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var isAborted = false; var sut = new InputFlowControl(90000, 1); @@ -204,7 +206,7 @@ static void Advance(ref bool isAborted, InputFlowControl sut) [Fact] public async Task Threads3Abort_AssertBefore() { - for (var i = 0; i < 3000; i++) + for (var i = 0; i < IterationCount; i++) { var isAborting = false; var sut = new InputFlowControl(30000, 1); @@ -243,7 +245,7 @@ static void Advance(ref bool isAborting, InputFlowControl sut) [Fact] public async Task Threads3WindowUpdates_AssertAfter() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var isAborted = false; var sut = new InputFlowControl(90000, 1); @@ -282,7 +284,7 @@ static void WindowUpdate(ref bool isAborted, InputFlowControl sut) [Fact] public async Task Threads3WindowUpdates_AssertBefore() { - for (var i = 0; i < 3000; i++) + for (var i = 0; i < IterationCount; i++) { var isAborting = false; var sut = new InputFlowControl(30000, 1); @@ -321,7 +323,7 @@ static void WindowUpdate(ref bool isAborting, InputFlowControl sut) [Fact] public async Task Threads3Advance_WindowUpdates() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var sut = new InputFlowControl(30000, 0); var t0 = Task.Run(() => WindowUpdate(sut)); @@ -354,7 +356,7 @@ static void Advance(InputFlowControl sut) [Fact] public async Task Threads3WindowUpdates_AssertSize() { - for (var i = 0; i < 1000; i++) + for (var i = 0; i < IterationCount; i++) { var sizeSum = 0; var sut = new InputFlowControl(15000, 10); diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/FlowControlBenchmark.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/FlowControlBenchmark.cs new file mode 100644 index 0000000000000..73d963d5f49ee --- /dev/null +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/FlowControlBenchmark.cs @@ -0,0 +1,55 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; + +namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks.Http2.FlowControl; + +public class FlowControlBenchmark +{ + private readonly InputFlowControl _flowControl = new(1000, 10); + private const int N = 100000; + private const int Spin = 50; + private const int ThreadsCount = 3; + + [IterationSetup] + public void IterationSetup() + { + _flowControl.Reset(); + } + + [Benchmark] + public async Task ThreadsAdvanceWithWindowUpdates() + { + var parallelThreads = new Task[ThreadsCount + 1]; + _flowControl.Reset(); + for (var i = 0; i < ThreadsCount; i++) + { + var advanceTask = Task.Factory.StartNew(() => + { + for (int i = 0; i < N; i++) + { + if (_flowControl.TryUpdateWindow(16, out _)) + { + for (int j = 0; j < Spin; j++) + { + } + } + } + }, TaskCreationOptions.LongRunning); + parallelThreads[i] = advanceTask; + var tAdvance = Task.Factory.StartNew(() => + { + for (var i = 0; i < N; i++) + { + _flowControl.TryAdvance(1); + } + _flowControl.Abort(); + }, TaskCreationOptions.LongRunning); + parallelThreads[ThreadsCount] = tAdvance; + } + + await Task.WhenAll(parallelThreads); + } +} diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/InputFlowControl.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/InputFlowControl.cs deleted file mode 100644 index d9f68592f6d9f..0000000000000 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/InputFlowControl.cs +++ /dev/null @@ -1,74 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using BenchmarkDotNet.Attributes; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; - -namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks.Http2.FlowControl; - -public class FlowControlBenchmark -{ - private readonly InputFlowControl _flowControl = new(1000, 10); - private const int N = 100000; - private const int Spin = 50; - - [IterationSetup] - public void IterationSetup() - { - _flowControl.Reset(); - } - - [Benchmark] - public async Task ThreadsAdvanceWithWindowUpdates() - { - _flowControl.Reset(); - var t1 = Task.Factory.StartNew(() => - { - for (int i = 0; i < N; i++) - { - _flowControl.TryUpdateWindow(16, out _); - } - _flowControl.Abort(); - }, TaskCreationOptions.LongRunning); - - var t2 = Task.Factory.StartNew(() => - { - for (int i = 0; i < N; i++) - { - if (_flowControl.TryAdvance(1)) - { - for (int j = 0; j < Spin; j++) - { - } - } - } - }, TaskCreationOptions.LongRunning); - - var t3 = Task.Factory.StartNew(() => - { - for (int i = 0; i < N; i++) - { - if (_flowControl.TryAdvance(1)) - { - for (int j = 0; j < Spin; j++) - { - } - } - } - }, TaskCreationOptions.LongRunning); - - var t4 = Task.Factory.StartNew(() => - { - for (int i = 0; i < N; i++) - { - if (_flowControl.TryAdvance(1)) - { - for (int j = 0; j < Spin; j++) - { - } - } - } - }); - await Task.WhenAll(t1, t2, t3, t4); - } -}