Skip to content

Don't use a thread to load the dep graph #116109

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

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Don't use a thread to load the dep graph #116109

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
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ pub use persist::load_query_result_cache;
pub use persist::prepare_session_directory;
pub use persist::save_dep_graph;
pub use persist::save_work_product_index;
pub use persist::setup_dep_graph;
pub use persist::LoadResult;
pub use persist::{build_dep_graph, load_dep_graph, DepGraphFuture};

use rustc_errors::{DiagnosticMessage, SubdiagnosticMessage};
use rustc_fluent_macro::fluent_messages;
Expand Down
132 changes: 62 additions & 70 deletions compiler/rustc_incremental/src/persist/load.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,19 @@
use crate::errors;
use rustc_data_structures::memmap::Mmap;
use rustc_data_structures::unord::UnordMap;
use rustc_middle::dep_graph::{DepsType, SerializedDepGraph, WorkProductMap};
use rustc_middle::dep_graph::{DepGraph, DepsType, SerializedDepGraph, WorkProductMap};
use rustc_middle::query::on_disk_cache::OnDiskCache;
use rustc_serialize::opaque::MemDecoder;
use rustc_serialize::Decodable;
use rustc_session::config::IncrementalStateAssertion;
use rustc_session::Session;
use rustc_session::{Session, StableCrateId};
use rustc_span::{ErrorGuaranteed, Symbol};
use std::path::{Path, PathBuf};

use super::data::*;
use super::file_format;
use super::fs::*;
use super::save::build_dep_graph;
use super::work_product;

#[derive(Debug)]
Expand Down Expand Up @@ -72,21 +74,12 @@ impl<T: Default> LoadResult<T> {
}

