Kestrel HTTP2 locking questions

[ladeak]

Why does CheckStreamWindow

aspnetcore/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs

Line 160 in 94e0528

internal long CheckStreamWindow(long bytes)

and CheckConnectionWindow

aspnetcore/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs

Line 1024 in 94e0528

private long CheckConnectionWindow(long bytes)

require locking? (Instead of a Volatile.Read())

Is there a reason for _streamWindow not using Interlocked instead of the regular lock statement?

_streamWindow can be updated by the Http2OutputWriter consuming it, increased by the WINDOW_UPDATE frames or by the connection window update from a SETTINGS frames (I think this can decrease the value).

The value is read unprotected in SchedulingState property (hence going back to the original question why it needs the lock)?

aspnetcore/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs

Line 92 in 94e0528

internal (int, long, State, State, long) SchedulingState => (Stream.StreamId, _unconsumedBytes, _unobservedState, _currentState, _streamWindow);

[ladeak]

I wonder if @Tratcher has some insights on this.

[Tratcher]

Interlocked.Read would make sense for CheckStreamWindow, but doesn't matter much since CheckStreamWindow has to lock to update it anyways. Consistency is useful.

SchedulingState is just for debugging.

Do you see these showing up as lock contention in a profiler?

[ladeak]

I have not ran into contention. I was around this file, and the question occurred to me (how come nobody ran into contention).
Thank you for the explanation.

[davidfowl]

I have not ran into contention

What does that mean. Did you send lots of concurrent request on the same stream and looked at the lock contention performance events?

[ladeak]

Really not. I am learning how HTTP2 works and how it is handled in Kestrel, so when I explored the codebase, this question occurred to me. There are lots of locking when stream or connection properties are updated. I simply wondered if this ever caused an issue, or what were the design decisions driving this implementation.

EDIT: I was considering other approaches, mainly because I am curious if one could achieve some gains using Interlocked / Interlock anything pattern, but it soon becomes quite complex.