Naive Dependency Replay #31837
Merged
Naive Dependency Replay #31837
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci test compiler performance |
slavapestov
reviewed
May 18, 2020
| llvm::SaveAndRestore<bool> restore(isRecording, true); | ||
| scratch = {}; | ||
| rec(*this); | ||
| for (const auto &request : stack) { |
There was a problem hiding this comment.
I wonder if instead of recording a dependency in each request on the stack, we only added them to the innermost request. Then upon completing a request (or fulfilling it from the cache), we add all of its entries to the next innermost request, and so on. This should be more efficient in the case where you add duplicate dependencies, because you will consolidate the dupes once instead of re-adding them at each level.
|
I'm puzzled at the wall time regression in release builds. I thought we don't write reference dependencies when building WMO, so we should skip this entire code path, no? |
|
Tracking and replay still occurs, it just never writes into anything. It's something I want to fix by removing the referenced name trackers altogether in the long run. In the short term, we can avoid doing work here by adding a mode bit that turns off evaluator dependency tracking in WMO. |
|
@swift-ci please test compiler performance |
Summary for master fullUnexpected test results, excluded stats for RxCocoa, SwifterSwift, Base64CoderSwiftUI Regressions found (see below) Debug-batchdebug-batch briefRegressed (2)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (1)
debug-batch detailedRegressed (6)
Improved (1)
Unchanged (delta < 1.0% or delta < 100.0ms) (233)
Releaserelease briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
release detailedRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (17)
|
Finish off private intransitive dependencies with an implementation of dependency replay. For the sake of illustration, imagine a chain of requests A -> B -> C -> ... Supposing each request is never cached, then every invocation of the compiler with the same inputs will always kick off the exact same set of requests. For the purposes of dependency tracking, that also means every single lookup request will run without issue, and all dependencies will be accurately reported. But we live in a world with cached requests. Suppose request B* is cached. The first time we encounter that request, its evaluation order looks identical: A -> B* -> C -> ... If we are in a mode that compiles single primaries, this is not a problem because every request graph will look like this. But if we are in a mode where we are compiling multiple primaries, then subsequent request graphs will *actually* hit the cache and never execute request C or any of its dependent computations! A -> B* Supposing C was a lookup request, that means the name(s) looked up downstream of B* will *never* be recorded in the referenced name tracker which can lead to miscompilation. Note that this is not a problem inherent to the design of the request evaluator - caches in the compiler have *always* hidden dependent lookups. In fact, the request evaluator provides us our first opportunity to resolve this correctness bug!
|
At least there's no regressions in WMO with this version. Alright, let's ship it. |
|
@swift-ci test |
|
Build failed |
|
Build failed |
|
@swift-ci test |
|
Build failed |
|
Build failed |
Split off the notion of "recording" dependencies from the notion of "collecting" dependencies. This corrects an oversight in the previous design where dependency replay and recording were actually not "free" in WMO where we actually never track dependencies. This architecture also lays the groundwork for the removal of the referenced name trackers. The algorithm builds upon the infrastructure for dependency sources and sinks laid down during the cut over to request-based dependency tracking in swiftlang#30723. The idea of the naive algorithm is this: For a chain of requests A -> B* -> C -> D* -> ... -> L where L is a lookup request and all starred requests are cached, once L writes into the dependency collector, the active stack is walked and at each cache-point the results of dependency collection are associated with the request itself (in this example, B* and D* have all the names L found associated with them). Subsequent evaluations of these cached requests (B* and D* et al) will then *replay* the previous lookup results from L into the active referenced name tracker. One complication is, suppose the evaluation of a cached request involves multiple downstream name lookups. More concretely, suppose we have the following request trace: A* -> B -> L | -> C -> L | -> D -> L | -> ... Then A* must see the union of the results of each L. If this reminds anyone of a union-find, that is no accident! A persistent union-find a la Conchon and Filliatre is probably in order to help bring down peak heap usage...
|
Ack, wrong conditional - cut off all dependency writing except with private dependencies enabled. We're not trying to go that far. @swift-ci test |
|
@swift-ci test Windows |
|
Build failed |
|
@swift-ci clean test Linux |
|
Build failed |
|
@swift-ci smoke test macOS |
|
⛵ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Split off the notion of "recording" dependencies from the notion of
"collecting" dependencies. This corrects an oversight in the previous
design where dependency replay and recording were actually not "free" in
WMO where we actually never track dependencies. This architecture also
lays the groundwork for the removal of the referenced name trackers.
The algorithm builds upon the infrastructure for dependency sources and
sinks laid down during the cut over to request-based dependency tracking
in #30723.
The idea of the naive algorithm is this:
For a chain of requests
A -> B* -> C -> D* -> ... -> LwhereLis a lookuprequest and all starred requests are cached, once
Lwrites into thedependency collector, the active stack is walked and at each cache-point
the results of dependency collection are associated with the request
itself (in this example,
B*andD*have all the namesLfound associatedwith them). Subsequent evaluations of these cached requests (
B*andD*et al) will then replay the previous lookup results from
Linto theactive referenced name tracker. One complication: suppose the
evaluation of a cached request involves multiple downstream name
lookups. More concretely, suppose we have the following request trace:
Then
A*must see the union of the results of eachL. If this remindsanyone of a union-find, that is no accident! A persistent union-find
a la Conchon and Filliatre is probably in order to help bring down peak
heap usage...