Skip to content

Conversation

Zalathar
Copy link
Contributor

There doesn't appear to be any reason to clear a ChunkedBitSet via its constructor (which allocates a new list of chunks), when we could just fill the existing allocation with Chunk::Zeros instead.

For comparison, the insert_all impl added by the same PR (#93984) does the simple thing here and just overwrites every chunk with Chunk::Ones.

(That fill was then made somewhat easier by #145480, which removes the chunk size from all-zero/all-one chunks.)

r? nnethercote (or compiler)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 13, 2025
@Zalathar
Copy link
Contributor Author

This clear method appears to currently be only used in one place (TransferFunction), so I have no idea how hot it is.

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rust-bors
Copy link

rust-bors bot commented Oct 13, 2025

⌛ Trying commit 279a5b3 with merge 9d4cb0c

To cancel the try build, run the command @bors try cancel.

Workflow: https://github.com/rust-lang/rust/actions/runs/18453583260

rust-bors bot added a commit that referenced this pull request Oct 13, 2025
Clear `ChunkedBitSet` without reallocating
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 13, 2025
Copy link
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, I wrote almost exactly the same commit earlier today, but I hadn't uploaded it yet:

commit 0418d4a82fd4b675b61afa284a28d0c81196af95
Author: Nicholas Nethercote <n.nethercote@gmail.com>
Date:   Mon Oct 13 08:06:32 2025 +1100

    Improve `ChunkedBitSet::clear`.

    The current code overwrites the existing bitset with a new empty bitset,
    which requires allocation. This PR changes it to just modify the
    existing bitset without needing extra allocations. This makes `clear`
    very similar to `insert_all`, which is nice. The new code is also
    faster, though this doesn't really matter because `clear` isn't hot.

diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs
index f7649c5f5c5..7360c6f3fb8 100644
--- a/compiler/rustc_index/src/bit_set.rs
+++ b/compiler/rustc_index/src/bit_set.rs
@@ -584,9 +584,11 @@ pub fn new_filled(domain_size: usize) -> Self {
         ChunkedBitSet::new(domain_size, /* is_empty */ false)
     }

+    /// Sets all bits to false.
     pub fn clear(&mut self) {
-        let domain_size = self.domain_size();
-        *self = ChunkedBitSet::new_empty(domain_size);
+        for chunk in self.chunks.iter_mut() {
+            *chunk = Zeros;
+        }
     }

     #[cfg(test)]

The only difference is I used a for loop.

r=me if you change clear to use a for loop or insert_all to use fill_with -- either way is fine, but it would be nice for them to look the same.

View changes since this review

@Zalathar
Copy link
Contributor Author

@bors try cancel

@rust-bors
Copy link

rust-bors bot commented Oct 13, 2025

Try build cancelled. Cancelled workflows:

@nnethercote
Copy link
Contributor

I don't think this needs a perf run.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 13, 2025

📌 Commit 9ff52bf has been approved by nnethercote

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-perf Status: Waiting on a perf run to be completed. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants