Skip to content

perf: dep_graph: stop collecting edges in the green marking walk [-1.70% incr-unchanged] #58

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.

Sign up for GitHub

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

Draft
xmakro wants to merge 1 commit into
base: perf/carry-unchanged-nodes
Choose a base branch
from
Draft

perf: dep_graph: stop collecting edges in the green marking walk [-1.70% incr-unchanged] #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
84 changes: 11 additions & 73 deletions compiler/rustc_middle/src/dep_graph/graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::assert_matches;
use std::cell::Cell;
use std::fmt::Debug;
use std::hash::Hash;
use std::sync::Arc;
Expand All @@ -10,7 +9,7 @@ use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::profiling::QueryInvocationId;
use rustc_data_structures::sharded::{self, ShardedHashMap};
use rustc_data_structures::stable_hash::{StableHash, StableHasher};
use rustc_data_structures::sync::{AtomicU64, Lock, WorkerLocal};
use rustc_data_structures::sync::{AtomicU64, Lock};
use rustc_data_structures::unord::UnordMap;
use rustc_errors::DiagInner;
use rustc_index::IndexVec;
Expand Down Expand Up @@ -90,38 +89,6 @@ pub(crate) struct MarkFrame<'a> {
parent: Option<&'a MarkFrame<'a>>,
}

/// The edge list of one node being marked green: it occupies `buf[start..]` of the shared
/// scratch buffer and is popped again on drop, restoring the buffer for the enclosing call.
struct EdgeFrame<'a> {
buf: &'a mut Vec<DepNodeIndex>,
start: usize,
}

impl<'a> EdgeFrame<'a> {
#[inline]
fn new(buf: &'a mut Vec<DepNodeIndex>) -> Self {
EdgeFrame { start: buf.len(), buf }
}

#[inline]
fn push(&mut self, edge: DepNodeIndex) {
self.buf.push(edge);
}

/// The edges pushed onto this frame so far.
#[inline]
fn get(&self) -> &[DepNodeIndex] {
&self.buf[self.start..]
}
}

impl Drop for EdgeFrame<'_> {
#[inline]
fn drop(&mut self) {
self.buf.truncate(self.start);
}
}

