Auto merge of #149125 - zachs18:btreemap-eq-perf, r=workingjubilee

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

Reverts rust-lang/rust#147101 in library/alloc/src/btree/

rust-lang/rust#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/rust#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.
This commit is contained in:
bors
2025-12-02 17:04:58 +00:00
3 changed files with 98 additions and 1 deletions
+1 -1
View File
@@ -2444,7 +2444,7 @@ fn default() -> BTreeMap<K, V> {
#[stable(feature = "rust1", since = "1.0.0")]
impl<K: PartialEq, V: PartialEq, A: Allocator + Clone> PartialEq for BTreeMap<K, V, A> {
fn eq(&self, other: &BTreeMap<K, V, A>) -> bool {
self.iter().eq(other)
self.len() == other.len() && self.iter().zip(other).all(|(a, b)| a == b)
}
}
@@ -0,0 +1,96 @@
//! Regression tests which fail if some collections' `PartialEq::eq` impls compare
//! elements when the collections have different sizes.
//! This behavior is not guaranteed either way, so regressing these tests is fine
//! if it is done on purpose.
use std::cmp::Ordering;
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, LinkedList};
/// This intentionally has a panicking `PartialEq` impl, to test that various
/// collections' `PartialEq` impls don't actually compare elements if their sizes
/// are unequal.
///
/// This is not advisable in normal code.
#[derive(Debug, Clone, Copy, Hash)]
struct Evil;
impl PartialEq for Evil {
fn eq(&self, _: &Self) -> bool {
panic!("Evil::eq is evil");
}
}
impl Eq for Evil {}
impl PartialOrd for Evil {
fn partial_cmp(&self, _: &Self) -> Option<Ordering> {
Some(Ordering::Equal)
}
}
impl Ord for Evil {
fn cmp(&self, _: &Self) -> Ordering {
// Constructing a `BTreeSet`/`BTreeMap` uses `cmp` on the elements,
// but comparing it with with `==` uses `eq` on the elements,
// so Evil::cmp doesn't need to be evil.
Ordering::Equal
}
}
// check Evil works
#[test]
#[should_panic = "Evil::eq is evil"]
fn evil_eq_works() {
let v1 = vec![Evil];
let v2 = vec![Evil];
_ = v1 == v2;
}
// check various containers don't compare if their sizes are different
#[test]
fn vec_evil_eq() {
let v1 = vec![Evil];
let v2 = vec![Evil; 2];
assert_eq!(false, v1 == v2);
}
#[test]
fn hashset_evil_eq() {
let s1 = HashSet::from([(0, Evil)]);
let s2 = HashSet::from([(0, Evil), (1, Evil)]);
assert_eq!(false, s1 == s2);
}
#[test]
fn hashmap_evil_eq() {
let m1 = HashMap::from([(0, Evil)]);
let m2 = HashMap::from([(0, Evil), (1, Evil)]);
assert_eq!(false, m1 == m2);
}
#[test]
fn btreeset_evil_eq() {
let s1 = BTreeSet::from([(0, Evil)]);
let s2 = BTreeSet::from([(0, Evil), (1, Evil)]);
assert_eq!(false, s1 == s2);
}
#[test]
fn btreemap_evil_eq() {
let m1 = BTreeMap::from([(0, Evil)]);
let m2 = BTreeMap::from([(0, Evil), (1, Evil)]);
assert_eq!(false, m1 == m2);
}
#[test]
fn linkedlist_evil_eq() {
let m1 = LinkedList::from([Evil]);
let m2 = LinkedList::from([Evil; 2]);
assert_eq!(false, m1 == m2);
}
@@ -1 +1,2 @@
mod binary_heap;
mod eq_diff_len;