Skip to content

Commit

Permalink
Auto merge of rust-lang#119192 - michaelwoerister:mcp533-push, r=<try>
Browse files Browse the repository at this point in the history
Replace a number of FxHashMaps/Sets with stable-iteration-order alternatives

This PR replaces almost all of the remaining `FxHashMap`s in query results with either `FxIndexMap` or `UnordMap`. The only case that is missing is the `EffectiveVisibilities` struct which turned out to not be straightforward to transform. Once that is done too, we can remove the `HashStable` implementation from `HashMap`.

The first commit adds the `StableCompare` trait which is a companion trait to `StableOrd`. Some types like `Symbol` can be compared in a cross-session stable way, but their `Ord` implementation is not stable. In such cases, a `StableCompare` implementation can be provided to offer a lightweight way for stable sorting. The more heavyweight option is to sort via `ToStableHashKey`, but then sorting needs to have access to a stable hashing context and `ToStableHashKey` can also be expensive as in the case of `Symbol` where it has to allocate a `String`.

The rest of the commits are rather mechanical and don't overlap, so they are best reviewed individually.

Part of [MCP 533](rust-lang/compiler-team#533).
  • Loading branch information
bors committed Dec 22, 2023
2 parents ef1b78e + e71af93 commit e0bf0d3
Show file tree
Hide file tree
Showing 34 changed files with 303 additions and 149 deletions.
14 changes: 7 additions & 7 deletions compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::base::allocator_kind_for_codegen;
use std::collections::hash_map::Entry::*;

use rustc_ast::expand::allocator::{ALLOCATOR_METHODS, NO_ALLOC_SHIM_IS_UNSTABLE};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::unord::UnordMap;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId, LOCAL_CRATE};
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
Expand Down Expand Up @@ -393,10 +393,10 @@ fn exported_symbols_provider_local(
fn upstream_monomorphizations_provider(
tcx: TyCtxt<'_>,
(): (),
) -> DefIdMap<FxHashMap<GenericArgsRef<'_>, CrateNum>> {
) -> DefIdMap<UnordMap<GenericArgsRef<'_>, CrateNum>> {
let cnums = tcx.crates(());

let mut instances: DefIdMap<FxHashMap<_, _>> = Default::default();
let mut instances: DefIdMap<UnordMap<_, _>> = Default::default();

let drop_in_place_fn_def_id = tcx.lang_items().drop_in_place_fn();

Expand Down Expand Up @@ -445,7 +445,7 @@ fn upstream_monomorphizations_provider(
fn upstream_monomorphizations_for_provider(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> Option<&FxHashMap<GenericArgsRef<'_>, CrateNum>> {
) -> Option<&UnordMap<GenericArgsRef<'_>, CrateNum>> {
debug_assert!(!def_id.is_local());
tcx.upstream_monomorphizations(()).get(&def_id)
}
Expand Down Expand Up @@ -656,7 +656,7 @@ fn maybe_emutls_symbol_name<'tcx>(
}
}

