perf: Give sharded maps their capacity in total rather than per shard [parallel only: -14.7% max-rss, -4.4% cycles at -Zthreads=8] - #81
Draft
xmakro wants to merge 1 commit into
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ShardedHashMap::with_capacitypasses the requested capacity to every shard, so with 32 shards a map asks for 32 times the memory its caller intended.CtxtInterners::newsizes its tables from observed interner sizes, and the comment there describes those figures as the size of the interner as a whole, sotype_atN * 16reserves room for about a million entries instead of the thirty odd thousand that were meant.CurrentDepGraph::newalready divides its estimate bysharded::shards()before handing it over, so the convention this restores is the one the other caller already follows.This only does anything when the compiler is running with more than one thread.
sharded::shards()is 1 unless the parallel front end is enabled, so a single threaded build reserves exactly what it reserved before and instruction counts do not move: I measure +0.14% on syn, serde, hyper and ripgrep, which is noise. Anyone screening this on the usualinstructions:usuite should expect to see nothing at all.At
-Zthreads=8on the same four crates, peak memory drops by 14.7% on the geometric mean, between 11.4% and 16.6% per crate. That was worth chasing because of where the time goes in a parallel build rather than because of the memory alone. Going from one thread to eight on this machine costs 76% more cycles for 2.9% more instructions, which is an IPC fall from 1.48 to 0.87, while blocking locks account for about 0.2% of samples. The threads are not waiting on each other, they are stalled on memory, so shrinking the footprint is a direct attack on the cost. Interleaved runs on syn confirm it: cycles fall by 4.4% and IPC rises from about 0.80 to about 0.83, repeatably, with instructions flat.I have not been able to confirm the corresponding wall clock improvement. The machine had an unrelated profiling job on it while I was measuring, and wall clock is not meaningful under that kind of load, so the figures above are deliberately limited to per process counters and peak memory, which are not affected by it. A rust-timer run would settle the wall clock question.