runtime (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation6 Commits10 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Reduces total number and size of allocations on my particular test by ~30%. The test just repeatedly hits the microsoft.com en-us home page:

using System; using System.IO; using System.Net; using System.Net.Http; using System.Threading; using System.Threading.Tasks;

class Program { static async Task Main() { var client = new HttpClient(new SocketsHttpHandler() { UseCookies = false }) { Timeout = Timeout.InfiniteTimeSpan, DefaultRequestVersion = HttpVersion.Version20 }; var uri = new Uri("https://www.microsoft.com/en-us/");

    for (int i = 0; i < 1000; i++)
    {
        using (HttpResponseMessage r = await client.GetAsync(uri, HttpCompletionOption.ResponseHeadersRead))
        using (Stream s = await r.Content.ReadAsStreamAsync())
        {
            await s.CopyToAsync(Stream.Null);
        }
    }
}

}

Primary changes:

Adds a CopyToAsync override on the HTTP/2 response content stream.
Stops forcing the Expect and TransferEncoding request headers into existence. They should now only be allocated if the dev explicitly sets them on the request.
Removes string allocations from Uri.IdnHost, while also reducing the amount of unsafe code used.
Removes the Http2Connection.ReadAtLeastAsync method, putting the relevant logic into its sole caller, EnsureIncomingBytesAsync, without increasing its size (and reducing overall number of lines of code).
Reduces the number of calls to ReadFrameAsync that actually need to yield, such that more complete synchronously and never need to allocate a state machine.
Reduce the size of the Http2Connection.SendAsync state machine.
Add additional common header values to KnownHeaders

Additionally:

When first measuring, I neglected to disable cookies, and the microsoft.com home page triggers a bunch of related functionality. Before disabling that in the test, I removed a bunch of temporarily allocations related to cookies, like unnecessary enumerator allocations, also removing some associated interface dispatch.

cc: @scalablecory, @davidsh, @MihaZupan, @JamesNK
Contributes to #31235

Make CookieParser/CookieTokenizer into structs. CookieParser is just created as a helper with some state to help a loop do parsing; it's not passed around in any way. And CookieTokenizer is just the implementation of CookieParser separated out for some reason. They can both be structs.
Remove enumerator allocations related to cookies. Also reduces interface method invocations.

This is happening for every request. But if the host is already all ASCII, we don't need to create a new string, and if it's already all lowercase, we don't need to create yet another new string. (This logic could stand to be cleaned up further; I just removed the allocations and some unnecessarily complex unsafe code along the way.)

There's no need for them to be separate, and separating them leads to an extra async stack frame / state machine.

Many frames either won't have a payload, or the act of waiting for the header will also end up waiting for the payload (in the same packet). By pulling out the wait in the hot path, we significantly reduce the number of times ReadFrameAsync will need to yield.

Reduces allocation / improves throughput when using responseStream.CopyToAsync.

Remove two lifted Int32s.

The order of a comparison operation is, based on C# required order of operations, forcing a temporary to be spilled and lifted to the state machine.

Avoids string allocations when these common response values are used.

As far as Uri is concerned, IdnEquivalent is doing a lot of work for nothing - that entire loop has no side-effects, it can be cut out entirely. Example

This method is only called from the Uri.IdnHost property getter.
IdnHost is currently not marked as nullable, but will still return null for empty hosts.
This change changes that to return string.Empty instead.

that entire loop has no side-effects

Which loop? Can you link to it?

Loop

Looking at other methods in that file, UnicodeEquivalent seems to be doing busy work as well to set ref parameters that are ignored by all call sites.

Looking at other methods in that file, UnicodeEquivalent seems to be doing busy work as well to set ref parameters that are ignored by all call sites.

I'll leave that to you to clean up separately. Thanks.

@ghost ghost locked as resolved and limited conversation to collaborators

Dec 10, 2020