Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 7e653fc

Browse files
committed
Auto merge of rust-lang#115747 - Zoxc:query-hashes, r=<try>
Optimize hash map operations in the query system This optimizes hash map operations in the query system by explicitly passing hashes and using more optimal operations. `find_or_find_insert_slot` in particular saves a hash table lookup over `entry`. It's not yet available in a safe API, but will be in rust-lang/hashbrown#466. <table><tr><td rowspan="2">Benchmark</td><td colspan="1"><b>Before</b></th><td colspan="2"><b>After</b></th></tr><tr><td align="right">Time</td><td align="right">Time</td><td align="right">%</th></tr><tr><td>🟣 <b>clap</b>:check</td><td align="right">1.6189s</td><td align="right">1.6129s</td><td align="right"> -0.37%</td></tr><tr><td>🟣 <b>hyper</b>:check</td><td align="right">0.2353s</td><td align="right">0.2337s</td><td align="right"> -0.67%</td></tr><tr><td>🟣 <b>regex</b>:check</td><td align="right">0.9344s</td><td align="right">0.9289s</td><td align="right"> -0.59%</td></tr><tr><td>🟣 <b>syn</b>:check</td><td align="right">1.4693s</td><td align="right">1.4652s</td><td align="right"> -0.28%</td></tr><tr><td>🟣 <b>syntex_syntax</b>:check</td><td align="right">5.6606s</td><td align="right">5.6439s</td><td align="right"> -0.30%</td></tr><tr><td>Total</td><td align="right">9.9185s</td><td align="right">9.8846s</td><td align="right"> -0.34%</td></tr><tr><td>Summary</td><td align="right">1.0000s</td><td align="right">0.9956s</td><td align="right"> -0.44%</td></tr></table> r? `@cjgillot`
2 parents 6ae4cfb + ca6f296 commit 7e653fc

File tree

4 files changed

+44
-24
lines changed

4 files changed

+44
-24
lines changed

compiler/rustc_data_structures/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ extern crate tracing;
4646
#[macro_use]
4747
extern crate rustc_macros;
4848

49+
#[cfg(parallel_compiler)]
50+
extern crate hashbrown;
51+
4952
use std::fmt;
5053

5154
pub use rustc_index::static_assert_size;

