diff options
| author | Dylan DPC <99973273+Dylan-DPC@users.noreply.github.com> | 2023-02-24 12:02:41 +0530 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2023-02-24 12:02:41 +0530 |
| commit | 440113ddf68522a3cecac66dc3cd0bdf88718013 (patch) | |
| tree | c0a29eba6d67427fc8c7a0b118ee3d1a32763ad6 /compiler/rustc_query_system | |
| parent | 6826a9606738e99c1b5e818061ea622492636baa (diff) | |
| parent | 056c5b3b57256e03ac0f439f699937d2ae9b208a (diff) | |
| download | rust-440113ddf68522a3cecac66dc3cd0bdf88718013.tar.gz rust-440113ddf68522a3cecac66dc3cd0bdf88718013.zip | |
Rollup merge of #108169 - Zoxc:query-key-copy, r=cjgillot
Make query keys `Copy` This regressed compiler performance locally, so I'm curious what perf will say about it. <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.7566s</td><td align="right">1.7657s</td><td align="right"> 0.52%</td></tr><tr><td>🟣 <b>hyper</b>:check</td><td align="right">0.2572s</td><td align="right">0.2578s</td><td align="right"> 0.20%</td></tr><tr><td>🟣 <b>regex</b>:check</td><td align="right">0.9863s</td><td align="right">0.9900s</td><td align="right"> 0.37%</td></tr><tr><td>🟣 <b>syn</b>:check</td><td align="right">1.6018s</td><td align="right">1.6073s</td><td align="right"> 0.34%</td></tr><tr><td>🟣 <b>syntex_syntax</b>:check</td><td align="right">6.2493s</td><td align="right">6.2920s</td><td align="right"> 0.68%</td></tr><tr><td>Total</td><td align="right">10.8512s</td><td align="right">10.9127s</td><td align="right"> 0.57%</td></tr><tr><td>Summary</td><td align="right">1.0000s</td><td align="right">1.0042s</td><td align="right"> 0.42%</td></tr></table>
Diffstat (limited to 'compiler/rustc_query_system')
| -rw-r--r-- | compiler/rustc_query_system/src/query/caches.rs | 6 | ||||
| -rw-r--r-- | compiler/rustc_query_system/src/query/config.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_query_system/src/query/plumbing.rs | 25 |
3 files changed, 18 insertions, 17 deletions
diff --git a/compiler/rustc_query_system/src/query/caches.rs b/compiler/rustc_query_system/src/query/caches.rs index e840108bdd8..4b3cd16c29f 100644 --- a/compiler/rustc_query_system/src/query/caches.rs +++ b/compiler/rustc_query_system/src/query/caches.rs @@ -21,7 +21,7 @@ pub trait QueryStorage { } pub trait QueryCache: QueryStorage + Sized { - type Key: Hash + Eq + Clone + Debug; + type Key: Hash + Eq + Copy + Debug; /// Checks if the query is already computed and in the cache. /// It returns the shard index and a lock guard to the shard, @@ -61,7 +61,7 @@ impl<K: Eq + Hash, V: Copy + Debug> QueryStorage for DefaultCache<K, V> { impl<K, V> QueryCache for DefaultCache<K, V> where - K: Eq + Hash + Clone + Debug, + K: Eq + Hash + Copy + Debug, V: Copy + Debug, { type Key = K; @@ -179,7 +179,7 @@ impl<K: Eq + Idx, V: Copy + Debug> QueryStorage for VecCache<K, V> { impl<K, V> QueryCache for VecCache<K, V> where - K: Eq + Idx + Clone + Debug, + K: Eq + Idx + Copy + Debug, V: Copy + Debug, { type Key = K; diff --git a/compiler/rustc_query_system/src/query/config.rs b/compiler/rustc_query_system/src/query/config.rs index 56247e827a2..d5637387346 100644 --- a/compiler/rustc_query_system/src/query/config.rs +++ b/compiler/rustc_query_system/src/query/config.rs @@ -19,7 +19,9 @@ pub type TryLoadFromDisk<Qcx, Q> = pub trait QueryConfig<Qcx: QueryContext> { const NAME: &'static str; - type Key: DepNodeParams<Qcx::DepContext> + Eq + Hash + Clone + Debug; + // `Key` and `Value` are `Copy` instead of `Clone` to ensure copying them stays cheap, + // but it isn't necessary. + type Key: DepNodeParams<Qcx::DepContext> + Eq + Hash + Copy + Debug; type Value: Debug + Copy; type Cache: QueryCache<Key = Self::Key, Value = Self::Value>; diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 21a0c73d720..5499165930d 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -48,7 +48,7 @@ enum QueryResult<D: DepKind> { impl<K, D> QueryState<K, D> where - K: Eq + Hash + Clone + Debug, + K: Eq + Hash + Copy + Debug, D: DepKind, { pub fn all_inactive(&self) -> bool { @@ -77,7 +77,7 @@ where for shard in shards.iter() { for (k, v) in shard.iter() { if let QueryResult::Started(ref job) = *v { - let query = make_query(qcx, k.clone()); + let query = make_query(qcx, *k); jobs.insert(job.id, QueryJobInfo { query, job: job.clone() }); } } @@ -91,7 +91,7 @@ where // really hurt much.) for (k, v) in self.active.try_lock()?.iter() { if let QueryResult::Started(ref job) = *v { - let query = make_query(qcx, k.clone()); + let query = make_query(qcx, *k); jobs.insert(job.id, QueryJobInfo { query, job: job.clone() }); } } @@ -111,7 +111,7 @@ impl<K, D: DepKind> Default for QueryState<K, D> { /// This will poison the relevant query if dropped. struct JobOwner<'tcx, K, D: DepKind> where - K: Eq + Hash + Clone, + K: Eq + Hash + Copy, { state: &'tcx QueryState<K, D>, key: K, @@ -163,7 +163,7 @@ where impl<'tcx, K, D: DepKind> JobOwner<'tcx, K, D> where - K: Eq + Hash + Clone, + K: Eq + Hash + Copy, { /// Either gets a `JobOwner` corresponding the query, allowing us to /// start executing the query, or returns with the result of the query. @@ -195,7 +195,7 @@ where let job = qcx.current_query_job(); let job = QueryJob::new(id, span, job); - let key = entry.key().clone(); + let key = *entry.key(); entry.insert(QueryResult::Started(job)); let owner = JobOwner { state, id, key }; @@ -274,7 +274,7 @@ where impl<'tcx, K, D> Drop for JobOwner<'tcx, K, D> where - K: Eq + Hash + Clone, + K: Eq + Hash + Copy, D: DepKind, { #[inline(never)] @@ -291,7 +291,7 @@ where QueryResult::Started(job) => job, QueryResult::Poisoned => panic!(), }; - shard.insert(self.key.clone(), QueryResult::Poisoned); + shard.insert(self.key, QueryResult::Poisoned); job }; // Also signal the completion of the job, so waiters @@ -310,7 +310,7 @@ pub(crate) struct CycleError<D: DepKind> { /// The result of `try_start`. enum TryGetJob<'tcx, K, D> where - K: Eq + Hash + Clone, + K: Eq + Hash + Copy, D: DepKind, { /// The query is not yet started. Contains a guard to the cache eventually used to start it. @@ -358,10 +358,9 @@ where Q: QueryConfig<Qcx>, Qcx: QueryContext, { - match JobOwner::<'_, Q::Key, Qcx::DepKind>::try_start(&qcx, state, span, key.clone()) { + match JobOwner::<'_, Q::Key, Qcx::DepKind>::try_start(&qcx, state, span, key) { TryGetJob::NotYetStarted(job) => { - let (result, dep_node_index) = - execute_job::<Q, Qcx>(qcx, key.clone(), dep_node, job.id); + let (result, dep_node_index) = execute_job::<Q, Qcx>(qcx, key, dep_node, job.id); if Q::FEEDABLE { // We should not compute queries that also got a value via feeding. // This can't happen, as query feeding adds the very dependencies to the fed query @@ -551,7 +550,7 @@ where let prof_timer = qcx.dep_context().profiler().query_provider(); // The dep-graph for this computation is already in-place. - let result = dep_graph.with_ignore(|| Q::compute(qcx, key.clone())); + let result = dep_graph.with_ignore(|| Q::compute(qcx, *key)); prof_timer.finish_with_query_invocation_id(dep_node_index.into()); |
