Skip to content

Commit bfc2e12

Browse files
author
ladeak
committed
Review comments
1 parent 8878dad commit bfc2e12

File tree

4 files changed

+74
-91
lines changed

4 files changed

+74
-91
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http2/FlowControl/InputFlowControl.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl;
1919
/// <seealso href="https://datatracker.ietf.org/doc/html/rfc9113#name-flow-control"/>
2020
internal sealed class InputFlowControl
2121
{
22-
private record struct FlowControlState
22+
private struct FlowControlState
2323
{
2424
private const long AbortedBitMask = 1L << 32; // uint MaxValue + 1
2525
internal long _state;
@@ -35,7 +35,7 @@ public FlowControlState(uint initialWindowSize, bool isAborted)
3535

3636
public uint Available => (uint)_state;
3737

38-
public bool IsAborted => (_state & AbortedBitMask) > 0;
38+
public bool IsAborted => _state > uint.MaxValue;
3939
}
4040

4141
private readonly uint _initialWindowSize;
@@ -84,7 +84,7 @@ public bool TryAdvance(int bytes)
8484
return false;
8585
}
8686

87-
computedFlow = new FlowControlState(currentFlow.Available - (uint)bytes, currentFlow.IsAborted);
87+
computedFlow = new FlowControlState(currentFlow.Available - (uint)bytes, isAborted: false);
8888
} while (currentFlow._state != Interlocked.CompareExchange(ref _flow._state, computedFlow._state, currentFlow._state));
8989

9090
return true;
@@ -110,7 +110,7 @@ public bool TryUpdateWindow(int bytes, out int updateSize)
110110
// It shouldn't be possible for the window size to ever exceed Http2PeerSettings.MaxWindowSize.
111111
Debug.Assert(false, $"{nameof(TryUpdateWindow)} attempted to grow window past max size.");
112112
}
113-
computedFlow = new FlowControlState(currentFlow.Available + (uint)bytes, currentFlow.IsAborted);
113+
computedFlow = new FlowControlState(currentFlow.Available + (uint)bytes, isAborted: false);
114114
} while (currentFlow._state != Interlocked.CompareExchange(ref _flow._state, computedFlow._state, currentFlow._state));
115115

116116
if (_windowUpdatesDisabled)
@@ -156,7 +156,7 @@ public int Abort()
156156
return 0;
157157
}
158158

159-
computedFlow = new FlowControlState(currentFlow.Available, true);
159+
computedFlow = new FlowControlState(currentFlow.Available, isAborted: true);
160160
} while (currentFlow._state != Interlocked.CompareExchange(ref _flow._state, computedFlow._state, currentFlow._state));
161161

162162
// Tell caller to return connection window space consumed by this stream. Even if window updates have

‎src/Servers/Kestrel/Core/test/Http2/FlowControl/InputFlowControlTests.cs‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests.Http2.FlowControl;
88