fn wasm_import_module_map(tcx: TyCtxt<'_>, cnum: CrateNum) -> FxHashMap<DefId, String> {
fn wasm_import_module_map(tcx: TyCtxt<'_>, cnum: CrateNum) -> DefIdMap<String> {
// Build up a map from DefId to a `NativeLib` structure, where
// `NativeLib` internally contains information about
// `#[link(wasm_import_module = "...")]` for example.
Expand All @@ -665,9 +665,9 @@ fn wasm_import_module_map(tcx: TyCtxt<'_>, cnum: CrateNum) -> FxHashMap<DefId, S
let def_id_to_native_lib = native_libs
.iter()
.filter_map(|lib| lib.foreign_module.map(|id| (id, lib)))
.collect::<FxHashMap<_, _>>();
.collect::<DefIdMap<_>>();

let mut ret = FxHashMap::default();
let mut ret = DefIdMap::default();
for (def_id, lib) in tcx.foreign_modules(cnum).iter() {
let module = def_id_to_native_lib.get(def_id).and_then(|s| s.wasm_import_module());
let Some(module) = module else { continue };
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::errors;
use rustc_ast::ast;
use rustc_attr::InstructionSetAttr;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::unord::UnordMap;
use rustc_errors::Applicability;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
Expand All @@ -18,7 +18,7 @@ use rustc_span::Span;
pub fn from_target_feature(
tcx: TyCtxt<'_>,
attr: &ast::Attribute,
supported_target_features: &FxHashMap<String, Option<Symbol>>,
supported_target_features: &UnordMap<String, Option<Symbol>>,
target_features: &mut Vec<Symbol>,
) {
let Some(list) = attr.meta_item_list() else { return };
Expand Down
26 changes: 26 additions & 0 deletions compiler/rustc_data_structures/src/stable_hasher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,32 @@ unsafe impl<T: StableOrd> StableOrd for &T {
const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;
}

/// This is a companion trait to `StableOrd`. Some types like `Symbol` can be
/// compared in a cross-session stable way, but their `Ord` implementation is
/// not stable. In such cases, a `StableOrd` implementation can be provided
/// to offer a lightweight way for stable sorting. (The more heavyweight option
/// is to sort via `ToStableHashKey`, but then sorting needs to have access to
/// a stable hashing context and `ToStableHashKey` can also be expensive as in
/// the case of `Symbol` where it has to allocate a `String`.)
///
/// See the documentation of [StableOrd] for how stable sort order is defined.
/// The same definition applies here. Be careful when implementing this trait.
pub trait StableCompare {
const CAN_USE_UNSTABLE_SORT: bool;

fn stable_cmp(&self, other: &Self) -> std::cmp::Ordering;
}

/// `StableOrd` denotes that the type's `Ord` implementation is stable, so
/// we can implement `StableCompare` by just delegating to `Ord`.
impl<T: StableOrd> StableCompare for T {
const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;

fn stable_cmp(&self, other: &Self) -> std::cmp::Ordering {
self.cmp(other)
}
}

/// Implement HashStable by just calling `Hash::hash()`. Also implement `StableOrd` for the type since
/// that has the same requirements.
///
Expand Down
122 changes: 95 additions & 27 deletions compiler/rustc_data_structures/src/unord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
//! as required by the query system.

use rustc_hash::{FxHashMap, FxHashSet};
use smallvec::SmallVec;
use std::{
borrow::Borrow,
borrow::{Borrow, BorrowMut},
collections::hash_map::Entry,
hash::Hash,
iter::{Product, Sum},
Expand All @@ -14,7 +13,7 @@ use std::{

use crate::{
fingerprint::Fingerprint,
stable_hasher::{HashStable, StableHasher, StableOrd, ToStableHashKey},
stable_hasher::{HashStable, StableCompare, StableHasher, ToStableHashKey},
};

/// `UnordItems` is the order-less version of `Iterator`. It only contains methods
Expand Down Expand Up @@ -134,36 +133,78 @@ impl<'a, T: Copy + 'a, I: Iterator<Item = &'a T>> UnordItems<&'a T, I> {
}
}

impl<T: Ord, I: Iterator<Item = T>> UnordItems<T, I> {
impl<T, I: Iterator<Item = T>> UnordItems<T, I> {
#[inline]
pub fn into_sorted<HCX>(self, hcx: &HCX) -> Vec<T>
where
T: ToStableHashKey<HCX>,
{
let mut items: Vec<T> = self.0.collect();
items.sort_by_cached_key(|x| x.to_stable_hash_key(hcx));
items
self.collect_sorted(hcx, true)
}

#[inline]
pub fn into_sorted_stable_ord(self) -> Vec<T>
where
T: Ord + StableOrd,
T: StableCompare,
{
self.collect_stable_ord_by_key(|x| x)
}

#[inline]
pub fn into_sorted_stable_ord_by_key<K, C>(self, project_to_key: C) -> Vec<T>
where
K: StableCompare,
C: for<'a> Fn(&'a T) -> &'a K,
{
let mut items: Vec<T> = self.0.collect();
if !T::CAN_USE_UNSTABLE_SORT {
items.sort();
} else {
items.sort_unstable()
self.collect_stable_ord_by_key(project_to_key)
}

#[inline]
pub fn collect_sorted<HCX, C>(self, hcx: &HCX, cache_sort_key: bool) -> C
where
T: ToStableHashKey<HCX>,
C: FromIterator<T> + BorrowMut<[T]>,
{
let mut items: C = self.0.collect();

let slice = items.borrow_mut();
if slice.len() > 1 {
if cache_sort_key {
slice.sort_by_cached_key(|x| x.to_stable_hash_key(hcx));
} else {
slice.sort_by_key(|x| x.to_stable_hash_key(hcx));
}
}

items
}

pub fn into_sorted_small_vec<HCX, const LEN: usize>(self, hcx: &HCX) -> SmallVec<[T; LEN]>
#[inline]
pub fn collect_stable_ord_by_key<K, C, P>(self, project_to_key: P) -> C
where
T: ToStableHashKey<HCX>,
K: StableCompare,
P: for<'a> Fn(&'a T) -> &'a K,
C: FromIterator<T> + BorrowMut<[T]>,
{
let mut items: SmallVec<[T; LEN]> = self.0.collect();
items.sort_by_cached_key(|x| x.to_stable_hash_key(hcx));
let mut items: C = self.0.collect();

let slice = items.borrow_mut();
if slice.len() > 1 {
if !K::CAN_USE_UNSTABLE_SORT {
slice.sort_by(|a, b| {
let a_key = project_to_key(a);
let b_key = project_to_key(b);
a_key.stable_cmp(b_key)
});
} else {
slice.sort_unstable_by(|a, b| {
let a_key = project_to_key(a);
let b_key = project_to_key(b);
a_key.stable_cmp(b_key)
});
}
}

items
}
}
Expand Down Expand Up @@ -268,16 +309,30 @@ impl<V: Eq + Hash> UnordSet<V> {
}

/// Returns the items of this set in stable sort order (as defined by
/// `StableOrd`). This method is much more efficient than
/// `StableCompare`). This method is much more efficient than
/// `into_sorted` because it does not need to transform keys to their
/// `ToStableHashKey` equivalent.
#[inline]
pub fn to_sorted_stable_ord(&self) -> Vec<V>
pub fn to_sorted_stable_ord(&self) -> Vec<&V>
where
V: Ord + StableOrd + Clone,
V: StableCompare,
{
let mut items: Vec<V> = self.inner.iter().cloned().collect();
items.sort_unstable();
let mut items: Vec<&V> = self.inner.iter().collect();
items.sort_unstable_by(|a, b| a.stable_cmp(*b));
items
}

/// Returns the items of this set in stable sort order (as defined by
/// `StableCompare`). This method is much more efficient than
/// `into_sorted` because it does not need to transform keys to their
/// `ToStableHashKey` equivalent.
#[inline]
pub fn into_sorted_stable_ord(self) -> Vec<V>
where
V: StableCompare,
{
let mut items: Vec<V> = self.inner.into_iter().collect();
items.sort_unstable_by(V::stable_cmp);
items
}

Expand Down Expand Up @@ -483,16 +538,16 @@ impl<K: Eq + Hash, V> UnordMap<K, V> {
to_sorted_vec(hcx, self.inner.iter(), cache_sort_key, |&(k, _)| k)
}

/// Returns the entries of this map in stable sort order (as defined by `StableOrd`).
/// Returns the entries of this map in stable sort order (as defined by `StableCompare`).
/// This method can be much more efficient than `into_sorted` because it does not need
/// to transform keys to their `ToStableHashKey` equivalent.
#[inline]
pub fn to_sorted_stable_ord(&self) -> Vec<(K, &V)>
pub fn to_sorted_stable_ord(&self) -> Vec<(&K, &V)>
where
K: Ord + StableOrd + Copy,
K: StableCompare,
{
let mut items: Vec<(K, &V)> = self.inner.iter().map(|(&k, v)| (k, v)).collect();
items.sort_unstable_by_key(|&(k, _)| k);
let mut items: Vec<_> = self.inner.iter().collect();
items.sort_unstable_by(|(a, _), (b, _)| a.stable_cmp(*b));
items
}

Expand All @@ -510,6 +565,19 @@ impl<K: Eq + Hash, V> UnordMap<K, V> {
to_sorted_vec(hcx, self.inner.into_iter(), cache_sort_key, |(k, _)| k)
}

/// Returns the entries of this map in stable sort order (as defined by `StableCompare`).
/// This method can be much more efficient than `into_sorted` because it does not need
/// to transform keys to their `ToStableHashKey` equivalent.
#[inline]
pub fn into_sorted_stable_ord(self) -> Vec<(K, V)>
where
K: StableCompare,
{
let mut items: Vec<(K, V)> = self.inner.into_iter().collect();
items.sort_unstable_by(|a, b| a.0.stable_cmp(&b.0));
items
}

/// Returns the values of this map in stable sort order (as defined by K's
/// `ToStableHashKey` implementation).
///
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{
SubdiagnosticMessage, Substitution, SubstitutionPart, SuggestionStyle,
};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::unord::UnordMap;
use rustc_error_messages::fluent_value_from_str_list_sep_by_and;
use rustc_error_messages::FluentValue;
use rustc_lint_defs::{Applicability, LintExpectationId};
Expand Down Expand Up @@ -280,7 +281,7 @@ impl Diagnostic {

pub(crate) fn update_unstable_expectation_id(
&mut self,
unstable_to_stable: &FxHashMap<LintExpectationId, LintExpectationId>,
unstable_to_stable: &UnordMap<LintExpectationId, LintExpectationId>,
) {
if let Level::Expect(expectation_id) | Level::Warning(Some(expectation_id)) =
&mut self.level
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@ extern crate self as rustc_errors;

pub use emitter::ColorConfig;

use rustc_data_structures::unord::UnordMap;
use rustc_lint_defs::LintExpectationId;
use Level::*;

use emitter::{is_case_difference, DynEmitter, Emitter, EmitterWriter};
use registry::Registry;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::stable_hasher::{Hash128, StableHasher};
use rustc_data_structures::sync::{Lock, Lrc};
use rustc_data_structures::AtomicRef;
Expand Down Expand Up @@ -1367,7 +1368,7 @@ impl DiagCtxt {

pub fn update_unstable_expectation_id(
&self,
unstable_to_stable: &FxHashMap<LintExpectationId, LintExpectationId>,
unstable_to_stable: &UnordMap<LintExpectationId, LintExpectationId>,
) {
let mut inner = self.inner.borrow_mut();
let diags = std::mem::take(&mut inner.unstable_expect_diagnostics);
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_hir_analysis/src/astconv/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::errors::{
use crate::fluent_generated as fluent;
use crate::traits::error_reporting::report_object_safety_error;
use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
use rustc_data_structures::unord::UnordMap;
use rustc_errors::{pluralize, struct_span_err, Applicability, Diagnostic, ErrorGuaranteed};
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
Expand Down Expand Up @@ -673,7 +674,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}))
})
.flatten()
.collect::<FxHashMap<Symbol, &ty::AssocItem>>();
.collect::<UnordMap<Symbol, &ty::AssocItem>>();

let mut names = names
.into_iter()
Expand Down Expand Up @@ -709,7 +710,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
let mut where_constraints = vec![];
let mut already_has_generics_args_suggestion = false;
for (span, assoc_items) in &associated_types {
let mut names: FxHashMap<_, usize> = FxHashMap::default();
let mut names: UnordMap<_, usize> = Default::default();
for item in assoc_items {
types_count += 1;
*names.entry(item.name).or_insert(0) += 1;
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::potentially_plural_count;
use crate::errors::LifetimesOrBoundsMismatchOnTrait;
use hir::def_id::{DefId, LocalDefId};
use hir::def_id::{DefId, DefIdMap, LocalDefId};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed};
use rustc_hir as hir;
Expand Down Expand Up @@ -478,7 +478,7 @@ fn compare_asyncness<'tcx>(
pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
tcx: TyCtxt<'tcx>,
impl_m_def_id: LocalDefId,
) -> Result<&'tcx FxHashMap<DefId, ty::EarlyBinder<Ty<'tcx>>>, ErrorGuaranteed> {
) -> Result<&'tcx DefIdMap<ty::EarlyBinder<Ty<'tcx>>>, ErrorGuaranteed> {
let impl_m = tcx.opt_associated_item(impl_m_def_id.to_def_id()).unwrap();
let trait_m = tcx.opt_associated_item(impl_m.trait_item_def_id.unwrap()).unwrap();
let impl_trait_ref =
Expand Down Expand Up @@ -706,7 +706,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
);
ocx.resolve_regions_and_report_errors(impl_m_def_id, &outlives_env)?;

let mut remapped_types = FxHashMap::default();
let mut remapped_types = DefIdMap::default();
for (def_id, (ty, args)) in collected_types {
match infcx.fully_resolve((ty, args)) {
Ok((ty, args)) => {
Expand Down
Loading

0 comments on commit e0bf0d3

Please sign in to comment.