perf: Store small bitsets inline in DenseBitSet - #160248
Conversation
|
@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.
perf: Store small bitsets inline in DenseBitSet
This comment has been minimized.
This comment has been minimized.
|
Note that this is how it was originally stored, but it's been removed in #147644 for performance reasons. There's been a few more experiments for DenseBitSet going on recently, notably: #153589, #141325 and #157024 These all try to address the similar problem. In my opinion, the Raw bit set from Zalathar is closest to what we'd probably want to do. I'm skeptical that the solutions that try to solve it internally will work, because they introduce branch to every access that's troublesome to optimize out. What I think is more fruitful experiment (and I want to try at some point, if nobody else beat me to it) is to make the BitSet generic over the storage and move the allocation decision to the creation time, such that we can only pass |
|
Finished benchmarking commit (98ed799): 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 0.7%, secondary 1.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -0.2%, secondary -0.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 492.09s -> 488.725s (-0.68%) |
|
Thanks @panstromek, I had profiled without |
|
No problem, sometimes it's also worth trying again, so don't worry about it. The change to |
DenseBitSetheap-allocated its words even for a tiny domain, and dataflow allocates one per basic block. Storing the words in aSmallVec<[Word; 2]>keeps bitsets up to 128 bits inline, so small ones no longer allocate.