99
public class InputFlowControlTests
1010
{
11+
private const int IterationCount = 1;
12+
1113
[Fact]
1214
public async Task Threads1_Advance()
1315
{
14-
for (var i = 0; i < 1000; i++)
16+
for (var i = 0; i < IterationCount; i++)
1517
{
1618
var sut = new InputFlowControl(5000, 1);
1719
await Task.Run(() => Advance(sut)); ;
@@ -30,7 +32,7 @@ static void Advance(InputFlowControl sut)
3032
[Fact]
3133
public async Task Threads2_Advance()
3234
{
33-
for (var i = 0; i < 1000; i++)
35+
for (var i = 0; i < IterationCount; i++)
3436
{
3537
var sut = new InputFlowControl(10000, 1);
3638
var t1 = Task.Run(() => Advance(sut));
@@ -51,7 +53,7 @@ static void Advance(InputFlowControl sut)
5153
[Fact]
5254
public async Task Threads3_Advance()
5355
{
54-
for (var i = 0; i < 1000; i++)
56+
for (var i = 0; i < IterationCount; i++)
5557
{
5658
var sut = new InputFlowControl(15000, 1);
5759
var t1 = Task.Run(() => Advance(sut));
@@ -73,7 +75,7 @@ static void Advance(InputFlowControl sut)
7375
[Fact]
7476
public async Task Threads3_WindowUpdates()
7577
{
76-
for (var i = 0; i < 1000; i++)
78+
for (var i = 0; i < IterationCount; i++)
7779
{
7880
var sut = new InputFlowControl(15000, 0);
7981
var t1 = Task.Run(() => WindowUpdate(sut));
@@ -96,7 +98,7 @@ static void WindowUpdate(InputFlowControl sut)
9698
[Fact]
9799
public async Task Threads4_Advance()
98100
{
99-
for (var i = 0; i < 1000; i++)
101+
for (var i = 0; i < IterationCount; i++)
100102
{
101103
var sut = new InputFlowControl(20000, 1);
102104
var t1 = Task.Run(() => Advance(sut));
@@ -118,7 +120,7 @@ static void Advance(InputFlowControl sut)
118120
[Fact]
119121
public async Task Threads3_Abort()
120122
{
121-
for (var i = 0; i < 1000; i++)
123+
for (var i = 0; i < IterationCount; i++)
122124
{
123125
var sut = new InputFlowControl(20000, 1);
124126
var t1 = Task.Run(() => Advance(sut));
@@ -165,7 +167,7 @@ public void Abort_Abort()
165167
[Fact]
166168
public async Task Threads3Abort_AssertAfter()
167169
{
168-
for (var i = 0; i < 1000; i++)
170+
for (var i = 0; i < IterationCount; i++)
169171
{
170172
var isAborted = false;
171173
var sut = new InputFlowControl(90000, 1);
@@ -204,7 +206,7 @@ static void Advance(ref bool isAborted, InputFlowControl sut)
204206
[Fact]
205207
public async Task Threads3Abort_AssertBefore()
206208
{
207-
for (var i = 0; i < 3000; i++)
209+
for (var i = 0; i < IterationCount; i++)
208210
{
209211
var isAborting = false;
210212
var sut = new InputFlowControl(30000, 1);
@@ -243,7 +245,7 @@ static void Advance(ref bool isAborting, InputFlowControl sut)
243245
[Fact]
244246
public async Task Threads3WindowUpdates_AssertAfter()
245247
{
246-
for (var i = 0; i < 1000; i++)
248+
for (var i = 0; i < IterationCount; i++)
247249
{
248250
var isAborted = false;
249251
var sut = new InputFlowControl(90000, 1);
@@ -282,7 +284,7 @@ static void WindowUpdate(ref bool isAborted, InputFlowControl sut)
282284
[Fact]
283285
public async Task Threads3WindowUpdates_AssertBefore()
284286
{
285-
for (var i = 0; i < 3000; i++)
287+
for (var i = 0; i < IterationCount; i++)
286288
{
287289
var isAborting = false;
288290
var sut = new InputFlowControl(30000, 1);
@@ -321,7 +323,7 @@ static void WindowUpdate(ref bool isAborting, InputFlowControl sut)
321323
[Fact]
322324
public async Task Threads3Advance_WindowUpdates()
323325
{
324-
for (var i = 0; i < 1000; i++)
326+
for (var i = 0; i < IterationCount; i++)
325327
{
326328
var sut = new InputFlowControl(30000, 0);
327329
var t0 = Task.Run(() => WindowUpdate(sut));
@@ -354,7 +356,7 @@ static void Advance(InputFlowControl sut)
354356
[Fact]
355357
public async Task Threads3WindowUpdates_AssertSize()
356358
{
357-
for (var i = 0; i < 1000; i++)
359+
for (var i = 0; i < IterationCount; i++)
358360
{
359361
var sizeSum = 0;
360362
var sut = new InputFlowControl(15000, 10);

‎src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/FlowControlBenchmark.cs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using BenchmarkDotNet.Attributes;
5+
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl;
6+
7+
namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks.Http2.FlowControl;
8+
9+
public class FlowControlBenchmark
10+
{
11+
private readonly InputFlowControl _flowControl = new(1000, 10);
12+
private const int N = 100000;
13+
private const int Spin = 50;
14+
private const int ThreadsCount = 3;
15+
16+
[IterationSetup]
17+
public void IterationSetup()
18+
{
19+
_flowControl.Reset();
20+
}
21+
22+
[Benchmark]
23+
public async Task ThreadsAdvanceWithWindowUpdates()
24+
{
25+
var parallelThreads = new Task[ThreadsCount + 1];
26+
_flowControl.Reset();
27+
for (var i = 0; i < ThreadsCount; i++)
28+
{
29+
var advanceTask = Task.Factory.StartNew(() =>
30+
{
31+
for (int i = 0; i < N; i++)
32+
{
33+
if (_flowControl.TryUpdateWindow(16, out _))
34+
{
35+
for (int j = 0; j < Spin; j++)
36+
{
37+
}
38+
}
39+
}
40+
}, TaskCreationOptions.LongRunning);
41+
parallelThreads[i] = advanceTask;
42+
var tAdvance = Task.Factory.StartNew(() =>
43+
{
44+
for (var i = 0; i < N; i++)
45+
{
46+
_flowControl.TryAdvance(1);
47+
}
48+
_flowControl.Abort();
49+
}, TaskCreationOptions.LongRunning);
50+
parallelThreads[ThreadsCount] = tAdvance;
51+
}
52+
53+
await Task.WhenAll(parallelThreads);
54+
}
55+
}

‎src/Servers/Kestrel/perf/Microbenchmarks/Http2/FlowControl/InputFlowControl.cs‎

Lines changed: 0 additions & 74 deletions
This file was deleted.

0 commit comments

Comments
 (0)