Skip to content

Replace a number of FxHashMaps/Sets with stable-iteration-order alternatives #119192

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.

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

Merged
merged 14 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
3 changes: 2 additions & 1 deletion compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::unord::UnordMap;
use rustc_errors::{Applicability, DiagnosticBuilder, ErrorGuaranteed, StashKey};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
Expand Down Expand Up @@ -979,7 +980,7 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {
})
// Check for duplicates
.and_then(|list| {
let mut set: FxHashMap<Symbol, Span> = FxHashMap::default();
let mut set: UnordMap<Symbol, Span> = Default::default();
let mut no_dups = true;

for ident in &*list {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
for ((span, add_where_or_comma), obligations) in type_params.into_iter() {
restrict_type_params = true;
// #74886: Sort here so that the output is always the same.
let obligations = obligations.to_sorted_stable_ord();
let obligations = obligations.into_sorted_stable_ord();
err.span_suggestion_verbose(
span,
format!(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
drop_order: bool,
) -> MigrationWarningReason {
MigrationWarningReason {
auto_traits: auto_trait_reasons.to_sorted_stable_ord(),
auto_traits: auto_trait_reasons.into_sorted_stable_ord(),
drop_order,
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
assert_eq!(fcx_typeck_results.hir_owner, self.typeck_results.hir_owner);

let fcx_coercion_casts = fcx_typeck_results.coercion_casts().to_sorted_stable_ord();
for local_id in fcx_coercion_casts {
for &local_id in fcx_coercion_casts {
self.typeck_results.set_coercion_cast(local_id);
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/src/persist/save.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub fn save_work_product_index(
// deleted during invalidation. Some object files don't change their
// content, they are just not needed anymore.
let previous_work_products = dep_graph.previous_work_products();
for (id, wp) in previous_work_products.to_sorted_stable_ord().iter() {
for (id, wp) in previous_work_products.to_sorted_stable_ord() {
if !new_work_products.contains_key(id) {
work_product::delete_workproduct_files(sess, wp);
debug_assert!(
Expand Down
Loading