Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library #147101

yotamofek · 2025-09-27T18:03:24Z

Now that #137122 has landed, we can replace stuff that looks like:

let a: &[T];
let b: &[T];
let eq = a.len() == b.len() && a.iter().zip(b).all(|(a,b)| a == b)

with the much simpler a.iter().eq(b), without losing the perf benefit of the different-length-fast-path.
Also dogfooded Iterator::eq_by (cc #64295 ) while I'm at it.

First commit (4d1b6fa) should be very straightforward to review, second one (049a460) is slightly more creative, but IMHO a nice cleanup.

rustbot · 2025-09-27T18:03:27Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_builtin_macros/src/autodiff.rs

cc @ZuseZ4

rustbot · 2025-09-27T18:03:29Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
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

jdonszelmann · 2025-09-27T23:02:38Z

r? jdonszelmann

jdonszelmann · 2025-09-28T13:13:15Z

compiler/rustc_attr_parsing/src/parser.rs

    /// `self` may contain more segments than the number matched  against.
    pub fn starts_with(&self, segments: &[Symbol]) -> bool {
-        segments.len() < self.len() && self.segments().zip(segments).all(|(a, b)| a.name == *b)
+        segments.len() < self.len()


is this really better than what was there before? you still need to take() after the less-than check

I'm not sure which version I like more, but it's definitely not a clear-cut improvement so I undid this change.

jdonszelmann · 2025-09-28T13:18:47Z

@rustbot author

rustbot · 2025-09-28T13:18:51Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

yotamofek · 2025-09-28T13:30:59Z

@rustbot ready

jdonszelmann · 2025-09-28T22:29:38Z

r=me if you squash the two commits, don't quite see why the 2nd commit is separate from the first

jdonszelmann · 2025-09-28T22:29:50Z

@rustbot author

rustbot · 2025-09-29T05:10:31Z

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.

yotamofek · 2025-09-29T05:27:26Z

r=me if you squash the two commits, don't quite see why the 2nd commit is separate from the first

The separation was just to make reviewing slightly easier :)
Squashed, thanks!
@rustbot ready

jdonszelmann · 2025-09-29T06:29:12Z

@bors r+ rollup

bors · 2025-09-29T06:29:15Z

📌 Commit 68a7c25 has been approved by jdonszelmann

It is now in the queue for this repository.

Rollup of 5 pull requests Successful merges: - #146653 (improve diagnostics for empty attributes) - #146987 (impl Ord for params and use unstable sort) - #147101 (Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library ) - #147123 (Fix removed version numbers of `doc_auto_cfg` and `doc_cfg_hide`) - #147149 (add joboet to library review rotation) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #147101 - yotamofek:pr/iter-eq-and-eq-by, r=jdonszelmann Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library Now that #137122 has landed, we can replace stuff that looks like: ```rust let a: &[T]; let b: &[T]; let eq = a.len() == b.len() && a.iter().zip(b).all(|(a,b)| a == b) ``` with the much simpler `a.iter().eq(b)`, without losing the perf benefit of the different-length-fast-path. Also dogfooded `Iterator::eq_by` (cc #64295 ) while I'm at it. First commit (4d1b6fa) should be very straightforward to review, second one (049a460) is slightly more creative, but IMHO a nice cleanup.

Rollup of 5 pull requests Successful merges: - rust-lang/rust#146653 (improve diagnostics for empty attributes) - rust-lang/rust#146987 (impl Ord for params and use unstable sort) - rust-lang/rust#147101 (Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library ) - rust-lang/rust#147123 (Fix removed version numbers of `doc_auto_cfg` and `doc_cfg_hide`) - rust-lang/rust#147149 (add joboet to library review rotation) r? `@ghost` `@rustbot` modify labels: rollup

…jdonszelmann Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library Now that rust-lang#137122 has landed, we can replace stuff that looks like: ```rust let a: &[T]; let b: &[T]; let eq = a.len() == b.len() && a.iter().zip(b).all(|(a,b)| a == b) ``` with the much simpler `a.iter().eq(b)`, without losing the perf benefit of the different-length-fast-path. Also dogfooded `Iterator::eq_by` (cc rust-lang#64295 ) while I'm at it. First commit (4d1b6fa) should be very straightforward to review, second one (049a460) is slightly more creative, but IMHO a nice cleanup.

Rollup of 5 pull requests Successful merges: - rust-lang/rust#146653 (improve diagnostics for empty attributes) - rust-lang/rust#146987 (impl Ord for params and use unstable sort) - rust-lang/rust#147101 (Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library ) - rust-lang/rust#147123 (Fix removed version numbers of `doc_auto_cfg` and `doc_cfg_hide`) - rust-lang/rust#147149 (add joboet to library review rotation) r? `@ghost` `@rustbot` modify labels: rollup

…gjubilee In `BTreeMap::eq`, do not compare the elements if the sizes are different. Reverts rust-lang#147101 in library/alloc/src/btree/ rust-lang#147101 replaced some instances of code like `a.len() == b.len() && a.iter().eq(&b)` with just `a.iter().eq(&b)`, but the optimization that PR introduced only applies for `TrustedLen` iterators, and `BTreeMap`'s itertors are not `TrustedLen`, so this theoretically regressed perf for comparing large `BTreeMap`/`BTreeSet`s with unequal lengths but equal prefixes, (and also made it so that comparing two different-length `BTreeMap`/`BTreeSet`s with elements whose `PartialEq` impls that can panic now can panic, though this is not a "promised" behaviour either way (cc rust-lang#149122)) Given that `TrustedLen` is an unsafe trait, I opted to not implement it for `BTreeMap`'s iterators, and instead just revert the change. If someone else wants to audit `BTreeMap`'s iterators to make sure they always return the right number of items (even in the face of incorrect user `Ord` impls) and then implement `TrustedLen` for them so that the optimization works for them, then this can be closed in favor of that (or if the perf regression is deemed too theoretical, this can be closed outright). Example of theoretical perf regression: https://play.rust-lang.org/?version=beta&mode=release&edition=2024&gist=a37e3d61e6bf02669b251315c9a44fe2 (very rough estimates, using `Instant::elapsed`). In release mode on stable the comparison takes ~23.68µs. In release mode on beta/nightly the comparison takes ~48.351057ms.

In `BTreeMap::eq`, do not compare the elements if the sizes are different. Reverts #147101 in library/alloc/src/btree/ #147101 replaced some instances of code like `a.len() == b.len() && a.iter().eq(&b)` with just `a.iter().eq(&b)`, but the optimization that PR introduced only applies for `TrustedLen` iterators, and `BTreeMap`'s itertors are not `TrustedLen`, so this theoretically regressed perf for comparing large `BTreeMap`/`BTreeSet`s with unequal lengths but equal prefixes, (and also made it so that comparing two different-length `BTreeMap`/`BTreeSet`s with elements whose `PartialEq` impls that can panic now can panic, though this is not a "promised" behaviour either way (cc #149122)) Given that `TrustedLen` is an unsafe trait, I opted to not implement it for `BTreeMap`'s iterators, and instead just revert the change. If someone else wants to audit `BTreeMap`'s iterators to make sure they always return the right number of items (even in the face of incorrect user `Ord` impls) and then implement `TrustedLen` for them so that the optimization works for them, then this can be closed in favor of that (or if the perf regression is deemed too theoretical, this can be closed outright). Example of theoretical perf regression: https://play.rust-lang.org/?version=beta&mode=release&edition=2024&gist=a37e3d61e6bf02669b251315c9a44fe2 (very rough estimates, using `Instant::elapsed`). In release mode on stable the comparison takes ~23.68µs. In release mode on beta/nightly the comparison takes ~48.351057ms.

rustbot assigned fee1-dead Sep 27, 2025

yotamofek mentioned this pull request Sep 27, 2025

Tracking issue for iter_order_by #64295

Open

2 tasks

This comment has been minimized.

Sign in to view

yotamofek force-pushed the pr/iter-eq-and-eq-by branch from 085e528 to 049a460 Compare September 27, 2025 18:55

This comment has been minimized.

Sign in to view

rustbot assigned jdonszelmann and unassigned fee1-dead Sep 27, 2025

jdonszelmann reviewed Sep 28, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 28, 2025

yotamofek force-pushed the pr/iter-eq-and-eq-by branch from 049a460 to f9d10c7 Compare September 28, 2025 13:30

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 28, 2025

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 28, 2025

Use Iterator::eq and (dogfood) eq_by in compiler and library

68a7c25

yotamofek force-pushed the pr/iter-eq-and-eq-by branch from f9d10c7 to 68a7c25 Compare September 29, 2025 05:10

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 29, 2025

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 Sep 29, 2025

Zalathar mentioned this pull request Sep 29, 2025

Rollup of 5 pull requests #147151

Merged

bors merged commit 08616a1 into rust-lang:master Sep 29, 2025
10 checks passed

rustbot added this to the 1.92.0 milestone Sep 29, 2025

yotamofek deleted the pr/iter-eq-and-eq-by branch October 30, 2025 18:17

theemathas mentioned this pull request Nov 20, 2025

1.92 regression: Behavior change in PartialEq impl of BTreeSet? #149122

Closed

zachs18 mentioned this pull request Nov 20, 2025

In BTreeMap::eq, do not compare the elements if the sizes are different. #149125

Merged

Use Iterator::eq and (dogfood) eq_by in compiler and library #147101

Use Iterator::eq and (dogfood) eq_by in compiler and library #147101

Uh oh!

Conversation

yotamofek commented Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 27, 2025

Uh oh!

rustbot commented Sep 27, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

jdonszelmann commented Sep 27, 2025

Uh oh!

jdonszelmann Sep 28, 2025

Choose a reason for hiding this comment

Uh oh!

yotamofek Sep 28, 2025

Choose a reason for hiding this comment

Uh oh!

jdonszelmann commented Sep 28, 2025

Uh oh!

rustbot commented Sep 28, 2025

Uh oh!

This comment has been minimized.

yotamofek commented Sep 28, 2025

Uh oh!

jdonszelmann commented Sep 28, 2025

Uh oh!

jdonszelmann commented Sep 28, 2025

Uh oh!

rustbot commented Sep 29, 2025

Uh oh!

yotamofek commented Sep 29, 2025

Uh oh!

jdonszelmann commented Sep 29, 2025

Uh oh!

bors commented Sep 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library #147101

Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library #147101

yotamofek commented Sep 27, 2025 •

edited

Loading