Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to enable multuple H/3 connections #1998

Merged
merged 5 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions build/httpclient-scenarios.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ parameters:
type: string
default: '--client.framework net9.0 --server.framework net8.0' # net8.0 for server is a temporary workaround

- name: getScenarios
- name: getScenarios
type: object
default:
- displayName: "GET"
arguments: --scenario httpclient-kestrel-get --variable useHttpMessageInvoker=true --property server=kestrel --property client=dotnetinvoker --property method=get --client.options.collectCounters true

- name: postScenarios
- name: postScenarios
type: object
default:
- displayName: "POST"
Expand All @@ -41,7 +41,9 @@ parameters:
- displayName: "HTTP/2.0 (mult conn)"
arguments: --variable httpVersion=2.0 --variable useHttps=true --variable http20EnableMultipleConnections=true --property httpversion=h2multconn
- displayName: "HTTP/3.0"
arguments: --variable httpVersion=3.0 --variable useHttps=true --property httpversion=h3
arguments: --variable httpVersion=3.0 --variable useHttps=true --variable http30EnableMultipleConnections=false --property httpversion=h3
- displayName: "HTTP/3.0 (mult conn)"
arguments: --variable httpVersion=3.0 --variable useHttps=true --variable http30EnableMultipleConnections=true --property httpversion=h3multconn

