Skip to content

Commit 659b758

Browse files
committed
Make DepNodeColor tri-value.
Currently it's binary, either `Green` or `Red`. But it's almost always used within an `Option`. So it's a bit neater, and possibly slightly faster, to make it tri-value with `Unknown` as a first-class variant.
1 parent a4162d9 commit 659b758

File tree

2 files changed

+31
-39
lines changed

2 files changed

+31
-39
lines changed

compiler/rustc_query_system/src/dep_graph/graph.rs

Lines changed: 28 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -68,20 +68,11 @@ pub struct MarkFrame<'a> {
6868

6969
#[derive(Debug)]
7070
pub(super) enum DepNodeColor {
71+
Unknown,
7172
Red,
7273
Green(DepNodeIndex),
7374
}
7475

75-
impl DepNodeColor {
76-
#[inline]
77-
fn is_green(self) -> bool {
78-
match self {
79-
DepNodeColor::Red => false,
80-
DepNodeColor::Green(_) => true,
81-
}
82-
}
83-
}
84-
8576
pub(crate) struct DepGraphData<D: Deps> {
8677
/// The new encoding of the dependency graph, optimized for red/green
8778
/// tracking. The `current` field is the dependency graph of only the
@@ -624,7 +615,7 @@ impl<D: Deps> DepGraphData<D> {
624615
) {
625616
if let Some(prev_index) = self.previous.node_to_index_opt(dep_node) {
626617
let current = self.colors.get(prev_index);
627-
assert!(current.is_none(), "{}", msg())
618+
assert_matches!(current, DepNodeColor::Unknown, "{}", msg())
628619
} else if let Some(nodes_in_current_session) = &self.current.nodes_in_current_session {
629620
outline(|| {
630621
let seen = nodes_in_current_session.lock().contains_key(dep_node);
@@ -633,20 +624,20 @@ impl<D: Deps> DepGraphData<D> {
633624
}
634625
}
635626

636-
fn node_color(&self, dep_node: &DepNode) -> Option<DepNodeColor> {
627+
fn node_color(&self, dep_node: &DepNode) -> DepNodeColor {
637628
if let Some(prev_index) = self.previous.node_to_index_opt(dep_node) {
638629
self.colors.get(prev_index)
639630
} else {
640631
// This is a node that did not exist in the previous compilation session.
641-
None
632+
DepNodeColor::Unknown
642633
}
643634
}
644635

645636
/// Returns true if the given node has been marked as green during the
646637
/// current compilation session. Used in various assertions
647638
#[inline]
648639
pub(crate) fn is_index_green(&self, prev_index: SerializedDepNodeIndex) -> bool {
649-
self.colors.get(prev_index).is_some_and(|c| c.is_green())
640+
matches!(self.colors.get(prev_index), DepNodeColor::Green(_))
650641
}
651642

652643
#[inline]
@@ -820,12 +811,12 @@ impl<D: Deps> DepGraph<D> {
820811
self.data.as_ref()?.dep_node_debug.borrow().get(&dep_node).cloned()
821812
}
822813

823-
fn node_color(&self, dep_node: &DepNode) -> Option<DepNodeColor> {
814+
fn node_color(&self, dep_node: &DepNode) -> DepNodeColor {
824815
if let Some(ref data) = self.data {
825816
return data.node_color(dep_node);
826817
}
827818

828-
None
819+
DepNodeColor::Unknown
829820
}
830821

831822
pub fn try_mark_green<Qcx: QueryContext<Deps = D>>(
@@ -854,9 +845,9 @@ impl<D: Deps> DepGraphData<D> {
854845
let prev_index = self.previous.node_to_index_opt(dep_node)?;
855846

856847
match self.colors.get(prev_index) {
857-
Some(DepNodeColor::Green(dep_node_index)) => Some((prev_index, dep_node_index)),
858-
Some(DepNodeColor::Red) => None,
859-
None => {
848+
DepNodeColor::Green(dep_node_index) => Some((prev_index, dep_node_index)),
849+
DepNodeColor::Red => None,
850+
DepNodeColor::Unknown => {
860851
// This DepNode and the corresponding query invocation existed
861852
// in the previous compilation session too, so we can try to
862853
// mark it as green by recursively marking all of its
@@ -877,7 +868,7 @@ impl<D: Deps> DepGraphData<D> {
877868
let get_dep_dep_node = || self.previous.index_to_node(parent_dep_node_index);
878869

879870
match self.colors.get(parent_dep_node_index) {
880-
Some(DepNodeColor::Green(_)) => {
871+
DepNodeColor::Green(_) => {
881872
// This dependency has been marked as green before, we are
882873
// still fine and can continue with checking the other
883874
// dependencies.
@@ -888,15 +879,15 @@ impl<D: Deps> DepGraphData<D> {
888879
debug!("dependency {:?} was immediately green", get_dep_dep_node());
889880
return Some(());
890881
}
891-
Some(DepNodeColor::Red) => {
882+
DepNodeColor::Red => {
892883
// We found a dependency the value of which has changed
893884
// compared to the previous compilation session. We cannot
894885
// mark the DepNode as green and also don't need to bother
895886
// with checking any of the other dependencies.
896887
debug!("dependency {:?} was immediately red", get_dep_dep_node());
897888
return None;
898889
}
899-
None => {}
890+
DepNodeColor::Unknown => {}
900891
}
901892

902893
let dep_dep_node = &get_dep_dep_node();
@@ -927,15 +918,15 @@ impl<D: Deps> DepGraphData<D> {
927918
}
928919

929920
match self.colors.get(parent_dep_node_index) {
930-
Some(DepNodeColor::Green(_)) => {
921+
DepNodeColor::Green(_) => {
931922
debug!("managed to FORCE dependency {dep_dep_node:?} to green");
932923
return Some(());
933924
}
934-
Some(DepNodeColor::Red) => {
925+
DepNodeColor::Red => {
935926
debug!("dependency {dep_dep_node:?} was red after forcing");
936927
return None;
937928
}
938-
None => {}
929+
DepNodeColor::Unknown => {}
939930
}
940931

941932
if let None = qcx.dep_context().sess().dcx().has_errors_or_delayed_bugs() {
@@ -1000,13 +991,13 @@ impl<D: Deps> DepGraph<D> {
1000991
/// Returns true if the given node has been marked as red during the
1001992
/// current compilation session. Used in various assertions
1002993
pub fn is_red(&self, dep_node: &DepNode) -> bool {
1003-
matches!(self.node_color(dep_node), Some(DepNodeColor::Red))
994+
matches!(self.node_color(dep_node), DepNodeColor::Red)
1004995
}
1005996

1006997
/// Returns true if the given node has been marked as green during the
1007998
/// current compilation session. Used in various assertions
1008999
pub fn is_green(&self, dep_node: &DepNode) -> bool {
1009-
self.node_color(dep_node).is_some_and(|c| c.is_green())
1000+
matches!(self.node_color(dep_node), DepNodeColor::Green(_))
10101001
}
10111002

10121003
pub fn assert_dep_node_not_yet_allocated_in_current_session<S: std::fmt::Display>(
@@ -1033,11 +1024,11 @@ impl<D: Deps> DepGraph<D> {
10331024
let data = self.data.as_ref().unwrap();
10341025
for prev_index in data.colors.values.indices() {
10351026
match data.colors.get(prev_index) {
1036-
Some(DepNodeColor::Green(_)) => {
1027+
DepNodeColor::Green(_) => {
10371028
let dep_node = data.previous.index_to_node(prev_index);
10381029
tcx.try_load_from_on_disk_cache(dep_node);
10391030
}
1040-
None | Some(DepNodeColor::Red) => {
1031+
DepNodeColor::Unknown | DepNodeColor::Red => {
10411032
// We can skip red nodes because a node can only be marked
10421033
// as red if the query result was recomputed and thus is
10431034
// already in memory.
@@ -1324,14 +1315,14 @@ pub(super) struct DepNodeColorMap {
13241315
sync: bool,
13251316
}
13261317

1327-
const COMPRESSED_NONE: u32 = u32::MAX;
1318+
const COMPRESSED_UNKNOWN: u32 = u32::MAX;
13281319
const COMPRESSED_RED: u32 = u32::MAX - 1;
13291320

13301321
impl DepNodeColorMap {
13311322
fn new(size: usize) -> DepNodeColorMap {
13321323
debug_assert!(COMPRESSED_RED > DepNodeIndex::MAX_AS_U32);
13331324
DepNodeColorMap {
1334-
values: (0..size).map(|_| AtomicU32::new(COMPRESSED_NONE)).collect(),
1325+
values: (0..size).map(|_| AtomicU32::new(COMPRESSED_UNKNOWN)).collect(),
13351326
sync: is_dyn_thread_safe(),
13361327
}
13371328
}
@@ -1354,7 +1345,7 @@ impl DepNodeColorMap {
13541345
let value = &self.values[prev_index];
13551346
if self.sync {
13561347
match value.compare_exchange(
1357-
COMPRESSED_NONE,
1348+
COMPRESSED_UNKNOWN,
13581349
index.as_u32(),
13591350
Ordering::Relaxed,
13601351
Ordering::Relaxed,
@@ -1364,7 +1355,7 @@ impl DepNodeColorMap {
13641355
}
13651356
} else {
13661357
let v = value.load(Ordering::Relaxed);
1367-
if v == COMPRESSED_NONE {
1358+
if v == COMPRESSED_UNKNOWN {
13681359
value.store(index.as_u32(), Ordering::Relaxed);
13691360
Ok(())
13701361
} else {
@@ -1374,11 +1365,11 @@ impl DepNodeColorMap {
13741365
}
13751366

13761367
#[inline]
1377-
pub(super) fn get(&self, index: SerializedDepNodeIndex) -> Option<DepNodeColor> {
1368+
pub(super) fn get(&self, index: SerializedDepNodeIndex) -> DepNodeColor {
13781369
match self.values[index].load(Ordering::Acquire) {
1379-
COMPRESSED_NONE => None,
1380-
COMPRESSED_RED => Some(DepNodeColor::Red),
1381-
value => Some(DepNodeColor::Green(DepNodeIndex::from_u32(value))),
1370+
COMPRESSED_UNKNOWN => DepNodeColor::Unknown,
1371+
COMPRESSED_RED => DepNodeColor::Red,
1372+
value => DepNodeColor::Green(DepNodeIndex::from_u32(value)),
13821373
}
13831374
}
13841375

compiler/rustc_query_system/src/dep_graph/serialized.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -914,8 +914,9 @@ impl<D: Deps> GraphEncoder<D> {
914914
index
915915
}
916916

917-
/// Encodes a node that was promoted from the previous graph. It reads the information directly from
918-
/// the previous dep graph and expects all edges to already have a new dep node index assigned.
917+
/// Encodes a node that was promoted from the previous graph. It reads the information directly
918+
/// from the previous dep graph and expects all edges to already have a new dep node index
919+
/// assigned.
919920
///
920921
/// This will also ensure the dep node is marked green.
921922
#[inline]

0 commit comments

Comments
 (0)