Uh oh!
There was an error while loading. Please reload this page.
Add a != check to ChunkedBitSet::union. - #147619
Conversation
nnethercote
commented
Oct 13, 2025
@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.
Add a `!=` check to `ChunkedBitSet::union`.
This comment has been minimized.
This comment has been minimized.
Zalathar
commented
Oct 13, 2025
For future work, we could hypothetically get even more clever, and try to start the But that might not be worth the extra implementation complexity (or might even reduce the benefit of the equality check). |
rust-timer
commented
Oct 13, 2025
Finished benchmarking commit (701ca72): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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. @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 (primary 2.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -3.3%, secondary 2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 472.507s -> 474.412s (0.40%) |
nnethercote
commented
Oct 13, 2025
Interesting idea! I tried it out, with code like this: let i = match iter::zip(&self_chunk_words[0..num_words],&other_chunk_words[0..num_words],).position(|(a, b)| a != b){Some(i) => i,None => continue,// all elements equal, nothing more to do};let op = |a, b| a | b;if !bitwise_changes(&self_chunk_words[i..num_words],&other_chunk_words[i..num_words],
op,){continue;}// ... do the mutation ...And it ended up being a significant slowdown. I think because that |
b1eb8e1 to
ad8b2e0Comparennethercote
commented
Oct 13, 2025
I have update the code a little to include some refactoring. |
Zalathar
commented
Oct 13, 2025
Looks good, thanks! @bors r+ |
bors
commented
Oct 13, 2025
This comment has been minimized.
This comment has been minimized.
…illot,Zalathar Bitset cleanups Some minor cleanups I did while working on rust-lang#147619. r? `@Zalathar`
…illot,Zalathar Bitset cleanups Some minor cleanups I did while working on rust-lang#147619. r? `@Zalathar`
…illot,Zalathar Bitset cleanups Some minor cleanups I did while working on rust-lang#147619. r? ``@Zalathar``
…illot,Zalathar Bitset cleanups Some minor cleanups I did while working on rust-lang#147619. r? ```@Zalathar```
This comment has been minimized.
This comment has been minimized.
Uh, so what's happening? I'm generally capable of managing my own PRs. Have you taken this over now? Can I do anything with it or will that mess things up? |
This comment has been minimized.
This comment has been minimized.
Zalathar
commented
Oct 15, 2025
Ah, I was thinking it would be helpful to rebase this, but maybe I ended up causing more confusion. I've force-pushed back to your original commit, and I won't touch this any more, so please feel free to resolve it yourself and reapprove. |
nnethercote
commented
Oct 15, 2025
Ok, thanks. |
For less indentation, mostly.
It's a big speed win for cranelift-codegen-0.119.0.
ad8b2e0 to
6048182Comparerustbot
commented
Oct 15, 2025
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
nnethercote
commented
Oct 15, 2025
I have rebased. @bors r=Zalathar |
bors
commented
Oct 15, 2025
…athar Bitset cleanups Some minor cleanups I did while working on rust-lang/rust#147619. r? ```@Zalathar```
bors
commented
Oct 15, 2025
bors
commented
Oct 15, 2025
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 5413f7d (parent) -> 4f08307 (this PR) Test differencesNo test diffs found Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 4f08307f6efdbca86d27beecd257fad9913192e9 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
rust-timer
commented
Oct 15, 2025
Finished benchmarking commit (4f08307): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression 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 2.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -0.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 1.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 475.31s -> 476.148s (0.18%) |
Mark-Simulacrum
commented
Oct 21, 2025
The one regression looks spurious, dropping the label. |
…athar Bitset cleanups Some minor cleanups I did while working on rust-lang/rust#147619. r? ```@Zalathar```
…athar Bitset cleanups Some minor cleanups I did while working on rust-lang/rust#147619. r? ```@Zalathar```
It's a big speed win for
cranelift-codegen-0.119.0.r? @Zalathar