NIOHTTP2: fix outbound stream starvation

[nishaddeokar]

Problem

nextStreamToSend() returned self.flushableStreams.first, and since Set.first always yields the same stable element, under load the buffer kept handing the whole connection window to one stream while the rest starved to near-zero.

Fix

Rotate through the flushable streams instead of always picking the same one. flushableStreams stays the source of truth for membership, and alongside it we keep a queue of those streams (flushableQueue) in the order to serve them. We pop the front to pick the next stream, and append it to the back once it's been served, so every flushable stream gets a turn per sweep and none can be starved. Both the pop and the append are O(1). Removing a stream from the middle of the queue would be O(n), so we avoid this. A stream that stops being flushable is left as a stale entry and simply skipped when it reaches the front (lazy deletion), with a queuedStreams set to stop us ever enqueuing a duplicate.

Results

iperf3 -P 32 through an HTTP/2 CONNECT proxy (32 streams on one connection), on Linux, per-stream receiver throughput:

Stream	.first	.randomElement()	Round-robin (fix)
[1]	0 bit/s	152.4 Mbit/s	154.7 Mbit/s
[2]	0 bit/s	152.4 Mbit/s	154.8 Mbit/s
[3]	0 bit/s	152.5 Mbit/s	154.8 Mbit/s
[4]	0 bit/s	153.1 Mbit/s	154.8 Mbit/s
[5]	0 bit/s	153.3 Mbit/s	154.8 Mbit/s
[6]	0 bit/s	153.3 Mbit/s	154.9 Mbit/s
[7]	0 bit/s	153.5 Mbit/s	154.9 Mbit/s
[8]	0 bit/s	153.5 Mbit/s	154.9 Mbit/s
[9]	0 bit/s	153.5 Mbit/s	154.9 Mbit/s
[10]	0 bit/s	153.5 Mbit/s	154.9 Mbit/s
[11]	0 bit/s	153.7 Mbit/s	154.9 Mbit/s
[12]	237.7 Mbit/s	153.7 Mbit/s	155.0 Mbit/s
[13]	243.0 Mbit/s	153.9 Mbit/s	155.0 Mbit/s
[14]	334.9 Mbit/s	153.9 Mbit/s	155.0 Mbit/s
[15]	335.3 Mbit/s	153.9 Mbit/s	155.0 Mbit/s
[16]	335.7 Mbit/s	153.9 Mbit/s	155.0 Mbit/s
[17]	335.9 Mbit/s	154.0 Mbit/s	155.0 Mbit/s
[18]	336.0 Mbit/s	154.0 Mbit/s	155.0 Mbit/s
[19]	336.0 Mbit/s	154.0 Mbit/s	155.0 Mbit/s
[20]	336.0 Mbit/s	154.1 Mbit/s	155.0 Mbit/s
[21]	336.0 Mbit/s	154.1 Mbit/s	155.0 Mbit/s
[22]	336.0 Mbit/s	154.1 Mbit/s	155.0 Mbit/s
[23]	336.0 Mbit/s	154.5 Mbit/s	155.2 Mbit/s
[24]	336.0 Mbit/s	154.6 Mbit/s	155.2 Mbit/s
[25]	336.0 Mbit/s	154.9 Mbit/s	155.2 Mbit/s
[26]	336.0 Mbit/s	154.9 Mbit/s	155.2 Mbit/s
[27]	336.0 Mbit/s	155.0 Mbit/s	155.2 Mbit/s
[28]	336.0 Mbit/s	155.2 Mbit/s	155.2 Mbit/s
[29]	336.0 Mbit/s	155.2 Mbit/s	155.2 Mbit/s
[30]	336.0 Mbit/s	155.2 Mbit/s	155.2 Mbit/s
[31]	336.0 Mbit/s	155.2 Mbit/s	155.2 Mbit/s
[32]	336.0 Mbit/s	155.2 Mbit/s	155.2 Mbit/s
SUM	6.86 Gbit/s	4.93 Gbit/s	4.96 Gbit/s

Using Jain's fairness index to quantify fairness:

$$J = \frac{\left(\sum x_i\right)^2}{n \cdot \sum x_i^2}$$

.first: 6862.5² / (32 × 2,259,097.7) = 0.6514
.randomElement(): 4928.2² / (32 × 758,994.4) = 0.9999
Round-robin: 4960.3² / (32 × 768,893.7) = 1.0000

Why round-robin over .randomElement()

randomElement() also fixes the fairness problem, but round-robin gives a deterministic guarantee that every flushable stream is served once per sweep rather than being fair only on average. Round-robin also avoids the system call caused by randomElement() on Linux, leading to better throughput.

[glbrntt]

Can you add a unit test (or tests) for this? Given the approach is now round-robin you should be able to write deterministic tests for this.

[glbrntt]

Can we be more concrete here?

I expect served is just [1, 3, 5, 1, 3, 5, 1, 3, 5] which makes the round-robin nature of this explicit.

[glbrntt]

Thanks, this looks good now but it has regressed allocations. That's expected (Set + CircularBuffer) and acceptable because it's at the connection level, you will however need to update the allocation limits. swift-nio has some scripts to do this but they changed a little while ago so you'll need to figure out the right invocation.

[glbrntt]

great, thanks @nishaddeokar!