Skip to content

Move iter_results to dyn FnMut rather than a generic #84719

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
May 1, 2021
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
35 changes: 21 additions & 14 deletions compiler/rustc_middle/src/ty/query/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1185,20 +1185,27 @@ where

assert!(Q::query_state(tcx).all_inactive());
let cache = Q::query_cache(tcx);
cache.iter_results(|results| {
for (key, value, dep_node) in results {
if Q::cache_on_disk(tcx, &key, Some(value)) {
let dep_node = SerializedDepNodeIndex::new(dep_node.index());

// Record position of the cache entry.
query_result_index
.push((dep_node, AbsoluteBytePos::new(encoder.encoder.position())));

// Encode the type check tables with the `SerializedDepNodeIndex`
// as tag.
encoder.encode_tagged(dep_node, value)?;
let mut res = Ok(());
cache.iter_results(&mut |key, value, dep_node| {
if res.is_err() {
return;
}
if Q::cache_on_disk(tcx, &key, Some(value)) {
let dep_node = SerializedDepNodeIndex::new(dep_node.index());

// Record position of the cache entry.
query_result_index.push((dep_node, AbsoluteBytePos::new(encoder.encoder.position())));

// Encode the type check tables with the `SerializedDepNodeIndex`
// as tag.
match encoder.encode_tagged(dep_node, value) {
Ok(()) => {}
Err(e) => {
res = Err(e);
}
}
}
Ok(())
})
});

res
Comment on lines +1188 to +1210
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use a short circuit version for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what exactly you mean, but we don't really care about performance on the error path - the goal here is to minimize monomoprhizations and logic in general.

}
19 changes: 10 additions & 9 deletions compiler/rustc_query_impl/src/profiling_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ fn alloc_self_profile_query_strings_for_query_cache<'tcx, C>(
// need to invoke queries itself, we cannot keep the query caches
// locked while doing so. Instead we copy out the
// `(query_key, dep_node_index)` pairs and release the lock again.
let query_keys_and_indices: Vec<_> = query_cache
.iter_results(|results| results.map(|(k, _, i)| (k.clone(), i)).collect());
let mut query_keys_and_indices = Vec::new();
query_cache.iter_results(&mut |k, _, i| query_keys_and_indices.push((k.clone(), i)));

// Now actually allocate the strings. If allocating the strings
// generates new entries in the query cache, we'll miss them but
Expand All @@ -275,14 +275,15 @@ fn alloc_self_profile_query_strings_for_query_cache<'tcx, C>(
let query_name = profiler.get_or_alloc_cached_string(query_name);
let event_id = event_id_builder.from_label(query_name).to_string_id();

query_cache.iter_results(|results| {
let query_invocation_ids: Vec<_> = results.map(|v| v.2.into()).collect();

profiler.bulk_map_query_invocation_id_to_single_string(
query_invocation_ids.into_iter(),
event_id,
);
let mut query_invocation_ids = Vec::new();
query_cache.iter_results(&mut |_, _, i| {
query_invocation_ids.push(i.into());
});

profiler.bulk_map_query_invocation_id_to_single_string(
query_invocation_ids.into_iter(),
event_id,
);
}
});
}
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_query_impl/src/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,12 @@ where
key_type: type_name::<C::Key>(),
value_size: mem::size_of::<C::Value>(),
value_type: type_name::<C::Value>(),
entry_count: map.iter_results(|results| results.count()),
entry_count: 0,
local_def_id_keys: None,
};
map.iter_results(|results| {
for (key, _, _) in results {
key.key_stats(&mut stats)
}
map.iter_results(&mut |key, _, _| {
stats.entry_count += 1;
key.key_stats(&mut stats)
});
stats
}
Expand Down
34 changes: 19 additions & 15 deletions compiler/rustc_query_system/src/query/caches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,11 @@ pub trait QueryCache: QueryStorage {
index: DepNodeIndex,
) -> Self::Stored;

fn iter<R>(
fn iter(
&self,
shards: &Sharded<Self::Sharded>,
f: impl for<'a> FnOnce(
&'a mut dyn Iterator<Item = (&'a Self::Key, &'a Self::Value, DepNodeIndex)>,
) -> R,
) -> R;
f: &mut dyn FnMut(&Self::Key, &Self::Value, DepNodeIndex),
);
}

pub struct DefaultCacheSelector;
Expand Down Expand Up @@ -124,14 +122,17 @@ where
value
}

fn iter<R>(
fn iter(
&self,
shards: &Sharded<Self::Sharded>,
f: impl for<'a> FnOnce(&'a mut dyn Iterator<Item = (&'a K, &'a V, DepNodeIndex)>) -> R,
) -> R {
f: &mut dyn FnMut(&Self::Key, &Self::Value, DepNodeIndex),
) {
let shards = shards.lock_shards();
let mut results = shards.iter().flat_map(|shard| shard.iter()).map(|(k, v)| (k, &v.0, v.1));
f(&mut results)
for shard in shards.iter() {
for (k, v) in shard.iter() {
f(k, &v.0, v.1);
}
}
}
}

Expand Down Expand Up @@ -207,13 +208,16 @@ where
&value.0
}

fn iter<R>(
fn iter(
&self,
shards: &Sharded<Self::Sharded>,
f: impl for<'a> FnOnce(&'a mut dyn Iterator<Item = (&'a K, &'a V, DepNodeIndex)>) -> R,
) -> R {
f: &mut dyn FnMut(&Self::Key, &Self::Value, DepNodeIndex),
) {
let shards = shards.lock_shards();
let mut results = shards.iter().flat_map(|shard| shard.iter()).map(|(k, v)| (k, &v.0, v.1));
f(&mut results)
for shard in shards.iter() {
for (k, v) in shard.iter() {
f(k, &v.0, v.1);
}
}
}
}
7 changes: 1 addition & 6 deletions compiler/rustc_query_system/src/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,7 @@ impl<C: QueryCache> QueryCacheStore<C> {
(QueryLookup { key_hash, shard }, lock)
}

pub fn iter_results<R>(
&self,
f: impl for<'a> FnOnce(
&'a mut dyn Iterator<Item = (&'a C::Key, &'a C::Value, DepNodeIndex)>,
) -> R,
) -> R {
pub fn iter_results(&self, f: &mut dyn FnMut(&C::Key, &C::Value, DepNodeIndex)) {
self.cache.iter(&self.shards, f)
}
}
Expand Down