Avoid the would-change check for bitset chunks that are unique#153754
Avoid the would-change check for bitset chunks that are unique#153754Zalathar wants to merge 2 commits intorust-lang:mainfrom
Conversation
rustbot
added
S-waiting-on-author
|
Let's try benchmarking a version of this change that doesn't accidentally skip all the important work: @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.
Avoid the would-change check for bitset chunks that are unique
rustbot
added
the
S-waiting-on-perf
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (047dc4e): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary 1.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 480.488s -> 479.262s (-0.26%) |
rustbot
added
perf-regression
|
Oh, that's disappointing. Perhaps the vectorized equality test is actually faster than an unconditional update, on average. |
rustbot
removed
the
S-waiting-on-author
3 participants
ChunkedBitSet goes out of its way to check in advance whether a union/subtract/intersect operation would actually modify any bits, to avoid calling
Rc::make_mutif possible.But in the case where the Rc is already unique (i.e.
Rc::get_mutsucceeds), it's cheaper to skip the would-change check entirely, and just modify the words eagerly.This PR also adds some more thorough tests for ChunkedBitSet's union/subtract/intersect methods, which would have pinpointed a serious bug in the previous PR.