Skip to content

Commit

Permalink
Make create_def a side effect instead of marking the entire query a…
Browse files Browse the repository at this point in the history
…s always red
  • Loading branch information
oli-obk committed Sep 6, 2023
1 parent d6e884d commit 6f8f71c
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 35 deletions.
8 changes: 4 additions & 4 deletions compiler/rustc_interface/src/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) {
fn track_diagnostic(diagnostic: &mut Diagnostic, f: &mut dyn FnMut(&mut Diagnostic)) {
tls::with_context_opt(|icx| {
if let Some(icx) = icx {
if let Some(diagnostics) = icx.diagnostics {
let mut diagnostics = diagnostics.lock();
diagnostics.extend(Some(diagnostic.clone()));
std::mem::drop(diagnostics);
if let Some(side_effects) = icx.side_effects {
let mut side_effects = side_effects.lock();
side_effects.diagnostics.push(diagnostic.clone());
std::mem::drop(side_effects);
}

// Diagnostics are tracked, we can ignore the dependency.
Expand Down
15 changes: 10 additions & 5 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ use rustc_index::IndexVec;
use rustc_macros::HashStable;
use rustc_query_system::dep_graph::DepNodeIndex;
use rustc_query_system::ich::StableHashingContext;
use rustc_query_system::query::DefIdInfo;
use rustc_serialize::opaque::{FileEncodeResult, FileEncoder};
use rustc_session::config::CrateType;
use rustc_session::cstore::{CrateStoreDyn, Untracked};
Expand Down Expand Up @@ -918,11 +919,15 @@ impl<'tcx> TyCtxtAt<'tcx> {
parent: LocalDefId,
data: hir::definitions::DefPathData,
) -> TyCtxtFeed<'tcx, LocalDefId> {
// This function modifies `self.definitions` using a side-effect.
// We need to ensure that these side effects are re-run by the incr. comp. engine.
// Depending on the forever-red node will tell the graph that the calling query
// needs to be re-evaluated.
self.dep_graph.read_index(DepNodeIndex::FOREVER_RED_NODE);
tls::with_context_opt(|icx| {
if let Some(icx) = icx {
if let Some(side_effects) = icx.side_effects {
let mut side_effects = side_effects.lock();
side_effects.definitions.push(DefIdInfo { parent, data, span: self.span });
std::mem::drop(side_effects);
}
}
});

// The following call has the side effect of modifying the tables inside `definitions`.
// These very tables are relied on by the incr. comp. engine to decode DepNodes and to
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_middle/src/ty/context/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ use super::{GlobalCtxt, TyCtxt};
use crate::dep_graph::TaskDepsRef;
use crate::query::plumbing::QueryJobId;
use rustc_data_structures::sync::{self, Lock};
use rustc_errors::Diagnostic;
use rustc_query_system::query::QuerySideEffects;
#[cfg(not(parallel_compiler))]
use std::cell::Cell;
use std::mem;
use std::ptr;
use thin_vec::ThinVec;

/// This is the implicit state of rustc. It contains the current
/// `TyCtxt` and query. It is updated when creating a local interner or
Expand All @@ -26,7 +25,7 @@ pub struct ImplicitCtxt<'a, 'tcx> {

/// Where to store diagnostics for the current query job, if any.
/// This is updated by `JobOwner::start` in `ty::query::plumbing` when executing a query.
pub diagnostics: Option<&'a Lock<ThinVec<Diagnostic>>>,
pub side_effects: Option<&'a Lock<QuerySideEffects>>,

/// Used to prevent queries from calling too deeply.
pub query_depth: usize,
Expand All @@ -42,7 +41,7 @@ impl<'a, 'tcx> ImplicitCtxt<'a, 'tcx> {
ImplicitCtxt {
tcx,
query: None,
diagnostics: None,
side_effects: None,
query_depth: 0,
task_deps: TaskDepsRef::Ignore,
}
Expand Down
24 changes: 18 additions & 6 deletions compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use crate::rustc_middle::ty::TyEncoder;
use crate::QueryConfigRestored;
use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher};
use rustc_data_structures::sync::Lock;
use rustc_errors::Diagnostic;
use rustc_index::Idx;
use rustc_middle::dep_graph::{
self, DepKind, DepKindStruct, DepNode, DepNodeIndex, SerializedDepNodeIndex,
Expand All @@ -20,16 +19,15 @@ use rustc_middle::ty::{self, print::with_no_queries, TyCtxt};
use rustc_query_system::dep_graph::{DepNodeParams, HasDepContext};
use rustc_query_system::ich::StableHashingContext;
use rustc_query_system::query::{
force_query, QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffects,
QueryStackFrame,
force_query, DefIdInfo, QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap,
QuerySideEffects, QueryStackFrame,
};
use rustc_query_system::{LayoutOfDepth, QueryOverflow};
use rustc_serialize::Decodable;
use rustc_serialize::Encodable;
use rustc_session::Limit;
use rustc_span::def_id::LOCAL_CRATE;
use std::num::NonZeroU64;
use thin_vec::ThinVec;

#[derive(Copy, Clone)]
pub struct QueryCtxt<'tcx> {
Expand Down Expand Up @@ -125,7 +123,7 @@ impl QueryContext for QueryCtxt<'_> {
self,
token: QueryJobId,
depth_limit: bool,
diagnostics: Option<&Lock<ThinVec<Diagnostic>>>,
side_effects: Option<&Lock<QuerySideEffects>>,
compute: impl FnOnce() -> R,
) -> R {
// The `TyCtxt` stored in TLS has the same global interner lifetime
Expand All @@ -140,7 +138,7 @@ impl QueryContext for QueryCtxt<'_> {
let new_icx = ImplicitCtxt {
tcx: self.tcx,
query: Some(token),
diagnostics,
side_effects,
query_depth: current_icx.query_depth + depth_limit as usize,
task_deps: current_icx.task_deps,
};
Expand Down Expand Up @@ -172,6 +170,20 @@ impl QueryContext for QueryCtxt<'_> {
crate_name: self.crate_name(LOCAL_CRATE),
});
}

#[tracing::instrument(level = "trace", skip(self))]
fn apply_side_effects(self, side_effects: QuerySideEffects) {
let handle = self.sess.diagnostic();
let QuerySideEffects { diagnostics, definitions } = side_effects;

for mut diagnostic in diagnostics {
handle.emit_diagnostic(&mut diagnostic);
}

for DefIdInfo { parent, data, span: _ } in definitions {
self.tcx.untracked().definitions.write().create_def(parent, data);
}
}
}

pub(super) fn try_mark_green<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &dep_graph::DepNode) -> bool {
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_query_system/src/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -936,11 +936,7 @@ impl<K: DepKind> DepGraphData<K> {
// Promote the previous diagnostics to the current session.
qcx.store_side_effects(dep_node_index, side_effects.clone());

let handle = qcx.dep_context().sess().diagnostic();

for mut diagnostic in side_effects.diagnostics {
handle.emit_diagnostic(&mut diagnostic);
}
qcx.apply_side_effects(side_effects);
}
}
}
Expand Down
26 changes: 20 additions & 6 deletions compiler/rustc_query_system/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_data_structures::stable_hasher::Hash64;
use rustc_data_structures::sync::Lock;
use rustc_errors::Diagnostic;
use rustc_hir::def::DefKind;
use rustc_span::def_id::DefId;
use rustc_span::def_id::{DefId, LocalDefId};
use rustc_span::Span;
use thin_vec::ThinVec;

Expand Down Expand Up @@ -85,18 +85,29 @@ pub struct QuerySideEffects {
/// Stores any diagnostics emitted during query execution.
/// These diagnostics will be re-emitted if we mark
/// the query as green.
pub(super) diagnostics: ThinVec<Diagnostic>,
pub diagnostics: ThinVec<Diagnostic>,
/// Stores any `DefId`s that were created during query execution.
/// These `DefId`s will be re-created when we mark the query as green.
pub definitions: ThinVec<DefIdInfo>,
}

#[derive(Debug, Clone, Encodable, Decodable)]
pub struct DefIdInfo {
pub parent: LocalDefId,
pub data: rustc_hir::definitions::DefPathData,
pub span: Span,
}

impl QuerySideEffects {
#[inline]
pub fn is_empty(&self) -> bool {
let QuerySideEffects { diagnostics } = self;
diagnostics.is_empty()
let QuerySideEffects { diagnostics, definitions } = self;
diagnostics.is_empty() && definitions.is_empty()
}
pub fn append(&mut self, other: QuerySideEffects) {
let QuerySideEffects { diagnostics } = self;
let QuerySideEffects { diagnostics, definitions } = self;
diagnostics.extend(other.diagnostics);
definitions.extend(other.definitions);
}
}

Expand All @@ -114,6 +125,9 @@ pub trait QueryContext: HasDepContext {
/// Register diagnostics for the given node, for use in next session.
fn store_side_effects(self, dep_node_index: DepNodeIndex, side_effects: QuerySideEffects);

/// Actually execute the side effects
fn apply_side_effects(self, side_effects: QuerySideEffects);

/// Register diagnostics for the given node, for use in next session.
fn store_side_effects_for_anon_node(
self,
Expand All @@ -128,7 +142,7 @@ pub trait QueryContext: HasDepContext {
self,
token: QueryJobId,
depth_limit: bool,
diagnostics: Option<&Lock<ThinVec<Diagnostic>>>,
side_effects: Option<&Lock<QuerySideEffects>>,
compute: impl FnOnce() -> R,
) -> R;

Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_query_system/src/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use std::collections::hash_map::Entry;
use std::fmt::Debug;
use std::hash::Hash;
use std::mem;
use thin_vec::ThinVec;

use super::QueryConfig;

Expand Down Expand Up @@ -498,10 +497,10 @@ where
}

let prof_timer = qcx.dep_context().profiler().query_provider();
let diagnostics = Lock::new(ThinVec::new());
let side_effects = Lock::new(QuerySideEffects::default());

let (result, dep_node_index) =
qcx.start_query(job_id, query.depth_limit(), Some(&diagnostics), || {
qcx.start_query(job_id, query.depth_limit(), Some(&side_effects), || {
if query.anon() {
return dep_graph_data.with_anon_task(*qcx.dep_context(), query.dep_kind(), || {
query.compute(qcx, key)
Expand All @@ -523,8 +522,7 @@ where

prof_timer.finish_with_query_invocation_id(dep_node_index.into());

let diagnostics = diagnostics.into_inner();
let side_effects = QuerySideEffects { diagnostics };
let side_effects = side_effects.into_inner();

if std::intrinsics::unlikely(!side_effects.is_empty()) {
if query.anon() {
Expand Down

0 comments on commit 6f8f71c

Please sign in to comment.