Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upincr.comp.: Make sure we don't lose unused green results from the query cache. #46111
Conversation
rust-highfive
assigned
nikomatsakis
Nov 20, 2017
kennytm
added
the
S-waiting-on-review
label
Nov 20, 2017
| // above `cache_on_disk` methods returns true. | ||
| // Also, as a sanity check, it expects that the corresponding query | ||
| // invocation has been marked as green already. | ||
| pub fn load_from_on_disk_cache(&self, tcx: TyCtxt) { |
This comment has been minimized.
This comment has been minimized.
nikomatsakis
Nov 20, 2017
Contributor
I must be confused, but in what sense does this load from the on-disk cache? It appears to re-execute the query.
This comment has been minimized.
This comment has been minimized.
nikomatsakis
Nov 20, 2017
Contributor
Oh, I guess the point is that it will re-execute the query, which will force us to load. The reason this wrapper exists, then, is to do the assertions? Wouldn't there otherwise be some existing method we could use?
This comment has been minimized.
This comment has been minimized.
michaelwoerister
Nov 20, 2017
Author
Contributor
Oh, I guess the point is that it will re-execute the query, which will force us to load.
Yes
The reason this wrapper exists, then, is to do the assertions?
No, it maps from DepNode to query. There's also force_from_dep_node, which also maps from DepNode to query, but it assumes that the thing being forced has not been seen yet.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@bors r+ |
This comment has been minimized.
This comment has been minimized.
|
|
kennytm
added
S-waiting-on-bors
and removed
S-waiting-on-review
labels
Nov 21, 2017
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Nov 24, 2017
This comment has been minimized.
This comment has been minimized.
|
|
michaelwoerister commentedNov 20, 2017
In its current implementation, the query result cache works by bulk-writing the results of all cacheable queries into a monolithic binary file on disk. Prior to this PR, we would potentially lose query results during this process because only results that had already been loaded into memory were serialized. In contrast, results that were not needed during the given compilation session were not serialized again.
This PR will do one pass over all green
DepNodesthat represent a cacheable query and execute the corresponding query in order to make sure that the query result gets loaded into memory before cache serialization.In the future we might want to look into a serialization format the can be updated in-place so that we don't have to load unchanged results just for immediately storing them again.
r? @nikomatsakis