Skip to content

Make query keys Copy #108169

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 1 commit into from
Feb 24, 2023
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
6 changes: 3 additions & 3 deletions compiler/rustc_query_system/src/query/caches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_query_system/src/query/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
25 changes: 12 additions & 13 deletions compiler/rustc_query_system/src/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,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 {
Expand Down Expand Up @@ -78,7 +78,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() });
}
}
Expand All @@ -92,7 +92,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() });
}
}
Expand All @@ -112,7 +112,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,
Expand Down Expand Up @@ -164,7 +164,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.
Expand Down Expand Up @@ -196,7 +196,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 };
Expand Down Expand Up @@ -275,7 +275,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)]
Expand All @@ -292,7 +292,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
Expand All @@ -311,7 +311,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.
Expand Down Expand Up @@ -359,10 +359,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 may have put a value inside the cache from inside the execution.
// Verify that it has the same hash as what we have now, to ensure consistency.
Expand Down Expand Up @@ -547,7 +546,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());

Expand Down