Skip to content

Commit

Permalink
Auto merge of #122064 - Zoxc:dep-graph-encode-tweaks, r=cjgillot
Browse files Browse the repository at this point in the history 
Dep node encoding cleanups

This does some cleanups around dep node encoding.

Performance change with `-Zthreads=2`:

<table><tr><td rowspan="2">Benchmark</td><td colspan="1"><b>Before</b></th><td colspan="2"><b>After</b></th><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><td align="right">Memory</td><td align="right">Memory</td><td align="right">%</th></tr><tr><td>🟣 <b>clap</b>:check:unchanged</td><td align="right">0.4337s</td><td align="right">0.4306s</td><td align="right"> -0.72%</td><td align="right">88.90 MiB</td><td align="right">89.04 MiB</td><td align="right"> 0.15%</td></tr><tr><td>🟣 <b>hyper</b>:check:unchanged</td><td align="right">0.1541s</td><td align="right">0.1528s</td><td align="right"> -0.86%</td><td align="right">51.99 MiB</td><td align="right">52.03 MiB</td><td align="right"> 0.07%</td></tr><tr><td>🟣 <b>regex</b>:check:unchanged</td><td align="right">0.3286s</td><td align="right">0.3248s</td><td align="right">💚  -1.15%</td><td align="right">71.89 MiB</td><td align="right">71.74 MiB</td><td align="right"> -0.21%</td></tr><tr><td>🟣 <b>syn</b>:check:unchanged</td><td align="right">0.6118s</td><td align="right">0.6057s</td><td align="right">💚  -1.01%</td><td align="right">106.59 MiB</td><td align="right">106.66 MiB</td><td align="right"> 0.06%</td></tr><tr><td>🟣 <b>syntex_syntax</b>:check:unchanged</td><td align="right">1.4570s</td><td align="right">1.4463s</td><td align="right"> -0.74%</td><td align="right">197.29 MiB</td><td align="right">197.33 MiB</td><td align="right"> 0.02%</td></tr><tr><td>Total</td><td align="right">2.9852s</td><td align="right">2.9601s</td><td align="right"> -0.84%</td><td align="right">516.66 MiB</td><td align="right">516.80 MiB</td><td align="right"> 0.03%</td></tr><tr><td>Summary</td><td align="right">1.0000s</td><td align="right">0.9911s</td><td align="right"> -0.89%</td><td align="right">1 byte</td><td align="right">1.00 bytes</td><td align="right"> 0.02%</td></tr></table>

r? `@cjgillot`
  • Loading branch information
@bors
bors committed Mar 10, 2024
2 parents 5bc7b9a + 9707e10 commit 094a620
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 75 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ impl<'tcx> GlobalCtxt<'tcx> {
}

pub fn finish(&self) -> FileEncodeResult {
self.dep_graph.finish_encoding(&self.sess.prof)
self.dep_graph.finish_encoding()
}
}