- name: clientsxThreads
type: object
Expand Down
33 changes: 18 additions & 15 deletions scenarios/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Continuous benchmarking results are available on [this PowerBI dashboard](https:

## Requirements

These jobs can be executed using the .NET Crank global tool.
These jobs can be executed using the .NET Crank global tool.
[.NET Core](<http://dot.net>) is required to install the global tool.

Install `crank` with the following command:
Expand Down Expand Up @@ -62,7 +62,7 @@ crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenario

### Available scenarios

- `plaintext`: Middleware implementation
- `plaintext`: Middleware implementation
- `https`: Middleware implementation, using HTTPS
- `endpoint`: Middleware implementation with Endpoint routing
- `mvc`: Controller implementation
Expand All @@ -85,7 +85,7 @@ crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenario

### Available scenarios

- `json`: Middleware implementation
- `json`: Middleware implementation
- `https`: Middleware implementation, using HTTPS
- `mvc`: Controller implementation
- `mapaction`: Endpoint routing implementation
Expand Down Expand Up @@ -125,8 +125,8 @@ The following scenarios are using ASP.NET CORE MVC
- `fortunes_ef_mvc_https`

The suffixes represent different database access strategies:
- No suffix: Raw ADO.NET

- No suffix: Raw ADO.NET
- "ef" suffix: Entity Framework Core
- "dapper" suffix: Dapper

Expand Down Expand Up @@ -159,7 +159,7 @@ crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenario

These scenarios are running several web proxies, including [YARP](https://github.com/microsoft/reverse-proxy).

The downstream service returns a variable size content. By default the result is 10 bytes.
The downstream service returns a variable size content. By default the result is 10 bytes.

### Sample

Expand Down Expand Up @@ -261,7 +261,7 @@ crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenario
These scenarios measure the performance of different Grpc server and clients implementations.

- Go
- Native (C)
- Native (C)
- ASP.NET

### Sample
Expand All @@ -287,13 +287,13 @@ crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenario

#### Arguments

- Number of streams:
- Number of streams:
- `--variable streams=1`
- `--variable streams=70`
- Number of connections:
- Number of connections:
- `--variable connections=1`
- `--variable connections=28`
- Protocol:
- Protocol:
- `--variable protocol=h2`
- `--variable protocol=h3`
- `--variable protocol=h2c`
Expand Down Expand Up @@ -345,15 +345,15 @@ crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenario

#### Arguments

- Scenario:
- Scenario:
- `--variable scenario=broadcast`
- `--variable scenario=echo`
- `--variable scenario=echoAll`
- Transport:
- Transport:
- `--variable transport=websockets`
- `--variable transport=serversentevents`
- `--variable transport=longpolling`
- Protocol:
- Protocol:
- `--variable protocol=json`
- `--variable protocol=messagepack`

Expand Down Expand Up @@ -397,7 +397,7 @@ crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenario

These scenarios are running various distributed cache benchmarks.

For all the scenarios, the store is initialized with `CacheCount` cache entries. Each request will issue a read or a write based on the `WriteRatio`
For all the scenarios, the store is initialized with `CacheCount` cache entries. Each request will issue a read or a write based on the `WriteRatio`
argument choosing a key randomly. The HTTP response won't contain the cache entry data so that it doesn't impact the raw store perf measurement.

### Sample
Expand Down Expand Up @@ -512,6 +512,9 @@ crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenario
- Enable multiple HTTP/2.0 connections:
- `--variable http20EnableMultipleConnections=true` (default)
- `--variable http20EnableMultipleConnections=false`
- Enable multiple HTTP/3.0 connections (from .NET 9.0):
- `--variable http30EnableMultipleConnections=true` (default)
- `--variable http30EnableMultipleConnections=false`
- Whether to use WinHttpHandler instead of SocketsHttpHandler:
- `--variable useWinHttpHandler=false` (default)
- `--variable useWinHttpHandler=true` -- *requires Windows*
Expand All @@ -538,7 +541,7 @@ These scenarios provide benchmarks to help improve the performance of the .NET G
### Sample

```
crank --config https://raw.githubusercontent.com/dotnet/performance/main/src/benchmarks/gc/scenarios/CrankConfiguration.yaml --scenario 2gb-pinning --profile aspnet-citrine-win --application.framework net9.0
crank --config https://raw.githubusercontent.com/dotnet/performance/main/src/benchmarks/gc/scenarios/CrankConfiguration.yaml --scenario 2gb-pinning --profile aspnet-citrine-win --application.framework net9.0
```
### Available scenarios

Expand Down
4 changes: 3 additions & 1 deletion scenarios/httpclient.benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ variables:
# HttpClient-specific parameters:
http11MaxConnectionsPerServer: 0 #unlimited
http20EnableMultipleConnections: true
http30EnableMultipleConnections: true
useWinHttpHandler: false
useHttpMessageInvoker: false
useDefaultRequestHeaders: false
Expand Down Expand Up @@ -102,6 +103,7 @@ scenarios:
numberOfHttpClients: 1
concurrencyPerHttpClient: '{% if cores > 0 %}{{cores}}{% else %}1{% endif %}'
http20EnableMultipleConnections: false
http30EnableMultipleConnections: false
useHttpMessageInvoker: true
host: '{{secondaryAddress}}'

Expand Down Expand Up @@ -168,7 +170,7 @@ jobs:
isConsoleApp: true
waitForExit: true
timeout: 1200 #seconds
arguments: '--address {{host}} --port {{serverPort}} --useHttps {{useHttps}} --path /{{scenario}} --scenario {{scenario}} --httpVersion {{httpVersion}} --numberOfHttpClients {{numberOfHttpClients}} --concurrencyPerHttpClient {{concurrencyPerHttpClient}} --http11MaxConnectionsPerServer {{http11MaxConnectionsPerServer}} --http20EnableMultipleConnections {{http20EnableMultipleConnections}} --useWinHttpHandler {{useWinHttpHandler}} --useHttpMessageInvoker {{useHttpMessageInvoker}} --collectRequestTimings {{collectRequestTimings}} --contentSize {{requestContentSize}} --contentWriteSize {{requestContentWriteSize}} --contentFlushAfterWrite {{requestContentFlushAfterWrite}} --contentUnknownLength {{requestContentUnknownLength}} {{headersDictionary[requestHeaders]}} --generatedStaticHeadersCount {{generatedStaticRequestHeadersCount}} --generatedDynamicHeadersCount {{generatedDynamicRequestHeadersCount}} --useDefaultRequestHeaders {{useDefaultRequestHeaders}} --warmup {{warmup}} --duration {{duration}}'
arguments: '--address {{host}} --port {{serverPort}} --useHttps {{useHttps}} --path /{{scenario}} --scenario {{scenario}} --httpVersion {{httpVersion}} --numberOfHttpClients {{numberOfHttpClients}} --concurrencyPerHttpClient {{concurrencyPerHttpClient}} --http11MaxConnectionsPerServer {{http11MaxConnectionsPerServer}} --http20EnableMultipleConnections {{http20EnableMultipleConnections}} --http30EnableMultipleConnections {{http30EnableMultipleConnections}} --useWinHttpHandler {{useWinHttpHandler}} --useHttpMessageInvoker {{useHttpMessageInvoker}} --collectRequestTimings {{collectRequestTimings}} --contentSize {{requestContentSize}} --contentWriteSize {{requestContentWriteSize}} --contentFlushAfterWrite {{requestContentFlushAfterWrite}} --contentUnknownLength {{requestContentUnknownLength}} {{headersDictionary[requestHeaders]}} --generatedStaticHeadersCount {{generatedStaticRequestHeadersCount}} --generatedDynamicHeadersCount {{generatedDynamicRequestHeadersCount}} --useDefaultRequestHeaders {{useDefaultRequestHeaders}} --warmup {{warmup}} --duration {{duration}}'

wrk:
source:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public class ClientOptions
public int ConcurrencyPerHttpClient { get; set; }
public int Http11MaxConnectionsPerServer { get; set; }
public bool Http20EnableMultipleConnections { get; set; }
public bool Http30EnableMultipleConnections { get; set; }
public bool UseWinHttpHandler { get; set; }
public bool UseHttpMessageInvoker { get; set; }
public bool CollectRequestTimings { get; set; }
Expand All @@ -37,9 +38,9 @@ public override string ToString()
{
return $"Address={Address}; Port={Port}; UseHttps={UseHttps}; Path={Path}; HttpVersion={HttpVersion}; NumberOfHttpClients={NumberOfHttpClients}; " +
$"ConcurrencyPerHttpClient={ConcurrencyPerHttpClient}; Http11MaxConnectionsPerServer={Http11MaxConnectionsPerServer}; " +
$"Http20EnableMultipleConnections={Http20EnableMultipleConnections}; UseWinHttpHandler={UseWinHttpHandler}; " +
$"UseHttpMessageInvoker={UseHttpMessageInvoker}; CollectRequestTimings={CollectRequestTimings}; Scenario={Scenario}; " +
$"ContentSize={ContentSize}; ContentWriteSize={ContentWriteSize}; ContentFlushAfterWrite={ContentFlushAfterWrite}; " +
$"Http20EnableMultipleConnections={Http20EnableMultipleConnections}; Http30EnableMultipleConnections={Http30EnableMultipleConnections}; " +
$"UseWinHttpHandler={UseWinHttpHandler}; UseHttpMessageInvoker={UseHttpMessageInvoker}; CollectRequestTimings={CollectRequestTimings}; " +
$"Scenario={Scenario}; ContentSize={ContentSize}; ContentWriteSize={ContentWriteSize}; ContentFlushAfterWrite={ContentFlushAfterWrite}; " +
$"ContentUnknownLength={ContentUnknownLength}; Headers=[{string.Join(", ", Headers.Select(h => $"\"{h.Name}: {h.Value}\""))}]; " +
$"GeneratedStaticHeadersCount={GeneratedStaticHeadersCount}; GeneratedDynamicHeadersCount={GeneratedDynamicHeadersCount}; " +
$"UseDefaultRequestHeaders={UseDefaultRequestHeaders}; Warmup={Warmup}; Duration={Duration}";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class ClientOptionsBinder : BinderBase<ClientOptions>
public static Option<int> ConcurrencyPerHttpClientOption { get; } = new Option<int>("--concurrencyPerHttpClient", () => 1, "Number of concurrect requests per one HttpClient");
public static Option<int> Http11MaxConnectionsPerServerOption { get; } = new Option<int>("--http11MaxConnectionsPerServer", () => 0, "Max number of HTTP/1.1 connections per server, 0 for unlimited");
public static Option<bool> Http20EnableMultipleConnectionsOption { get; } = new Option<bool>("--http20EnableMultipleConnections", () => true, "Enable multiple HTTP/2.0 connections");
public static Option<bool> Http30EnableMultipleConnectionsOption { get; } = new Option<bool>("--http30EnableMultipleConnections", () => true, "Enable multiple HTTP/3.0 connections");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we maybe should keep the default as false for the time being.
Otherwise if we merge it like that, all the CI tests will start using that by default, but I think it should be a clear parameter as with H/2 (h2 vs h2multiconn)
It's not a blocker, just a way not to mix up measurements.