fn load_data(path: &Path, sess: &Session) -> LoadResult<(Mmap, usize)> {
load_data_no_sess(
match file_format::read_file(
path,
sess.opts.unstable_opts.incremental_info,
sess.is_nightly_build(),
sess.cfg_version,
)
}

fn load_data_no_sess(
path: &Path,
report_incremental_info: bool,
is_nightly_build: bool,
cfg_version: &'static str,
) -> LoadResult<(Mmap, usize)> {
match file_format::read_file(path, report_incremental_info, is_nightly_build, cfg_version) {
) {
Ok(Some(data_and_pos)) => LoadResult::Ok { data: data_and_pos },
Ok(None) => {
// The file either didn't exist or was produced by an incompatible
Expand All @@ -102,47 +95,19 @@ fn delete_dirty_work_product(sess: &Session, swp: SerializedWorkProduct) {
work_product::delete_workproduct_files(sess, &swp.work_product);
}

/// Either a result that has already be computed or a
/// handle that will let us wait until it is computed
/// by a background thread.
pub enum MaybeAsync<T> {
Sync(T),
Async(std::thread::JoinHandle<T>),
}

impl<T> MaybeAsync<LoadResult<T>> {
/// Accesses the data returned in [`LoadResult::Ok`] in an asynchronous way if possible.
pub fn open(self) -> LoadResult<T> {
match self {
MaybeAsync::Sync(result) => result,
MaybeAsync::Async(handle) => {
handle.join().unwrap_or_else(|e| LoadResult::DecodeIncrCache(e))
}
}
}
}

/// An asynchronous type for computing the dependency graph.
pub type DepGraphFuture = MaybeAsync<LoadResult<(SerializedDepGraph, WorkProductMap)>>;

/// Launch a thread and load the dependency graph in the background.
pub fn load_dep_graph(sess: &Session) -> DepGraphFuture {
// Since `sess` isn't `Sync`, we perform all accesses to `sess`
// before we fire the background thread.

fn load_dep_graph(sess: &Session) -> LoadResult<(SerializedDepGraph, WorkProductMap)> {
let prof = sess.prof.clone();

if sess.opts.incremental.is_none() {
// No incremental compilation.
return MaybeAsync::Sync(LoadResult::Ok { data: Default::default() });
return LoadResult::Ok { data: Default::default() };
}

let _timer = sess.prof.generic_activity("incr_comp_prepare_load_dep_graph");

// Calling `sess.incr_comp_session_dir()` will panic if `sess.opts.incremental.is_none()`.
// Fortunately, we just checked that this isn't the case.
let path = dep_graph_path(&sess);
let report_incremental_info = sess.opts.unstable_opts.incremental_info;
let expected_hash = sess.opts.dep_tracking_hash(false);

let mut prev_work_products = UnordMap::default();
Expand Down Expand Up @@ -180,40 +145,35 @@ pub fn load_dep_graph(sess: &Session) -> DepGraphFuture {
}
}

let is_nightly_build = sess.is_nightly_build();
let cfg_version = sess.cfg_version;

MaybeAsync::Async(std::thread::spawn(move || {
let _prof_timer = prof.generic_activity("incr_comp_load_dep_graph");
let _prof_timer = prof.generic_activity("incr_comp_load_dep_graph");

match load_data_no_sess(&path, report_incremental_info, is_nightly_build, cfg_version) {
LoadResult::DataOutOfDate => LoadResult::DataOutOfDate,
LoadResult::LoadDepGraph(path, err) => LoadResult::LoadDepGraph(path, err),
LoadResult::DecodeIncrCache(err) => LoadResult::DecodeIncrCache(err),
LoadResult::Ok { data: (bytes, start_pos) } => {
let mut decoder = MemDecoder::new(&bytes, start_pos);
let prev_commandline_args_hash = u64::decode(&mut decoder);
match load_data(&path, sess) {
LoadResult::DataOutOfDate => LoadResult::DataOutOfDate,
LoadResult::LoadDepGraph(path, err) => LoadResult::LoadDepGraph(path, err),
LoadResult::DecodeIncrCache(err) => LoadResult::DecodeIncrCache(err),
LoadResult::Ok { data: (bytes, start_pos) } => {
let mut decoder = MemDecoder::new(&bytes, start_pos);
let prev_commandline_args_hash = u64::decode(&mut decoder);

if prev_commandline_args_hash != expected_hash {
if report_incremental_info {
eprintln!(
"[incremental] completely ignoring cache because of \
if prev_commandline_args_hash != expected_hash {
if sess.opts.unstable_opts.incremental_info {
eprintln!(
"[incremental] completely ignoring cache because of \
differing commandline arguments"
);
}
// We can't reuse the cache, purge it.
debug!("load_dep_graph_new: differing commandline arg hashes");

// No need to do any further work
return LoadResult::DataOutOfDate;
);
}
// We can't reuse the cache, purge it.
debug!("load_dep_graph_new: differing commandline arg hashes");

let dep_graph = SerializedDepGraph::decode::<DepsType>(&mut decoder);

LoadResult::Ok { data: (dep_graph, prev_work_products) }
// No need to do any further work
return LoadResult::DataOutOfDate;
}

let dep_graph = SerializedDepGraph::decode::<DepsType>(&mut decoder);

LoadResult::Ok { data: (dep_graph, prev_work_products) }
}
}))
}
}

/// Attempts to load the query result cache from disk
Expand All @@ -235,3 +195,35 @@ pub fn load_query_result_cache(sess: &Session) -> Option<OnDiskCache<'_>> {
_ => Some(OnDiskCache::new_empty(sess.source_map())),
}
}

/// Setups the dependency graph by loading an existing graph from disk and set up streaming of a
/// new graph to an incremental session directory.
pub fn setup_dep_graph(
sess: &Session,
crate_name: Symbol,
stable_crate_id: StableCrateId,
) -> Result<DepGraph, ErrorGuaranteed> {
// `load_dep_graph` can only be called after `prepare_session_directory`.
prepare_session_directory(sess, crate_name, stable_crate_id)?;

let res = sess.opts.build_dep_graph().then(|| load_dep_graph(sess));

if sess.opts.incremental.is_some() {
sess.time("incr_comp_garbage_collect_session_directories", || {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There does seem to be a small performance hit which is probably this no longer being done in parallel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like noise to me on the graph of the regressed perf tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does look like the time incr_comp_garbage_collect_session_directories takes in practice is quite small, so it's probably just noise.

Copy link
Member

@lqd lqd Sep 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a relative/absolute graph thingy again. It looks small and maybe not worth looking into, but seems unlikely to be noise with this shape.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was referring to the wall time regression in the initial perf run. The instruction count isn't very useful as this PR involves parallelism.

if let Err(e) = garbage_collect_session_directories(sess) {
warn!(
"Error while trying to garbage collect incremental \
compilation cache directory: {}",
e
);
}
});
}

Ok(res
.and_then(|result| {
let (prev_graph, prev_work_products) = result.open(sess);
build_dep_graph(sess, prev_graph, prev_work_products)
})
.unwrap_or_else(DepGraph::new_disabled))
}
3 changes: 1 addition & 2 deletions compiler/rustc_incremental/src/persist/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ pub use fs::in_incr_comp_dir;
pub use fs::in_incr_comp_dir_sess;
pub use fs::prepare_session_directory;
pub use load::load_query_result_cache;
pub use load::setup_dep_graph;
pub use load::LoadResult;
pub use load::{load_dep_graph, DepGraphFuture};
pub use save::build_dep_graph;
pub use save::save_dep_graph;
pub use save::save_work_product_index;
pub use work_product::copy_cgu_workproduct_to_incr_comp_cache_dir;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/src/persist/save.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ fn encode_query_cache(tcx: TyCtxt<'_>, encoder: FileEncoder) -> FileEncodeResult
/// execution, the new dependency information is not kept in memory but directly
/// output to this file. `save_dep_graph` then finalizes the staging dep-graph
/// and moves it to the permanent dep-graph path
pub fn build_dep_graph(
pub(crate) fn build_dep_graph(
sess: &Session,
prev_graph: SerializedDepGraph,
prev_work_products: WorkProductMap,
Expand Down
47 changes: 3 additions & 44 deletions compiler/rustc_interface/src/queries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::{AppendOnlyIndexVec, FreezeLock, Lrc, OnceLock, WorkerLocal};
use rustc_hir::def_id::{StableCrateId, CRATE_DEF_ID, LOCAL_CRATE};
use rustc_hir::definitions::Definitions;
use rustc_incremental::DepGraphFuture;
use rustc_incremental::setup_dep_graph;
use rustc_metadata::creader::CStore;
use rustc_middle::arena::Arena;
use rustc_middle::dep_graph::DepGraph;
Expand All @@ -19,7 +19,6 @@ use rustc_session::config::{self, CrateType, OutputFilenames, OutputType};
use rustc_session::cstore::Untracked;
use rustc_session::{output::find_crate_name, Session};
use rustc_span::symbol::sym;
use rustc_span::Symbol;
use std::any::Any;
use std::cell::{RefCell, RefMut};
use std::sync::Arc;
Expand Down Expand Up @@ -132,43 +131,6 @@ impl<'tcx> Queries<'tcx> {
})
}

fn dep_graph_future(
&self,
crate_name: Symbol,
stable_crate_id: StableCrateId,
) -> Result<Option<DepGraphFuture>> {
let sess = self.session();

// `load_dep_graph` can only be called after `prepare_session_directory`.
rustc_incremental::prepare_session_directory(sess, crate_name, stable_crate_id)?;
let res = sess.opts.build_dep_graph().then(|| rustc_incremental::load_dep_graph(sess));

if sess.opts.incremental.is_some() {
sess.time("incr_comp_garbage_collect_session_directories", || {
if let Err(e) = rustc_incremental::garbage_collect_session_directories(sess) {
warn!(
"Error while trying to garbage collect incremental \
compilation cache directory: {}",
e
);
}
});
}

Ok(res)
}

fn dep_graph(&self, dep_graph_future: Option<DepGraphFuture>) -> DepGraph {
dep_graph_future
.and_then(|future| {
let sess = self.session();
let (prev_graph, prev_work_products) =
sess.time("blocked_on_dep_graph_loading", || future.open().open(sess));
rustc_incremental::build_dep_graph(sess, prev_graph, prev_work_products)
})
.unwrap_or_else(DepGraph::new_disabled)
}

pub fn global_ctxt(&'tcx self) -> Result<QueryResult<'_, &'tcx GlobalCtxt<'tcx>>> {
self.gcx.compute(|| {
let sess = self.session();
Expand All @@ -184,10 +146,7 @@ impl<'tcx> Queries<'tcx> {
sess.opts.cg.metadata.clone(),
sess.cfg_version,
);

// Compute the dependency graph (in the background). We want to do this as early as
// possible, to give the DepGraph maximum time to load before `dep_graph` is called.
let dep_graph_future = self.dep_graph_future(crate_name, stable_crate_id)?;
let dep_graph = setup_dep_graph(sess, crate_name, stable_crate_id)?;

let lint_store = Lrc::new(passes::create_lint_store(
sess,
Expand All @@ -210,7 +169,7 @@ impl<'tcx> Queries<'tcx> {
crate_types,
stable_crate_id,
lint_store,
self.dep_graph(dep_graph_future),
dep_graph,
untracked,
&self.gcx_cell,
&self.arena,
Expand Down