Expand Down
84 changes: 20 additions & 64 deletions compiler/rustc_query_system/src/dep_graph/graph.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::profiling::{EventId, QueryInvocationId, SelfProfilerRef};
use rustc_data_structures::profiling::{QueryInvocationId, SelfProfilerRef};
use rustc_data_structures::sharded::{self, Sharded};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::steal::Steal;
use rustc_data_structures::sync::{AtomicU32, AtomicU64, Lock, Lrc};
use rustc_data_structures::unord::UnordMap;
use rustc_index::IndexVec;
Expand Down Expand Up @@ -134,7 +133,6 @@ impl<D: Deps> DepGraph<D> {

// Instantiate a dependy-less node only once for anonymous queries.
let _green_node_index = current.intern_new_node(
profiler,
DepNode { kind: D::DEP_KIND_NULL, hash: current.anon_id_seed.into() },
EdgesVec::new(),
Fingerprint::ZERO,
Expand All @@ -143,7 +141,6 @@ impl<D: Deps> DepGraph<D> {

// Instantiate a dependy-less red node only once for anonymous queries.
let (red_node_index, red_node_prev_index_and_color) = current.intern_node(
profiler,
&prev_graph,
DepNode { kind: D::DEP_KIND_RED, hash: Fingerprint::ZERO.into() },
EdgesVec::new(),
Expand Down Expand Up @@ -196,7 +193,7 @@ impl<D: Deps> DepGraph<D> {

pub fn with_query(&self, f: impl Fn(&DepGraphQuery)) {
if let Some(data) = &self.data {
data.current.encoder.borrow().with_query(f)
data.current.encoder.with_query(f)
}
}

Expand Down Expand Up @@ -372,13 +369,8 @@ impl<D: Deps> DepGraphData<D> {
hash_result.map(|f| dcx.with_stable_hashing_context(|mut hcx| f(&mut hcx, &result)));

// Intern the new `DepNode`.
let (dep_node_index, prev_and_color) = self.current.intern_node(
dcx.profiler(),
&self.previous,
key,
edges,
current_fingerprint,
);
let (dep_node_index, prev_and_color) =
self.current.intern_node(&self.previous, key, edges, current_fingerprint);

hashing_timer.finish_with_query_invocation_id(dep_node_index.into());

Expand Down Expand Up @@ -443,12 +435,7 @@ impl<D: Deps> DepGraphData<D> {
hash: self.current.anon_id_seed.combine(hasher.finish()).into(),
};

self.current.intern_new_node(
cx.profiler(),
target_dep_node,
task_deps,
Fingerprint::ZERO,
)
self.current.intern_new_node(target_dep_node, task_deps, Fingerprint::ZERO)
}
};

Expand Down Expand Up @@ -585,13 +572,8 @@ impl<D: Deps> DepGraph<D> {
});

// Intern the new `DepNode` with the dependencies up-to-now.
let (dep_node_index, prev_and_color) = data.current.intern_node(
cx.profiler(),
&data.previous,
node,
edges,
current_fingerprint,
);
let (dep_node_index, prev_and_color) =
data.current.intern_node(&data.previous, node, edges, current_fingerprint);

hashing_timer.finish_with_query_invocation_id(dep_node_index.into());

Expand Down Expand Up @@ -871,11 +853,8 @@ impl<D: Deps> DepGraphData<D> {

// We allocating an entry for the node in the current dependency graph and
// adding all the appropriate edges imported from the previous graph
let dep_node_index = self.current.promote_node_and_deps_to_current(
qcx.dep_context().profiler(),
&self.previous,
prev_dep_node_index,
);
let dep_node_index =
self.current.promote_node_and_deps_to_current(&self.previous, prev_dep_node_index);

// ... emitting any stored diagnostic ...

Expand Down Expand Up @@ -974,19 +953,15 @@ impl<D: Deps> DepGraph<D> {

pub fn print_incremental_info(&self) {
if let Some(data) = &self.data {
data.current.encoder.borrow().print_incremental_info(
data.current.encoder.print_incremental_info(
data.current.total_read_count.load(Ordering::Relaxed),
data.current.total_duplicate_read_count.load(Ordering::Relaxed),
)
}
}

pub fn finish_encoding(&self, profiler: &SelfProfilerRef) -> FileEncodeResult {
if let Some(data) = &self.data {
data.current.encoder.steal().finish(profiler)
} else {
Ok(0)
}
pub fn finish_encoding(&self) -> FileEncodeResult {
if let Some(data) = &self.data { data.current.encoder.finish() } else { Ok(0) }
}

pub(crate) fn next_virtual_depnode_index(&self) -> DepNodeIndex {
Expand Down Expand Up @@ -1069,7 +1044,7 @@ rustc_index::newtype_index! {
/// manipulating both, we acquire `new_node_to_index` or `prev_index_to_index`
/// first, and `data` second.
pub(super) struct CurrentDepGraph<D: Deps> {
encoder: Steal<GraphEncoder<D>>,
encoder: GraphEncoder<D>,
new_node_to_index: Sharded<FxHashMap<DepNode, DepNodeIndex>>,
prev_index_to_index: Lock<IndexVec<SerializedDepNodeIndex, Option<DepNodeIndex>>>,

Expand Down Expand Up @@ -1100,12 +1075,6 @@ pub(super) struct CurrentDepGraph<D: Deps> {
/// debugging and only active with `debug_assertions`.
total_read_count: AtomicU64,
total_duplicate_read_count: AtomicU64,

/// The cached event id for profiling node interning. This saves us
/// from having to look up the event id every time we intern a node
/// which may incur too much overhead.
/// This will be None if self-profiling is disabled.
node_intern_event_id: Option<EventId>,
}

impl<D: Deps> CurrentDepGraph<D> {
Expand Down Expand Up @@ -1140,17 +1109,14 @@ impl<D: Deps> CurrentDepGraph<D> {

let new_node_count_estimate = 102 * prev_graph_node_count / 100 + 200;

let node_intern_event_id = profiler
.get_or_alloc_cached_string("incr_comp_intern_dep_graph_node")
.map(EventId::from_label);

CurrentDepGraph {
encoder: Steal::new(GraphEncoder::new(
encoder: GraphEncoder::new(
encoder,
prev_graph_node_count,
record_graph,
record_stats,
)),
profiler,
),
new_node_to_index: Sharded::new(|| {
FxHashMap::with_capacity_and_hasher(
new_node_count_estimate / sharded::shards(),
Expand All @@ -1165,7 +1131,6 @@ impl<D: Deps> CurrentDepGraph<D> {
fingerprints: Lock::new(IndexVec::from_elem_n(None, new_node_count_estimate)),
total_read_count: AtomicU64::new(0),
total_duplicate_read_count: AtomicU64::new(0),
node_intern_event_id,
}
}

Expand All @@ -1183,16 +1148,14 @@ impl<D: Deps> CurrentDepGraph<D> {
#[inline(always)]
fn intern_new_node(
&self,
profiler: &SelfProfilerRef,
key: DepNode,
edges: EdgesVec,
current_fingerprint: Fingerprint,
) -> DepNodeIndex {
let dep_node_index = match self.new_node_to_index.lock_shard_by_value(&key).entry(key) {
Entry::Occupied(entry) => *entry.get(),
Entry::Vacant(entry) => {
let dep_node_index =
self.encoder.borrow().send(profiler, key, current_fingerprint, edges);
let dep_node_index = self.encoder.send(key, current_fingerprint, edges);
entry.insert(dep_node_index);
dep_node_index
}
Expand All @@ -1206,25 +1169,19 @@ impl<D: Deps> CurrentDepGraph<D> {

fn intern_node(
&self,
profiler: &SelfProfilerRef,
prev_graph: &SerializedDepGraph,
key: DepNode,
edges: EdgesVec,
fingerprint: Option<Fingerprint>,
) -> (DepNodeIndex, Option<(SerializedDepNodeIndex, DepNodeColor)>) {
// Get timer for profiling `DepNode` interning
let _node_intern_timer =
self.node_intern_event_id.map(|eid| profiler.generic_activity_with_event_id(eid));

if let Some(prev_index) = prev_graph.node_to_index_opt(&key) {
let get_dep_node_index = |fingerprint| {
let mut prev_index_to_index = self.prev_index_to_index.lock();

let dep_node_index = match prev_index_to_index[prev_index] {
Some(dep_node_index) => dep_node_index,
None => {
let dep_node_index =
self.encoder.borrow().send(profiler, key, fingerprint, edges);
let dep_node_index = self.encoder.send(key, fingerprint, edges);
prev_index_to_index[prev_index] = Some(dep_node_index);
dep_node_index
}
Expand Down Expand Up @@ -1261,15 +1218,14 @@ impl<D: Deps> CurrentDepGraph<D> {
let fingerprint = fingerprint.unwrap_or(Fingerprint::ZERO);

// This is a new node: it didn't exist in the previous compilation session.
let dep_node_index = self.intern_new_node(profiler, key, edges, fingerprint);
let dep_node_index = self.intern_new_node(key, edges, fingerprint);

(dep_node_index, None)
}
}

fn promote_node_and_deps_to_current(
&self,
profiler: &SelfProfilerRef,
prev_graph: &SerializedDepGraph,
prev_index: SerializedDepNodeIndex,
) -> DepNodeIndex {
Expand All @@ -1286,7 +1242,7 @@ impl<D: Deps> CurrentDepGraph<D> {
.map(|i| prev_index_to_index[i].unwrap())
.collect();
let fingerprint = prev_graph.fingerprint_by_index(prev_index);
let dep_node_index = self.encoder.borrow().send(profiler, key, fingerprint, edges);
let dep_node_index = self.encoder.send(key, fingerprint, edges);
prev_index_to_index[prev_index] = Some(dep_node_index);
#[cfg(debug_assertions)]
self.record_edge(dep_node_index, key, fingerprint);
Expand Down
23 changes: 13 additions & 10 deletions compiler/rustc_query_system/src/dep_graph/serialized.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,8 @@ impl<D: Deps> EncoderState<D> {
}

pub struct GraphEncoder<D: Deps> {
status: Lock<EncoderState<D>>,
profiler: SelfProfilerRef,
status: Lock<Option<EncoderState<D>>>,
record_graph: Option<Lock<DepGraphQuery>>,
}

Expand All @@ -514,10 +515,11 @@ impl<D: Deps> GraphEncoder<D> {
prev_node_count: usize,
record_graph: bool,
record_stats: bool,
profiler: &SelfProfilerRef,
) -> Self {
let record_graph = record_graph.then(|| Lock::new(DepGraphQuery::new(prev_node_count)));
let status = Lock::new(EncoderState::new(encoder, record_stats));
GraphEncoder { status, record_graph }
let status = Lock::new(Some(EncoderState::new(encoder, record_stats)));
GraphEncoder { status, record_graph, profiler: profiler.clone() }
}

pub(crate) fn with_query(&self, f: impl Fn(&DepGraphQuery)) {
Expand All @@ -531,7 +533,8 @@ impl<D: Deps> GraphEncoder<D> {
total_read_count: u64,
total_duplicate_read_count: u64,
) {
let status = self.status.lock();
let mut status = self.status.lock();
let status = status.as_mut().unwrap();
if let Some(record_stats) = &status.stats {
let mut stats: Vec<_> = record_stats.values().collect();
stats.sort_by_key(|s| -(s.node_counter as i64));
Expand Down Expand Up @@ -580,18 +583,18 @@ impl<D: Deps> GraphEncoder<D> {

pub(crate) fn send(
&self,
profiler: &SelfProfilerRef,
node: DepNode,
fingerprint: Fingerprint,
edges: EdgesVec,
) -> DepNodeIndex {
let _prof_timer = profiler.generic_activity("incr_comp_encode_dep_graph");
let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph");
let node = NodeInfo { node, fingerprint, edges };
self.status.lock().encode_node(&node, &self.record_graph)
self.status.lock().as_mut().unwrap().encode_node(&node, &self.record_graph)
}

pub fn finish(self, profiler: &SelfProfilerRef) -> FileEncodeResult {
let _prof_timer = profiler.generic_activity("incr_comp_encode_dep_graph");
self.status.into_inner().finish(profiler)
pub fn finish(&self) -> FileEncodeResult {
let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph_finish");

self.status.lock().take().unwrap().finish(&self.profiler)
}
}

0 comments on commit 094a620

Please sign in to comment.