[experiment] Use DenseBitSetStorage to store MixedBitSet::Small inline - #161454
[experiment] Use DenseBitSetStorage to store MixedBitSet::Small inline#161454panstromek wants to merge 9 commits into
Conversation
This doesn't change any behavior and still keeps the default storage to Vec<Word>.
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
[experiment] Use DenseBitSetStorage to store MixedBitSet::Small inline
This comment has been minimized.
This comment has been minimized.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Finished benchmarking commit (477da19): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -1.0%, secondary -2.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.2%, secondary 4.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.1%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 457.604s -> 455.214s (-0.52%) |
|
Turns out there's a very subtle bug in this, the failing test is a symptom, but I'm surprised that something else didn't fail much more loudly. |
Depends on the base from #161345, uses slightly different design with a new DenseBitSetStorage trait, that makes it somewhat easier to thread generic params through the codebase.
We use this new design to store
MixedBitSet::Smallvariant inline in 2-word array, instead of Heap.I'm not super happy about this test case for 2 reasons
MixedBitSetis not really a good example of the target use case for this abstractionBut it's also hottest usage of small
DenseBitSetso I hope I'll see some effect compared to #161345 (which was tested on roughly second hottest small bitset onserde).Smallbitsets, which might have side effects that mask the potential improvementsBut according to my measurements,
::Smallvariant is almost always <=2 words in our benchmark suite. I believe there's at most 1 case where it is 4 words (maybe none, but line numbers changed since I measured this so I'm not sure).