incr.comp.: Make sure we don't lose unused green results from the query cache. #46111
Conversation
kennytm
added
the
S-waiting-on-review
| // 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) { |
There was a problem hiding this comment.
I must be confused, but in what sense does this load from the on-disk cache? It appears to re-execute the query.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
OK, that sounds roughly like what I meant.
|
@bors r+ |
|
📌 Commit 0ea4b47 has been approved by |
kennytm
added
S-waiting-on-bors
incr.comp.: Make sure we don't lose unused green results from the query cache. 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 `DepNodes` that 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
|
☀️ Test successful - status-appveyor, status-travis |
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