compiler/rustc_data_structures/src/marker.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ cfg_match! {
8585
[std::sync::mpsc::Sender<T> where T: DynSend]
8686
[std::sync::Arc<T> where T: ?Sized + DynSync + DynSend]
8787
[std::sync::LazyLock<T, F> where T: DynSend, F: DynSend]
88+
[hashbrown::HashTable<T> where T: DynSend]
8889
[std::collections::HashSet<K, S> where K: DynSend, S: DynSend]
8990
[std::collections::HashMap<K, V, S> where K: DynSend, V: DynSend, S: DynSend]
9091
[std::collections::BTreeMap<K, V, A> where K: DynSend, V: DynSend, A: std::alloc::Allocator + Clone + DynSend]

compiler/rustc_query_system/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ extern crate rustc_data_structures;
1414
#[macro_use]
1515
extern crate rustc_macros;
1616

17+
#[allow(unused_extern_crates)]
18+
extern crate hashbrown;
19+
1720
pub mod cache;
1821
pub mod dep_graph;
1922
mod error;

compiler/rustc_query_system/src/query/plumbing.rs

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,26 +12,30 @@ use crate::query::job::{report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobI
1212
use crate::query::SerializedDepNodeIndex;
1313
use crate::query::{QueryContext, QueryMap, QuerySideEffects, QueryStackFrame};
1414
use crate::HandleCycleError;
15+
use hashbrown::hash_table::Entry;
1516
use rustc_data_structures::fingerprint::Fingerprint;
16-
use rustc_data_structures::fx::FxHashMap;
17-
use rustc_data_structures::sharded::Sharded;
17+
use rustc_data_structures::sharded::{self, Sharded};
1818
use rustc_data_structures::stack::ensure_sufficient_stack;
1919
use rustc_data_structures::sync::Lock;
2020
#[cfg(parallel_compiler)]
2121
use rustc_data_structures::{outline, sync};
2222
use rustc_errors::{DiagnosticBuilder, FatalError, StashKey};
2323
use rustc_span::{Span, DUMMY_SP};
2424
use std::cell::Cell;
25-
use std::collections::hash_map::Entry;
2625
use std::fmt::Debug;
2726
use std::hash::Hash;
2827
use std::mem;
2928
use thin_vec::ThinVec;
3029

3130
use super::QueryConfig;
3231

32+
#[inline]
33+
fn equivalent_key<K: Eq, V>(k: &K) -> impl Fn(&(K, V)) -> bool + '_ {
34+
move |x| x.0 == *k
35+
}
36+
3337
pub struct QueryState<K> {
34-
active: Sharded<FxHashMap<K, QueryResult>>,
38+
active: Sharded<hashbrown::HashTable<(K, QueryResult)>>,
3539
}
3640

3741
/// Indicates the state of a query for a given key in a query map.
@@ -165,7 +169,7 @@ where
165169
{
166170
/// Completes the query by updating the query cache with the `result`,
167171
/// signals the waiter and forgets the JobOwner, so it won't poison the query
168-
fn complete<C>(self, cache: &C, result: C::Value, dep_node_index: DepNodeIndex)
172+
fn complete<C>(self, cache: &C, key_hash: u64, result: C::Value, dep_node_index: DepNodeIndex)
169173
where
170174
C: QueryCache<Key = K>,
171175
{
@@ -180,8 +184,11 @@ where
180184
cache.complete(key, result, dep_node_index);
181185

182186
let job = {
183-
let mut lock = state.active.lock_shard_by_value(&key);
184-
lock.remove(&key).unwrap().expect_job()
187+
let mut shard = state.active.lock_shard_by_hash(key_hash);
188+
match shard.find_entry(key_hash, equivalent_key(&key)) {
189+
Err(_) => panic!(),
190+
Ok(occupied) => occupied.remove().0.1.expect_job(),
191+
}
185192
};
186193

187194
job.signal_complete();
@@ -198,11 +205,16 @@ where
198205
// Poison the query so jobs waiting on it panic.
199206
let state = self.state;
200207
let job = {
201-
let mut shard = state.active.lock_shard_by_value(&self.key);
202-
let job = shard.remove(&self.key).unwrap().expect_job();
203-
204-
shard.insert(self.key, QueryResult::Poisoned);
205-
job
208+
let key_hash = sharded::make_hash(&self.key);
209+
let mut shard = state.active.lock_shard_by_hash(key_hash);
210+
match shard.find_entry(key_hash, equivalent_key(&self.key)) {
211+
Err(_) => panic!(),
212+
Ok(occupied) => {
213+
let ((key, value), vacant) = occupied.remove();
214+
vacant.insert((key, QueryResult::Poisoned));
215+
value.expect_job()
216+
}
217+
}
206218
};
207219
// Also signal the completion of the job, so waiters
208220
// will continue execution.
@@ -283,12 +295,11 @@ where
283295
outline(|| {
284296
// We didn't find the query result in the query cache. Check if it was
285297
// poisoned due to a panic instead.
286-
let lock = query.query_state(qcx).active.get_shard_by_value(&key).lock();
287-
288-
match lock.get(&key) {
289-
Some(QueryResult::Poisoned) => {
290-
panic!("query '{}' not cached due to poisoning", query.name())
291-
}
298+
let key_hash = sharded::make_hash(&key);
299+
let shard = query.query_state(qcx).active.lock_shard_by_hash(key_hash);
300+
match shard.find(key_hash, equivalent_key(&key)) {
301+
// The query we waited on panicked. Continue unwinding here.
302+
Some((_, QueryResult::Poisoned)) => FatalError.raise(),
292303
_ => panic!(
293304
"query '{}' result must be in the cache or the query must be poisoned after a wait",
294305
query.name()
@@ -319,7 +330,8 @@ where
319330
Qcx: QueryContext,
320331
{
321332
let state = query.query_state(qcx);
322-
let mut state_lock = state.active.lock_shard_by_value(&key);
333+
let key_hash = sharded::make_hash(&key);
334+
let mut state_lock = state.active.lock_shard_by_hash(key_hash);
323335

324336
// For the parallel compiler we need to check both the query cache and query state structures
325337
// while holding the state lock to ensure that 1) the query has not yet completed and 2) the
@@ -336,21 +348,21 @@ where
336348

337349
let current_job_id = qcx.current_query_job();
338350

339-
match state_lock.entry(key) {
351+
match state_lock.entry(key_hash, equivalent_key(&key), |(k, _)| sharded::make_hash(k)) {
340352
Entry::Vacant(entry) => {
341353
// Nothing has computed or is computing the query, so we start a new job and insert it in the
342354
// state map.
343355
let id = qcx.next_job_id();
344356
let job = QueryJob::new(id, span, current_job_id);
345-
entry.insert(QueryResult::Started(job));
357+
entry.insert((key, QueryResult::Started(job)));
346358

347359
// Drop the lock before we start executing the query
348360
drop(state_lock);
349361

350-
execute_job::<_, _, INCR>(query, qcx, state, key, id, dep_node)
362+
execute_job::<_, _, INCR>(query, qcx, state, key, key_hash, id, dep_node)
351363
}
352364
Entry::Occupied(mut entry) => {
353-
match entry.get_mut() {
365+
match &mut entry.get_mut().1 {
354366
QueryResult::Started(job) => {
355367
#[cfg(parallel_compiler)]
356368
if sync::is_dyn_thread_safe() {
@@ -382,6 +394,7 @@ fn execute_job<Q, Qcx, const INCR: bool>(
382394
qcx: Qcx,
383395
state: &QueryState<Q::Key>,
384396
key: Q::Key,
397+
key_hash: u64,
385398
id: QueryJobId,
386399
dep_node: Option<DepNode>,
387400
) -> (Q::Value, Option<DepNodeIndex>)
@@ -442,7 +455,7 @@ where
442455
}
443456
}
444457
}
445-
job_owner.complete(cache, result, dep_node_index);
458+
job_owner.complete(cache, key_hash, result, dep_node_index);
446459

447460
(result, Some(dep_node_index))
448461
}

0 commit comments

Comments
 (0)