Skip to content

Commit 549f855

Browse files
committed
Refactor ensure and try_get_with into read_node_index
There was a bit of code shared between `try_get_with` and `ensure`, after I added `ensure`. I refactored that shared code into a query-agnostic method called `read_node_index`. The new method `read_node_index` will attempt to find the node index (`DepNodeIndex`) of a query. When `read_node_index` finds the `DepNodeIndex`, it marks the current query as a reader of the node it's requesting the index of. This is used by `try_get_with` and `ensure` as it elides the unimportant (to them) details of if the query is invalidated by previous changed computation (Red) or new and if they had to mark the query green. For both `try_get_with` and `ensure`, they just need to know if they can lookup the results or have to reevaluate.
1 parent 2689fd2 commit 549f855

File tree

2 files changed

+43
-48
lines changed

2 files changed

+43
-48
lines changed

src/librustc/dep_graph/graph.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,40 @@ impl DepGraph {
427427
self.data.as_ref().and_then(|data| data.colors.borrow().get(dep_node).cloned())
428428
}
429429

430+
/// Try to read a node index for the node dep_node.
431+
/// A node will have an index, when it's already been marked green, or when we can mark it
432+
/// green. This function will mark the current task as a reader of the specified node, when
433+
/// the a node index can be found for that node.
434+
pub fn read_node_index(&self, tcx: TyCtxt, dep_node: &DepNode) -> Option<DepNodeIndex> {
435+
match self.node_color(dep_node) {
436+
Some(DepNodeColor::Green(dep_node_index)) => {
437+
self.read_index(dep_node_index);
438+
Some(dep_node_index)
439+
}
440+
Some(DepNodeColor::Red) => {
441+
None
442+
}
443+
None => {
444+
// try_mark_green (called below) will panic when full incremental
445+
// compilation is disabled. If that's the case, we can't try to mark nodes
446+
// as green anyway, so we can safely return None here.
447+
if !self.is_fully_enabled() {
448+
return None;
449+
}
450+
match self.try_mark_green(tcx, &dep_node) {
451+
Some(dep_node_index) => {
452+
debug_assert!(tcx.dep_graph.is_green(dep_node_index));
453+
tcx.dep_graph.read_index(dep_node_index);
454+
Some(dep_node_index)
455+
}
456+
None => {
457+
None
458+
}
459+
}
460+
}
461+
}
462+
}
463+
430464
pub fn try_mark_green(&self,
431465
tcx: TyCtxt,
432466
dep_node: &DepNode)

src/librustc/ty/maps/plumbing.rs

Lines changed: 9 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -309,25 +309,8 @@ macro_rules! define_maps {
309309
}
310310

311311
if !dep_node.kind.is_input() {
312-
use dep_graph::DepNodeColor;
313-
if let Some(DepNodeColor::Green(dep_node_index)) = tcx.dep_graph
314-
.node_color(&dep_node) {
312+
if let Some(dep_node_index) = tcx.dep_graph.read_node_index(tcx, &dep_node) {
315313
profq_msg!(tcx, ProfileQueriesMsg::CacheHit);
316-
tcx.dep_graph.read_index(dep_node_index);
317-
return Self::load_from_disk_and_cache_in_memory(tcx,
318-
key,
319-
span,
320-
dep_node_index)
321-
}
322-
323-
debug!("ty::queries::{}::try_get_with(key={:?}) - running try_mark_green",
324-
stringify!($name),
325-
key);
326-
327-
if let Some(dep_node_index) = tcx.dep_graph.try_mark_green(tcx, &dep_node) {
328-
debug_assert!(tcx.dep_graph.is_green(dep_node_index));
329-
profq_msg!(tcx, ProfileQueriesMsg::CacheHit);
330-
tcx.dep_graph.read_index(dep_node_index);
331314
return Self::load_from_disk_and_cache_in_memory(tcx,
332315
key,
333316
span,
@@ -357,36 +340,14 @@ macro_rules! define_maps {
357340
// Ensuring an "input" or anonymous query makes no sense
358341
assert!(!dep_node.kind.is_anon());
359342
assert!(!dep_node.kind.is_input());
360-
use dep_graph::DepNodeColor;
361-
match tcx.dep_graph.node_color(&dep_node) {
362-
Some(DepNodeColor::Green(dep_node_index)) => {
363-
tcx.dep_graph.read_index(dep_node_index);
364-
}
365-
Some(DepNodeColor::Red) => {
366-
// A DepNodeColor::Red DepNode means that this query was executed
367-
// before. We can not call `dep_graph.read()` here as we don't have
368-
// the DepNodeIndex. Instead, We call the query again to issue the
369-
// appropriate `dep_graph.read()` call. The performance cost this
370-
// introduces should be negligible as we'll immediately hit the
371-
// in-memory cache.
372-
let _ = tcx.$name(key);
373-
}
374-
None => {
375-
// Huh
376-
if !tcx.dep_graph.is_fully_enabled() {
377-
let _ = tcx.$name(key);
378-
return;
379-
}
380-
match tcx.dep_graph.try_mark_green(tcx, &dep_node) {
381-
Some(dep_node_index) => {
382-
debug_assert!(tcx.dep_graph.is_green(dep_node_index));
383-
tcx.dep_graph.read_index(dep_node_index);
384-
}
385-
None => {
386-
let _ = tcx.$name(key);
387-
}
388-
}
389-
}
343+
if tcx.dep_graph.read_node_index(tcx, &dep_node).is_none() {
344+
// A None return from `read_node_index` means that this is either
345+
// a new dep node or that the dep node has already been marked red.
346+
// Either way, we can't call `dep_graph.read()` as we don't have the
347+
// DepNodeIndex. We must invoke the query itself. The performance cost
348+
// this introduces should be negligible as we'll immediately hit the
349+
// in-memory cache, or another query down the line will.
350+
let _ = tcx.$name(key);
390351
}
391352
}
392353

0 commit comments

Comments
 (0)