#[derive(Debug)]
pub(super) enum DepNodeColor {
Green(DepNodeIndex),
Expand Down Expand Up @@ -153,9 +120,6 @@ pub struct DepGraphData {
/// (not just marked green)
debug_loaded_from_disk: Lock<FxHashSet<DepNode>>,

/// Per-worker edge buffer amortized across `try_mark_green` calls.
green_edge_buf: WorkerLocal<Cell<Vec<DepNodeIndex>>>,

/// Pool of read recorders, amortized across tasks. Global rather than per worker so the
/// retained memory is bounded by the total number of concurrently recording tasks.
read_recorder_pool: Lock<Vec<ReadsRecorder>>,
Expand Down Expand Up @@ -216,7 +180,6 @@ impl DepGraph {
previous: prev_graph,
colors,
debug_loaded_from_disk: Default::default(),
green_edge_buf: WorkerLocal::default(),
read_recorder_pool: Lock::new(Vec::new()),
})),
virtual_dep_node_index: Arc::new(AtomicU32::new(0)),
Expand Down Expand Up @@ -828,9 +791,8 @@ impl DepGraphData {
fn promote_node_and_deps_to_current(
&self,
prev_index: SerializedDepNodeIndex,
edges: &[DepNodeIndex],
) -> Option<DepNodeIndex> {
let dep_node_index = self.current.encoder.send_promoted(prev_index, &self.colors, edges);
let dep_node_index = self.current.encoder.send_promoted(prev_index, &self.colors);

#[cfg(debug_assertions)]
if let Some(dep_node_index) = dep_node_index {
Expand Down Expand Up @@ -917,29 +879,20 @@ impl DepGraphData {
// in the previous compilation session too, so we can try to
// mark it as green by recursively marking all of its
// dependencies green.

// Reuse a per-worker buffer for the edges instead of allocating one per call.
// The recursion gives it back empty: each `EdgeFrame` pops its edges on drop.
let mut edge_buf = self.green_edge_buf.take();
let result = self.try_mark_previous_green(tcx, prev_index, None, &mut edge_buf);
debug_assert!(edge_buf.is_empty());
self.green_edge_buf.set(edge_buf);
result.map(|dep_node_index| (prev_index, dep_node_index))
self.try_mark_previous_green(tcx, prev_index, None)
.map(|dep_node_index| (prev_index, dep_node_index))
}
}
}

/// Try to mark a dep-node which existed in the previous compilation session as green.
#[instrument(skip(self, tcx, prev_dep_node_index, frame, edge_buf), level = "debug")]
#[instrument(skip(self, tcx, prev_dep_node_index, frame), level = "debug")]
fn try_mark_previous_green<'tcx>(
&self,
tcx: TyCtxt<'tcx>,
prev_dep_node_index: SerializedDepNodeIndex,
frame: Option<&MarkFrame<'_>>,
// Amortized buffer to store edges in.
edge_buf: &mut Vec<DepNodeIndex>,
) -> Option<DepNodeIndex> {
let mut edges = EdgeFrame::new(edge_buf);
let frame = MarkFrame { index: prev_dep_node_index, parent: frame };

// We never try to mark eval_always nodes as green
Expand All @@ -949,10 +902,7 @@ impl DepGraphData {
match self.colors.get(parent_dep_node_index) {
// This dependency has been marked as green before, we are still ok and can
// continue checking the remaining dependencies.
DepNodeColor::Green(parent_index) => {
edges.push(parent_index);
continue;
}
DepNodeColor::Green(_) => continue,

// This dependency's result is different to the previous compilation session. We
// cannot mark this dep_node as green, so stop checking.
Expand All @@ -966,16 +916,8 @@ impl DepGraphData {

// If this dependency isn't eval_always, try to mark it green recursively.
if !tcx.is_eval_always(parent_dep_node.kind)
&& let Some(parent_index) = self.try_mark_previous_green(
tcx,
parent_dep_node_index,
Some(&frame),
// Pass the edge buffer to the recursive call.
// It will use an `EdgeFrame` to give it back unchanged.
edges.buf,
)
&& self.try_mark_previous_green(tcx, parent_dep_node_index, Some(&frame)).is_some()
{
edges.push(parent_index);
continue;
}

Expand All @@ -985,10 +927,7 @@ impl DepGraphData {
}

match self.colors.get(parent_dep_node_index) {
DepNodeColor::Green(parent_index) => {
edges.push(parent_index);
continue;
}
DepNodeColor::Green(_) => continue,
DepNodeColor::Red => return None,
DepNodeColor::Unknown => {}
}
Expand All @@ -1012,12 +951,11 @@ impl DepGraphData {

// There may be multiple threads trying to mark the same dep node green concurrently.

// We allocating an entry for the node in the current dependency graph and
// adding all the appropriate edges imported from the previous graph.
// We allocating an entry for the node in the current dependency graph, keeping the
// edges it had in the previous graph.
//
// `no_hash` nodes may fail this promotion due to already being conservatively colored red.
let dep_node_index =
self.promote_node_and_deps_to_current(prev_dep_node_index, edges.get())?;
let dep_node_index = self.promote_node_and_deps_to_current(prev_dep_node_index)?;

// ... and finally storing a "Green" entry in the color map.
// Multiple threads can all write the same color here.
Expand Down
28 changes: 18 additions & 10 deletions compiler/rustc_middle/src/dep_graph/serialized.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,23 +885,33 @@ impl EncoderState {
/// as is, instead of being rebuilt.
///
/// The node and its edge targets keep their previous indices, so the record on disk is
/// still the right one. `edges` holds the same targets as that record, gathered by the
/// marking walk; only the retained graph reads it.
/// still the right one and the edge list is never needed in memory. The retained graph
/// is the one reader that wants it, and it can be read back off the previous graph.
#[inline]
fn promote_node(
&self,
prev_index: SerializedDepNodeIndex,
retained_graph: &Option<Lock<RetainedDepGraph>>,
local: &mut LocalEncoderState,
edges: &[DepNodeIndex],
) {
let edge_count = self.previous.re_emit_record(prev_index, local.encoder());
debug_assert_eq!(edge_count, edges.len());
self.flush_mem_encoder(&mut *local);
self.count_node(&mut *local);
let node = self.previous.index_to_node(prev_index);
let index = DepNodeIndex::from_u32(prev_index.as_u32());
self.record(node, index, edge_count, edges, retained_graph, &mut *local);

// Every target of a promoted node is green and so keeps its previous index, which
// makes the current edge list the previous one unchanged.
let edges: Vec<DepNodeIndex> = if retained_graph.is_some() {
self.previous
.edge_targets_from(prev_index)
.map(|target| DepNodeIndex::from_u32(target.as_u32()))
.collect()
} else {
Vec::new()
};

self.record(node, index, edge_count, &edges, retained_graph, &mut *local);
}

fn finish(&self, profiler: &SelfProfilerRef, current: &CurrentDepGraph) -> FileEncodeResult {
Expand Down Expand Up @@ -1124,9 +1134,8 @@ impl GraphEncoder {
index
}

/// Encodes a node that was promoted from the previous graph. It reads the information directly
/// from the previous dep graph and expects all edges to already have a new dep node index
/// assigned.
/// Encodes a node that was promoted from the previous graph. It reads the information
/// directly from the previous dep graph and expects all of its edge targets to be green.
///
/// Tries to mark the dep node green, and returns Some if it is now green,
/// or None if had already been concurrently marked red.
Expand All @@ -1135,7 +1144,6 @@ impl GraphEncoder {
&self,
prev_index: SerializedDepNodeIndex,
colors: &DepNodeColorMap,
edges: &[DepNodeIndex],
) -> Option<DepNodeIndex> {
let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph");

Expand All @@ -1153,7 +1161,7 @@ impl GraphEncoder {
.all(|target| matches!(colors.get(target), DepNodeColor::Green(_))),
"promoted node {prev_index:?} names a target that is not green",
);
self.status.promote_node(prev_index, &self.retained_graph, &mut *local, edges);
self.status.promote_node(prev_index, &self.retained_graph, &mut *local);
Some(index)
}
// The query was re-executed in the meantime, by another thread or while
Expand Down