Or we can change both crank config yml and ci test config yml right away to include the parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???
Do you want me to change something here or not?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1)
"Keep the default as false" -- Minimal change

- ... new Option<bool>(..., () => true, ...);
+ ... new Option<bool>(..., () => false, ...);

-OR-

(2)
"Change crank config yml" -- Modify scenarios/httpclient.benchmarks.yml to include new parameter as a variable (with default value = false), e.g.:

http20EnableMultipleConnections: true

and pass it to client, e.g.: --http20EnableMultipleConnections {{http20EnableMultipleConnections}}
arguments: '--address {{host}} --port {{serverPort}} --useHttps {{useHttps}} --path /{{scenario}} --scenario {{scenario}} --httpVersion {{httpVersion}} --numberOfHttpClients {{numberOfHttpClients}} --concurrencyPerHttpClient {{concurrencyPerHttpClient}} --http11MaxConnectionsPerServer {{http11MaxConnectionsPerServer}} --http20EnableMultipleConnections {{http20EnableMultipleConnections}} --useWinHttpHandler {{useWinHttpHandler}} --useHttpMessageInvoker {{useHttpMessageInvoker}} --collectRequestTimings {{collectRequestTimings}} --contentSize {{requestContentSize}} --contentWriteSize {{requestContentWriteSize}} --contentFlushAfterWrite {{requestContentFlushAfterWrite}} --contentUnknownLength {{requestContentUnknownLength}} {{headersDictionary[requestHeaders]}} --generatedStaticHeadersCount {{generatedStaticRequestHeadersCount}} --generatedDynamicHeadersCount {{generatedDynamicRequestHeadersCount}} --useDefaultRequestHeaders {{useDefaultRequestHeaders}} --warmup {{warmup}} --duration {{duration}}'

