Configure HTTP/2 receive window size

[CarnaViire]

Background and Motivation

Current HTTP/2 receive window size is 64 Kb, which is too small for many scenarios. It leads to poor throughput, see e.g. #43086

Just increasing the default window size may result in unwanted memory consumption in some cases. It is proposed to enable users to configure flow control properties that so they can reach a compromise between memory consumption and throughput.

Proposed API

public sealed partial class SocketsHttpHandler
{
    ...
    public bool EnableMultipleHttp2Connections { get; set; }
+   public int InitialHttp2ConnectionWindowSize { get; set; }
+   public int InitialHttp2StreamWindowSize { get; set; }
+   public int Http2StreamWindowUpdateRatio { get; set; }
}

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and Motivation

Current HTTP/2 receive window size is 64 Kb, which is too small for many scenarios. It leads to poor throughput, see e.g. #43086

Just increasing the default window size may result in unwanted memory consumption in some cases. It is proposed to enable users to configure that so they can reach a compromise between memory consumption and throughput.

Proposed API

public sealed partial class SocketsHttpHandler
{
    ...
    public bool EnableMultipleHttp2Connections { get; set; }
+   public int InitialHttp2StreamWindowSize { get; set; }
}

Author:	CarnaViire
Assignees:	-
Labels:	api-suggestion, area-System.Net.Http
Milestone:	6.0.0

[halter73]

How is the initial connection window size currently determined? Should that also be configurable?

[CarnaViire]

Connection's window size is 64 Mb. I guess configuring it makes sense, in case you would want a really big stream window, so connection window won't become a bottleneck... I'll add that

[ManickaP]

Should we also consider the size of the increments?
https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs#L91
We're now using 1/8 of the window size, whereas Kestrel is using 1/2 (if I understand it right):
https://source.dot.net/#Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http2/Http2Stream.cs,62

So to achieve similar window update sizes (32KB) one would need to raise this value 4x to 256KB...

[JamesNK]

Are these configuration settings designed to allow good performance by default, or are they for advanced users to tune performance?

I believe the HTTP/2 functionality in Grpc.Core (which uses a C++ HTTP/2 stack) has good performance without having to tune this configuration. Ideally SocketsHttpHandler would be the same.

If we make devs change these settings, most aren't going to know to and will continue to have HTTP/2 perf issues with large requests.

[CarnaViire]

@JamesNK we intend to update the defaults so it will allow good performance in most of the cases without the need to change anything. The main reason for these settings is to be sure that we will not break/block anyone with our suddenly increased defaults and they could set it to something smaller. And as a second reason we'll have some advanced tuning for those who are interested.

[antonfirsov]

[API] The proposal started with 1 property, now we have 3 :) How would the introduction of the dynamic window algorithm impact this API? Will there be a new bool property to turn it on/off? Are the properties InitialHttp2***WindowSize and Http2StreamWindowUpdateRatio meaningful in that case? Will there be any further properties specific to the dynamic window algorithm?

[CarnaViire]

@antonfirsov all of the properties will make sense even with an introduction of the dynamic algorithm. InitialHttp2***WindowSize will indicate a start from which the window might grow bigger depending on RTT information. Http2StreamWindowUpdateRatio will bear the same meaning it has now, indicating the ratio of the WINDOW_UPDATE size to the full window size, regardless of whether the window size is changing or not.

New properties for turning dynamic algorithm on/off or other configs might appear, however, it's better to discuss them separately when we decide to implement dynamic algorithm.

[antonfirsov]

New properties for turning dynamic algorithm on/off or other configs might appear

In this case, wouldn't it make sense to extract the properties to a separate Http2WindowOptions class now? Alternatively, we can prefix the properties, just like we do with KeepAlivePingDelay + KeepAlivePingPolicy + KeepAlivePingTimeout, but I don't see an good candidate for a prefix here.

Personally, I dislike how SocketsHttpHandler became a long flat list of seemingly unrelated options, it hurts discoverability. @scalablecory do you know why does the design keep evolving this way?

[CarnaViire]

Separate Http2FlowControlOptions class seems ok to me.

public sealed partial class SocketsHttpHandler
{
    ...
    public bool EnableMultipleHttp2Connections { get; set; }
+   public Http2FlowControlOptions? Http2FlowControlOptions { get; set; } = null;
}

+public class Http2FlowControlOptions
+{
+   public int InitialConnectionWindowSize { get; set; }
+   public int InitialStreamWindowSize { get; set; } 
+   public int StreamWindowUpdateRatio { get; set; }
+}

I personally don't see a better place to put these options. SocketsHttpHandler seems like a lowest level thing available to configure when using HttpClient. It made sense to me to put new configs there as it would made sense for me to search for all the configs there if I was writing the code... I'm interested in your opinion though, what other place for configuration do you have in mind?

[antonfirsov]

I didn't mean to have them in another place :) I just don't understand why do we prefer the current flat structure instead of defining separate option classes for different configuration concerns. I think if a group contains 3+ related properties, it makes sense to extract it to a separate class, but not sure if others share my opinion.

[ManickaP]

Personally, I think we should stick with just a single property, InitialStreamWindowSize.
For the connection, we already try not to enforce limits there:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs

Lines 85 to 88 in f690d59

	// We don't really care about limiting control flow at the connection level.
	// We limit it per stream, and the user controls how many streams are created.
	// So set the connection window size to a large value.
	private const int ConnectionWindowSize = 64 * 1024 * 1024;

So just making it higher than stream should suffice.
And for the ratio, we could follow Kestrel's lead and just change the 1/8 to 1/2.

[CarnaViire]

If we allow to change only InitialStreamWindowSize, then connection window might become a bottleneck if anyone would wish to set stream window size to e.g. 64 Mb or bigger for some reason. Kestrel allows configuring both of these properties https://github.com/dotnet/aspnetcore/blob/701b294342ff2381bc9e291d2ec828191c28fd34/src/Servers/Kestrel/Core/src/Http2Limits.cs#L105-L147

I believe it is not enough to just have connection's window bigger that a stream one, as there could be many streams in one connection.

As for the ratio, as it does not affect memory consumption, only performance - I'm ok with not exposing it for now.

[geoffkizer]

At a minimum we need InitialStreamWindowSize or something like that. I think we should avoid the term "Initial" here as it doesn't really describe how it works in practice. I'd call it Http2StreamWindowSize or something like that.

We need to define the new default value for this property as part of this proposal.

I don't have a strong opinion re ConnectionWindowSize. I'm not sure when a user would set this in general. I think we want it to not be a bottleneck in general, which is why it's as large as it is today, and if we think that's still too small (given the new default window size) then we could increase it even further. I suppose the reason a user would want to set this would be to set an overall memory usage limit, in cases where they have lots of streams and may need a large window per stream, but still want to ensure that memory usage doesn't grow really large overall. Thoughts?

That said, since we are exposing the stream window size, it's reasonable to expose the connection window size too, for completeness if nothing else.

[geoffkizer]

Regarding update ratio:

I don't think we should expose this. I think we should be able to just do the right thing here without user input. And I'm not sure how the user would reason about how to set this.

Consider also there are possible optimizations we could make here in the future, like proactively sending window update frames with other frames we are sending anyway; if we did this, it's not clear what the user "update ratio" setting means in this model.

As to what we should set this to: We chose 1/8 in order to try to ensure some reasonable amortization of the cost of sending a window update, while minimizing impact on the effective window size. This still seems like a reasonable value to me, though if we have perf data otherwise we could tune it.

In particular, I am not sure why ASP.NET chose 1/2; this seems pretty large to me. @JamesNK @halter73 thoughts here?

[CarnaViire]

I've added "Initial" so it would still make sense in case dynamic algorithm will be added. But it is also called "Initial" in SETTINGS frame (SETTINGS_INITIAL_WINDOW_SIZE)

[geoffkizer]

Yeah, but in the SETTINGS frame it specifically is the initial window size, and the ongoing window size is controlled by sending window updates.

[karelz]

Triage: Based on discussion, we should wait with this until we have the dynamic window update, then decide if parts/all is needed at all in .NET 6.

[antonfirsov]

Closing this in favor of #53372