Skip to content

mbe: Change unused_macro_rules to a DenseBitSet #143456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 7, 2025
Merged

mbe: Change unused_macro_rules to a DenseBitSet #143456

merged 1 commit into from
Jul 7, 2025
+9 −6

Conversation

joshtriplett
Copy link
Member

Now that it only contains indexes, and no other information, a bitset provides a more compact and simpler representation.

This builds on #143416. Only the last commit is new.

@joshtriplett joshtriplett added the A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) label Jul 4, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 4, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jul 4, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 4, 2025

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@joshtriplett
Copy link
Member Author

@bors try @rust-timer queue

I don't expect an outsized performance impact, but it's worth checking.

@rust-timer

This comment has been minimized.

Sign in to view
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 4, 2025
@bors
Copy link
Collaborator

bors commented Jul 4, 2025

⌛ Trying commit 78c30b8 with merge e47107f...

bors added a commit that referenced this pull request Jul 4, 2025
@bors
Auto merge of #143456 - joshtriplett:mbe-unused-rules-bitset, r=<try>
e47107f 
mbe: Change `unused_macro_rules` to a `DenseBitSet`

Now that it only contains indexes, and no other information, a bitset provides a more compact and simpler representation.

This builds on <#143416>. Only the last commit is new.
@bors
Copy link
Collaborator

bors commented Jul 4, 2025

☀️ Try build successful - checks-actions
Build commit: e47107f (e47107ffd2fb38d522e6f93e174e250d36d43368)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e47107f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking 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
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -3.0%, secondary 0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 2.1% [1.4%, 2.9%] 2
Improvements ✅
(primary)		 -3.0% [-3.0%, -3.0%] 1
Improvements ✅
(secondary)		 -3.8% [-3.8%, -3.8%] 1
All ❌✅ (primary) -3.0% [-3.0%, -3.0%] 1

Cycles

Results (primary 2.0%, secondary -1.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 2.0% [2.0%, 2.0%] 1
Regressions ❌
(secondary)		 1.7% [0.8%, 2.7%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -3.4% [-4.0%, -2.3%] 3
All ❌✅ (primary) 2.0% [2.0%, 2.0%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 460.645s -> 459.958s (-0.15%)
Artifact size: 372.20 MiB -> 372.16 MiB (-0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 5, 2025
@joshtriplett
Copy link
Member Author

@bors rollup

@lqd
Copy link
Member

lqd commented Jul 5, 2025 •
edited
Loading

A mixed bitset is a dense bitset at low counts, so unless one has >2K macro rules for a given node there won't be a difference. It doesn't feel common today, but if we find some stress-testy crate, or if regular macro rules are increasingly used more (e.g. with macro rules derives), we'll easily be able to switch to that in the future. So this looks good to me.

And probably not even worth trying a SparseBitMatrix here, since these bitsets are used in a map with numerous keys.

r? lqd and r=me on the last commit once #143416 lands

@rustbot rustbot assigned lqd and unassigned jieyouxu Jul 5, 2025
@joshtriplett joshtriplett force-pushed the mbe-unused-rules-bitset branch from 78c30b8 to 1aa5fd9 Compare July 5, 2025 10:09
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@joshtriplett joshtriplett force-pushed the mbe-unused-rules-bitset branch from 1aa5fd9 to 2a8e5c1 Compare July 5, 2025 23:26
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@joshtriplett joshtriplett force-pushed the mbe-unused-rules-bitset branch from 2a8e5c1 to b2058ee Compare July 6, 2025 02:36
@joshtriplett
mbe: Change unused_macro_rules to a DenseBitSet
8ee1e98 
Now that it only contains indexes, and no other information, a bitset
provides a more compact and simpler representation.
@joshtriplett joshtriplett force-pushed the mbe-unused-rules-bitset branch from b2058ee to 8ee1e98 Compare July 6, 2025 19:02
@joshtriplett
Copy link
Member Author

@bors r=lqd rollup

@bors
Copy link
Collaborator

bors commented Jul 6, 2025

📌 Commit 8ee1e98 has been approved by lqd

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 6, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 6, 2025
@jhpratt jhpratt mentioned this pull request Jul 7, 2025
Merged
bors added a commit that referenced this pull request Jul 7, 2025
@bors
Auto merge of #143556 - jhpratt:rollup-nid39y2, r=jhpratt
0d11be5 
Rollup of 9 pull requests

Successful merges:

 - #143206 (Align attr fixes)
 - #143236 (Stabilize `mixed_integer_ops_unsigned_sub`)
 - #143344 (Port `#[path]` to the new attribute parsing infrastructure )
 - #143359 (Link to 2024 edition page for `!` fallback changes)
 - #143456 (mbe: Change `unused_macro_rules` to a `DenseBitSet`)
 - #143529 (Renamed retain_mut to retain on LinkedList as mentioned in the ACP)
 - #143535 (Remove duplicate word)
 - #143544 (compiler: rename BareFn to FnPtr)
 - #143552 (lib: more eagerly return `self.len()` from `ceil_char_boundary`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a1b51aa into rust-lang:master Jul 7, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 7, 2025
rust-timer added a commit that referenced this pull request Jul 7, 2025
@rust-timer
Unrolled build for #143456
0a42533 
Rollup merge of #143456 - joshtriplett:mbe-unused-rules-bitset, r=lqd

mbe: Change `unused_macro_rules` to a `DenseBitSet`

Now that it only contains indexes, and no other information, a bitset provides a more compact and simpler representation.

This builds on <#143416>. Only the last commit is new.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@workingjubilee workingjubilee workingjubilee left review comments

Assignees

@lqd lqd

Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.90.0
Development

Successfully merging this pull request may close these issues.
8 participants
@joshtriplett @rustbot @rust-timer @bors @lqd @rust-log-analyzer @workingjubilee @jieyouxu