-OR-

(3)
"Change both crank config yml and ci test config yml" -- (2) AND modify build/httpclient-scenarios.yml to include new parameter with both true and false to test matrix, e.g.:

- displayName: "HTTP/2.0"
arguments: --variable httpVersion=2.0 --variable useHttps=true --variable http20EnableMultipleConnections=false --property httpversion=h2
- displayName: "HTTP/2.0 (mult conn)"
arguments: --variable httpVersion=2.0 --variable useHttps=true --variable http20EnableMultipleConnections=true --property httpversion=h2multconn

and as httpVersion is already passed to the scenario (we're not adding a new dimension), it should then appear automatically in "pretty graphs" as soon as the new value will appear in the result table.


Do you want me to change something here or not?

It is hard for me to understand how urgently you'd like to merge the changes (I guess not really, given the PR's in draft), and how much effort are you willing to put into it.

My preferred option is (3) but it can be done as a follow-up, as long as the default is false.

But I am open to the discussion about the default as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is hard for me to understand how urgently you'd like to merge the changes

Not urgent at all, it just wasn't obvious to me from the original comment whether you expect any changes to this PR or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I attempted to do (3). I searched for all http20EnableMultipleConnections occurrences and added h3 equivalents.

But I am open to the discussion about the default as well.

I copy pasted H/2 settings everywhere. If that's not what you wanted, we can chat, but I think it should follow the same approach as H/2.

public static Option<bool> UseWinHttpHandlerOption { get; } = new Option<bool>("--useWinHttpHandler", () => false, "Use WinHttpHandler instead of SocketsHttpHandler");
public static Option<bool> UseHttpMessageInvokerOption { get; } = new Option<bool>("--useHttpMessageInvoker", () => false, "Use HttpMessageInvoker instead of HttpClient");
public static Option<bool> CollectRequestTimingsOption { get; } = new Option<bool>("--collectRequestTimings", () => false, "Collect percentiled metrics of request timings");
Expand All @@ -41,6 +42,7 @@ public static void AddOptionsToCommand(RootCommand command)
command.AddOption(ConcurrencyPerHttpClientOption);
command.AddOption(Http11MaxConnectionsPerServerOption);
command.AddOption(Http20EnableMultipleConnectionsOption);
command.AddOption(Http30EnableMultipleConnectionsOption);
command.AddOption(UseWinHttpHandlerOption);
command.AddOption(UseHttpMessageInvokerOption);
command.AddOption(CollectRequestTimingsOption);
Expand Down Expand Up @@ -72,6 +74,7 @@ protected override ClientOptions GetBoundValue(BindingContext bindingContext)
ConcurrencyPerHttpClient = parsed.GetValueForOption(ConcurrencyPerHttpClientOption),
Http11MaxConnectionsPerServer = parsed.GetValueForOption(Http11MaxConnectionsPerServerOption),
Http20EnableMultipleConnections = parsed.GetValueForOption(Http20EnableMultipleConnectionsOption),
Http30EnableMultipleConnections = parsed.GetValueForOption(Http30EnableMultipleConnectionsOption),
UseWinHttpHandler = parsed.GetValueForOption(UseWinHttpHandlerOption),
UseHttpMessageInvoker = parsed.GetValueForOption(UseHttpMessageInvokerOption),
CollectRequestTimings = parsed.GetValueForOption(CollectRequestTimingsOption),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ private static async Task Setup()
SslOptions = new SslClientAuthenticationOptions { RemoteCertificateValidationCallback = delegate { return true; } },
MaxConnectionsPerServer = max11ConnectionsPerServer,
EnableMultipleHttp2Connections = s_options.Http20EnableMultipleConnections,
#if NET9_0_OR_GREATER
EnableMultipleHttp3Connections = s_options.Http30EnableMultipleConnections,
#endif
ConnectTimeout = Timeout.InfiniteTimeSpan,
};
}
Expand Down Expand Up @@ -396,7 +399,7 @@ private static void CreateRequestContentData()
}

private static HttpRequestMessage CreateRequest(HttpMethod method, Uri uri) =>
new HttpRequestMessage(method, uri) {
new HttpRequestMessage(method, uri) {
Version = s_options.HttpVersion!,
VersionPolicy = HttpVersionPolicy.RequestVersionExact
};
Expand Down