Lazy decoding of DefPathTable from crate metadata (non-incremental case) #75813
Conversation
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit df57e28 with merge 5bfc961207c712abcd597589d06a63035ab54a69... |
|
Dammit, forgot the capacity commit (15643d5) before starting perf. |
|
Ok, rust-timer didn't start anyway (probably due to pushes). |
|
Awaiting bors try build completion |
|
⌛ Trying commit 6a5e657 with merge d55cd4439a28e659793f7cdf4f25280fc106d0f0... |
|
☀️ Try build successful - checks-actions, checks-azure |
|
Queued d55cd4439a28e659793f7cdf4f25280fc106d0f0 with parent 663d2f5, future comparison URL. |
|
Finished benchmarking try commit (d55cd4439a28e659793f7cdf4f25280fc106d0f0): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
@petrochenkov: Thanks for splitting this out! @bors r+ |
|
📌 Commit 6a5e657 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions, checks-azure |
|
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Final perf results show a major win on the smaller benchmarks where we seem to spend a lot of our time serializing and deserializing crate metadata. However, this is also a noticeable improvement on some non-trivial benchmarks like the |
…, r=pnkfelix Implement lazy decoding of DefPathTable during incremental compilation PR rust-lang#75813 implemented lazy decoding of the `DefPathTable` from crate metadata. However, it requires decoding the entire `DefPathTable` when incremental compilation is active, so that we can map a decoded `DefPathHash` to a `DefId` from an arbitrary crate. This PR adds support for lazy decoding of dependency `DefPathTable`s when incremental compilation si active. When we load the incremental cache and dep graph, we need the ability to map a `DefPathHash` to a `DefId` in the current compilation session (if the corresponding definition still exists). This is accomplished by storing the old `DefId` (that is, the `DefId` from the previous compilation session) for each `DefPathHash` we need to remap. Since a `DefPathHash` includes the owning crate, the old crate is guaranteed to be the right one (if the definition still exists). We then use the old `DefIndex` as an initial guess, which we validate by comparing the expected and actual `DefPathHash`es. In most cases, foreign crates will be completely unchanged, which means that we our guess will be correct. If our guess is wrong, we fall back to decoding the entire `DefPathTable` for the foreign crate. This still represents an improvement over the status quo, since we can skip decoding the entire `DefPathTable` for other crates (where all of our guesses were correct).
The is the half of #74967 that doesn't touch incremental-related structures.
We are still decoding def path hashes eagerly if we are in incremental mode.
The incremental part of #74967 feels hacky, but I'm not qualified enough to suggest improvements. I'll reassign it so someone else once this PR lands.
@Aaron1011, I wasn't asking you to do this split because I wasn't sure that it's feasible (or simple to do).
r? @Aaron1011
UPD: Self-contained description:
If incremental compilation is enabled, then we still have to partially decode the contents of
DefPathTableup front, but only def path hashes.