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

Bump SharedArrayPool's max arrays per partition default from 8 to 32 #87905

Merged
merged 1 commit into from Jun 29, 2023

Conversation

stephentoub
Copy link
Member

@stephentoub stephentoub commented Jun 22, 2023

The 8 value was picked arbitrarily years ago, with a smaller value being picked because nothing was ever trimmed from the pool. Since then, we've seen a significant increase in use of the pool, putting pressure on its storage, and we also added trimming so that memory pressure causes arrays to be pitched. Longer term, we might want to remove this limit entirely and have more of a dynamic scheme for allowing the buckets to grow and shrink. For now, though, I'm bumping the limit up from 8 arrays per core to 32 arrays per core to provide some more wiggle room. 32 is also somewhat arbitrary, though recent examples of a few real services that were hitting the 8 limit (resulting in increased allocation and contention) were mollified by 32.

@ghost
Copy link

ghost commented Jun 22, 2023

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

Issue Details

The 8 value was picked arbitrarily years ago, with a smaller value being picked because nothing was ever trimmed from the pool. Since then, we've seen a significant increase in use of the pool, putting pressure on its storage, and we also added trimming so that memory pressure causes arrays to be pitched. Longer term, we might want to remove this limit entirely and have more of a dynamic scheme for allowing the buckets to grow and shrink. For now, though, I'm bumping the limit up from 8 arrays per core to 32 arrays per core to provide some more wiggle room. 32 is also somewhat arbitrary, though recent examples on a few real services that were hitting the 8 limit (resulting in increased allocation and contention) were mollified by 32.

Author: stephentoub
Assignees: stephentoub
Labels:

area-System.Buffers

Milestone: -

The 8 value was picked arbitrarily years ago, with a smaller value being picked because nothing was ever trimmed from the pool.  Since then, we've seen a significant increase in use of the pool, putting pressure on its storage, and we also added trimming so that memory pressure causes arrays to be pitched.  Longer term, we might want to remove this limit entirely and have more of a dynamic scheme for allowing the buckets to grow and shrink.  For now, though, I'm bumping the limit up from 8 arrays per core to 32 arrays per core to provide some more wiggle room. 32 is also somewhat arbitrary, though recent examples on a few real services that were hitting the 8 limit (resulting in increased allocation and contention) were mollified by 32.
@stephentoub stephentoub merged commit d4fcdc1 into dotnet:main Jun 29, 2023
168 checks passed
@stephentoub stephentoub deleted the bumppool branch June 29, 2023 13:56
@dotnet dotnet locked as resolved and limited conversation to collaborators Jul 29, 2023
@MihaZupan
Copy link
Member

FYI I tracked a 10-15 % RPS bump in YARP HTTP2-HTTP2 benchmarks to this change.

crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenarios/proxy.benchmarks.yml --profile aspnet-gold-lin --scenario proxy-yarp-h2load --variable path=/?s=1024 --variable serverScheme=https --variable serverProtocol=http2 --variable downstreamScheme=https --variable downstreamProtocol=http2 --application.framework net8.0 --application.environmentVariables DOTNET_SYSTEM_BUFFERS_SHAREDARRAYPOOL_MAXARRAYSPERPARTITION=64 --json 64g.json

aspnet-citrine-lin (28 cores)

ArraysPerPartition RPS
8 326,289
16 343,371
24 352,604
32 367,828
40 376,155
48 379,122
64 388,685
96 388,794

aspnet-gold-lin (56 cores)

ArraysPerPartition RPS
8 666,206
32 732,067
48 745,488
64 737,689

@stephentoub
Copy link
Member Author

FYI I tracked a 10-15 % RPS bump in YARP HTTP2-HTTP2 benchmarks to this change.

Nice. Does it improve further if you set the DOTNET_SYSTEM_BUFFERS_SHAREDARRAYPOOL_MAXARRAYSPERPARTITION environment variable to an even larger value?

@stephentoub
Copy link
Member Author

stephentoub commented Sep 7, 2023

Nevermind, I see that's what you did in those tables. Looks like 32 is indeed a sweet spot then for yarp?

@MihaZupan
Copy link
Member

Looks like a higher limit can improve it a bit further, depending on machine.
E.g. 64 seems to be ~5 % better on the 28-core machine, but makes no difference on the 56-core one.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants