Use the new async state machine pooling feature on websockets

[davidfowl]

Description

Looking at an allocation profile for SignalR, the top allocations are coming from state machines allocated by Kestrel on websocket reads. We should be able to remove these by using the new pooling introduced here #50116.

Regression?

No

Data

Analysis

This is the code path:

runtime/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs

Line 685 in d927fcb

private async ValueTask<TWebSocketReceiveResult> ReceiveAsyncPrivate<TWebSocketReceiveResultGetter, TWebSocketReceiveResult>(

cc @stephentoub @CarnaViire

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

Issue Details

---

Description

Looking at an allocation profile for SignalR, the top allocations are coming from state machines allocated by Kestrel on websocket reads. We should be able to remove these by using the new pooling introduced here #50116.

Regression?

No

Data

Analysis

This is teh codez:

runtime/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs

Line 685 in d927fcb

private async ValueTask<TWebSocketReceiveResult> ReceiveAsyncPrivate<TWebSocketReceiveResultGetter, TWebSocketReceiveResult>(

cc @stephentoub @CarnaViire

Author:	davidfowl
Assignees:	-
Labels:	area-System.Net, tenet-performance, untriaged
Milestone:	-

[stephentoub]

The plan is to use the feature in a few places based on profiling to see that it not only reduces allocations but also improves throughput under load. This is certainly one of the places we'll experiment with.

We haven't done that yet because the feature hasn't been implemented in the compiler yet. There's no action that can be reasonably taken until that happens.

[davidfowl]

The plan is to use the feature in a few places based on profiling to see that it not only reduces allocations but also improves throughput under load.

This is the one that always gets me. CPU time spent in the GC vs CPU time spent in our own pools calling rent/return.

We haven't done that yet because the feature hasn't been implemented in the compiler yet. There's no action that can be reasonably taken until that happens.

Yea, I know, I'm just filing issues and storing profiles.

[stephentoub]

This is the one that always gets me. CPU time spent in the GC vs CPU time spent in our own pools calling rent/return.

"gets you" meaning you forget that there's a cost to pooling, or "gets you" meaning you think we shouldn't be concerned about such costs and should always pool everything?

[davidfowl]

"gets you" meaning you forget that there's a cost to pooling, or "gets you" meaning you think we shouldn't be concerned about such costs and should always pool everything?

The latter assuming it doesn't make things worse because allocations are like paper cuts that add up (peanut butter). There are a couple of things we don't have:

• IMO not allocating for infrastructure things where it can be easily avoided makes sense
• As libraries that run in various scenarios, us allocating means the end application needs to tune the GC to meet their performance needs. This means they need to understand the framework and the runtime's allocations.

We don't know how to measure the CPU impact of all of these small pools and that's problematic when comparing to if it's better than GC if you don't see a throughput increase (or working set decrease).

The other very unsatisfying thing IMO is not seeing a working set reduction when reducing allocations... (gen0 ones). It feels like we're just delaying the time to when a GC happens but not saving on overall memory of the application.

[stephentoub]

They're all "allocations"; it's just that only some of them show up in GC allocation profile traces. We could allocate and free native memory, that wouldn't show up on a managed profile, but it could be a worse issue. We could allocate and free by renting and returning from some pool, and that similarly wouldn't show up on a profile. But these things all have costs. In sense, the GC itself is a pool: it grabs some memory from the OS, and then effectively rents out pieces of it... it's just that rather than those pieces being explicitly returned by code, the GC goes and returns them itself. There are many considerations that need to factor in when pooling, and most pool implementations don't, e.g. using objects in a manner less likely to cause cache problems.

We don't know how to measure the CPU impact of all of these small pools and that's problematic when comparing to if it's better than GC if you don't see a throughput increase (or working set decrease).

Right. So if we can't come up with a scenario where we see a meaningful improvement, we're better off not adding the complexity of a custom pool, just to hide it from a managed allocation profile, with no other benefits we're able to measure, especially if that pool doesn't play nicely with all of the GC's knobs, which close to none do.

The latter assuming it doesn't make things worse

This is where we differ. You say "do it as long as it doesn't make things worse". I say "do it if it makes things better".

cc: @Maoni0

[davidfowl]

We could allocate and free by renting and returning from some pool, and that similarly wouldn't show up on a profile. But these things all have costs. In sense, the GC itself is a pool: it grabs some memory from the OS, and then effectively rents out pieces of it... it's just that rather than those pieces being explicitly returned by code, the GC goes and returns them itself. There are many considerations that need to factor in when pooling, and most pool implementations don't, e.g. using objects in a manner less likely to cause cache problems.

This is exactly why we need to be able to measure because we measure scenarios and make decisions, when as a library author, you don't have any control over those scenarios. The peanut butter nature of gen0 allocations is what bugs me and why it "feels right" as a library author to err on the side of reuse as much as possible where it doesn't make the code crazy. Now I'm no advocating for global pools for all the things (I think they suck in the various ways you mention) but its really the only control you have outside of telling the app developer to

Right. So if we can't come up with a scenario where we see a meaningful improvement, we're better off not adding the complexity of a custom pool, just to hide it from a managed allocation profile, with no other benefits we're able to measure, especially if that pool doesn't play nicely with all of the GC's knobs, which close to none do.

Complexity is in the eye of the beholder. I think we just need to be able to measure the cost of these custom pools better. It's the only knob we have as library authors.

This is where we differ. You say "do it as long as it doesn't make things worse". I say "do it if it makes things better".

This is where we disagree 😄, because I plan to pool where possible (and where it seems wasteful) and where it doesn't make things too complex (judgement call obviously). I see it as giving more headroom to the applications where libraries exist as a guest.

[geoffkizer]

using objects in a manner less likely to cause cache problems

I wonder about this specifically re pooling of state machines. The state machines hold the async "stack". Seems like GC allocations would do better at locality than pooling.

[davidfowl]

using objects in a manner less likely to cause cache problems

I'd also like to be able to measure some of this as well because I assume this is also very much like peanut butter.

[davidfowl]

I guess this comes down to the fact that gen0 isn't better than pooling in lots of cases. We've seen evidence in Kestrel that pooling has cumulatively improved the overall throughput. I guess you could write a pool that was really bad and ended up making the performance worse or one that is too complex to maintain but generally well isolated pools, or simple object reuse is better.

[benaadams]

The other very unsatisfying thing IMO is not seeing a working set reduction when reducing allocations... (gen0 ones). It feels like we're just delaying the time to when a GC happens but not saving on overall memory of the application.

I see this; short of reducing allocs to 0 or adding the settings to cap the GC size, even if your allocations are only 1000 Tasks per second eventually it will tend to a 1GB of heap even if only 5MB is ever in use. I did think there was some heuristic that the GC used to adjust its allocation budget as to collect to match the memory size to allocation rate? https://devblogs.microsoft.com/dotnet/garbage-collection-at-food-courts/

This is the same as GC adjusts the amount of allocations before the next collection. It’s called the allocation budget. And this “adjusting” that GC does is part of its dynamic tuning. As the process runs, GC will modify this allocation budget according to the survival rate it observes to keep itself productive.

Do the startup long surviving allocations throw it off; or maybe I'm misunderstanding the allocation budget. Perhaps it starts off "infinite" and reduces if allocations are high (large heap -> collected faster large heap); rather than starting off low and increasing if allocations are high (small heap -> large heap) /cc @Maoni0

[benaadams]

eventually it will tend to a 1GB of heap even if only 5MB is ever in use

Well; maybe not 1GB anymore with the segment size changes; but larger than expected as a slow allocation rate waits a very long time for a GC by which time the heap has grown significantly even if nothing is ever particularly in use (on a machine with significant RAM and no GC setting tweaking).

Saying that I haven't experienced any performance issues from this large heap collection; its more of a perception issue from looking at what the OS reports as the app is using, so perhaps its more of a feel good factor? 😅