Skip to content

incr.comp.: Implement query result cache and use it to cache type checking tables. #46004

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
bors merged 21 commits into from
Nov 17, 2017
Merged

incr.comp.: Implement query result cache and use it to cache type checking tables. #46004

Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8cbc022
incr.comp.: Include header when loading cache files in order to get t…
michaelwoerister Nov 13, 2017
c08e03a
incr.comp.: Add position() method to TyEncoder.
michaelwoerister Nov 13, 2017
bc96d9d
incr.comp.: Implement UseSpecializedXXcodable for DefIndex and DefId.
michaelwoerister Nov 13, 2017
9ac1026
incr.comp.: Properly use ty::codec::decode_cnum() in rustc_metadata::…
michaelwoerister Nov 13, 2017
3bd333c
incr.comp.: Add CacheEncoder for encoding query results into the incr…
michaelwoerister Nov 13, 2017
15db165
incr.comp.: Implement TyDecoder for on_disk_cache::CacheDecoder.
michaelwoerister Nov 14, 2017
bedb44c
incr.comp.: Allow for mapping from prev-session-CrateNums to current-…
michaelwoerister Nov 14, 2017
de0317e
incr.comp.: Encode DefPathTables for reconstructing DefIds.
michaelwoerister Nov 14, 2017
2087d5e
incr.comp.: Do some verification on data decoded from incr. comp. cache.
michaelwoerister Nov 14, 2017
4bfab89
incr.comp.: Store the query result index which records where query re…
michaelwoerister Nov 14, 2017
0b14383
incr.comp.: Add 'tcx to QueryDescription.
michaelwoerister Nov 14, 2017
2c1aedd
incr.comp.: Cache TypeckTables and add -Zincremental-queries flag.
michaelwoerister Nov 14, 2017
279b6df
incr.comp.: Refactor query cache serialization to be more re-usable.
michaelwoerister Nov 15, 2017
13582c6
incr.comp.: Add missing [input] annotation for DepNode::MaybeUnusedEx…
michaelwoerister Nov 15, 2017
2f50e62
incr.comp.: Only save and load query result cache when -Zincremental-…
michaelwoerister Nov 15, 2017
24e54dd
Introduce LocalDefId which provides a type-level guarantee that the D…
michaelwoerister Nov 16, 2017
2f44ef2
incr.comp.: Encode DefIds as DefPathHashes instead of recomputing tho…
michaelwoerister Nov 16, 2017
723028f
incr.comp.: Remove some code duplication around TyDecoder by factorin…
michaelwoerister Nov 16, 2017
cb1ff24
incr.comp.: Remove default serialization implementations for things i…
michaelwoerister Nov 16, 2017
4c4f7a3
Fix some tidy errors in ty::codec.
michaelwoerister Nov 16, 2017
0a1f6dd
Add doc comment for LocalDefId.
michaelwoerister Nov 16, 2017
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
32 changes: 30 additions & 2 deletions src/librustc/ty/maps/on_disk_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pub struct OnDiskCache<'sess> {

prev_cnums: Vec<(u32, String, CrateDisambiguator)>,
cnum_map: RefCell<Option<IndexVec<CrateNum, Option<CrateNum>>>>,
prev_def_path_tables: Vec<DefPathTable>,
Copy link
Contributor

@nikomatsakis nikomatsakis Nov 15, 2017

Choose a reason for hiding this comment

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

Does this signal a shift away from the "reverse lookup by hash" strategy?

Copy link
Member Author

@michaelwoerister michaelwoerister Nov 16, 2017

Choose a reason for hiding this comment

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

No, the DefPathTables are stored for facilitating the "reverse lookup by hash". The implementation in this PR does the following:

  • Store a DefId unmodified in the cache.
  • When loading, we need to
    1. map the serialized DefId to the session-independent DefPathHash and then
    2. look up the new DefId by this DefPathHash.

In order to be able to do step (i), we need the old DefPathTable because it maps DefIndices to DefPathHashes.

But,... I'm not surprised that this caught your eye. There is an alternative approach that is simpler to implement and maybe more robust: Map DefIds and DefIndices to DefPathHashes at serialization time and thus get rid of the requirement to store the old DefPathTables. I'm not sure if this affects performance and space requirements in a good or in a bad way. But I'd like to implement it soonish and measure the impact.

Copy link
Contributor

@nikomatsakis nikomatsakis Nov 16, 2017

Choose a reason for hiding this comment

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

OK, I see.


_prev_filemap_starts: BTreeMap<BytePos, StableFilemapId>,
codemap: &'sess CodeMap,
Expand All @@ -73,9 +74,12 @@ impl<'sess> OnDiskCache<'sess> {
debug_assert!(sess.opts.incremental.is_some());

let mut decoder = opaque::Decoder::new(&data[..], start_pos);


// Decode the header
let header = Header::decode(&mut decoder).unwrap();

let prev_diagnostics = {
let (prev_diagnostics, prev_def_path_tables) = {
let mut decoder = CacheDecoder {
tcx: None,
opaque: decoder,
Expand All @@ -85,21 +89,28 @@ impl<'sess> OnDiskCache<'sess> {
prev_def_path_tables: &Vec::new(),
};

// Decode Diagnostics
let prev_diagnostics: FxHashMap<_, _> = {
Copy link
Contributor

@nikomatsakis nikomatsakis Nov 15, 2017

Choose a reason for hiding this comment

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

Why do we have this new variable here? It doesn't serve an obvious purpose. =)

Copy link
Member Author

@michaelwoerister michaelwoerister Nov 16, 2017

Choose a reason for hiding this comment

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

It does in later commits where this block is used to limit the scope of the CacheDecoder.

let diagnostics = EncodedPrevDiagnostics::decode(&mut decoder)
.expect("Error while trying to decode prev. diagnostics \
from incr. comp. cache.");
diagnostics.into_iter().collect()
};

prev_diagnostics
// Decode DefPathTables
let prev_def_path_tables: Vec<DefPathTable> =
Decodable::decode(&mut decoder)
.expect("Error while trying to decode cached DefPathTables");

(prev_diagnostics, prev_def_path_tables)
};

OnDiskCache {
prev_diagnostics,
_prev_filemap_starts: header.prev_filemap_starts,
prev_cnums: header.prev_cnums,
cnum_map: RefCell::new(None),
prev_def_path_tables,
codemap: sess.codemap(),
current_diagnostics: RefCell::new(FxHashMap()),
}
Expand All @@ -111,6 +122,7 @@ impl<'sess> OnDiskCache<'sess> {
_prev_filemap_starts: BTreeMap::new(),
prev_cnums: vec![],
cnum_map: RefCell::new(None),
prev_def_path_tables: Vec::new(),
codemap,
current_diagnostics: RefCell::new(FxHashMap()),
}
Expand Down Expand Up @@ -166,6 +178,22 @@ impl<'sess> OnDiskCache<'sess> {

diagnostics.encode(&mut encoder)?;


// Encode all DefPathTables
let upstream_def_path_tables = tcx.all_crate_nums(LOCAL_CRATE)
.iter()
.map(|&cnum| (cnum, cstore.def_path_table(cnum)))
.collect::<FxHashMap<_,_>>();
let def_path_tables: Vec<&DefPathTable> = sorted_cnums.into_iter().map(|cnum| {
if cnum == LOCAL_CRATE {
tcx.hir.definitions().def_path_table()
} else {
&*upstream_def_path_tables[&cnum]
}
}).collect();

def_path_tables.encode(&mut encoder)?;

return Ok(());

fn sorted_cnums_including_local_crate(cstore: &CrateStore) -> Vec<CrateNum> {
Expand Down