Create fewer generic signature builders #12062
Merged
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
When type-checking a function or subscript that itself does not have generic parameters (but is within a generic context), we were creating a generic signature builder which will always produce the same generic signature as the enclosing context. Stop creating that generic signature builder. Instead, teach the CompleteGenericTypeResolver to use the generic signature + the canonical generic signature builder for that signature to resolve types, which also eliminates some extraneous re-type-checking. Improves type-checking performance of the standard library by 36%.
…ype. An unconstrained extension will always have the same generic signature as the nominal type it extends. Rather can constructing a new generic signature builder to tell us that, just re-use the generic signature. Improves type-checking performance of the standard library by 15%.
We had two similar loops that performed name lookup for nested types on a potential archetype, so consolidate those into a single implementation on the equivalence class itself.
The full state of the GSB isn’t all that useful for testing, creates a ton of noise and gets in the way of some cleanups we’d like to make in the interface. Stop dumping it as part of `-debug-generic-signatures`.
Funnel all places where we create a generic signature builder to compute the generic signature through a single entry point in the GSB (`computeGenericSignature()`), and make `finalize` and `getGenericSignature` private so no new uses crop up. Tighten up the signature of `computeGenericSignature()` so it only works on GSB rvalues, and ensure that all clients consider the GSB dead after that point by clearing out the internal representation of the GSB.
It’s small and straightforward. NFC
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
|
In my local testing, this cuts the time to type check the standard library down by a third. |
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
|
@swift-ci please test compiler performance |
1 similar comment
|
@swift-ci please test compiler performance |
Build comment file:Summary for master smoketestRegressions found (see below) DebugPR vs. head (debug)PR vs. head, changed counters (debug)
PR vs. head, changed timers (debug)
PR vs. baseline (debug)PR vs. baseline, changed counters (debug)
PR vs. baseline, changed timers (debug)
Wmo-ononePR vs. head (wmo-onone)PR vs. head, changed counters (wmo-onone)
PR vs. head, changed timers (wmo-onone)
PR vs. baseline (wmo-onone)PR vs. baseline, changed counters (wmo-onone)
PR vs. baseline, changed timers (wmo-onone)
ReleasePR vs. head (release)PR vs. head, changed counters (release)
PR vs. head, changed timers (release)
PR vs. baseline (release)PR vs. baseline, changed counters (release)
PR vs. baseline, changed timers (release)
Last baseline commit on smoketest-master-debug.csv
commit 4de0b8570ecc3a81748350219435c85f1060ac16
Author: Graydon Hoare
Date: Tue Sep 19 00:18:10 2017 -0700
Add smoketest cperf-baselines for Xcode 9.0 GM 9A235
Last baseline commit on smoketest-master-wmo-onone.csv commit 4de0b8570ecc3a81748350219435c85f1060ac16
Author: Graydon Hoare
Date: Tue Sep 19 00:18:10 2017 -0700
Add smoketest cperf-baselines for Xcode 9.0 GM 9A235
Last baseline commit on smoketest-master-release.csv commit 4de0b8570ecc3a81748350219435c85f1060ac16
Author: Graydon Hoare
Date: Tue Sep 19 00:18:10 2017 -0700
Add smoketest cperf-baselines for Xcode 9.0 GM 9A235
|
|
@swift-ci please test compiler performance |
|
@swift-ci please smoke test and merge |
|
@swift-ci please test compiler performance |
|
@swift-ci please smoke test and merge |
Build comment file:Summary for master smoketestRegressions found (see below) DebugPR vs. head (debug)PR vs. head, changed counters (debug)
PR vs. head, changed timers (debug)
PR vs. baseline (debug)PR vs. baseline, changed counters (debug)
PR vs. baseline, changed timers (debug)
Wmo-ononePR vs. head (wmo-onone)PR vs. head, changed counters (wmo-onone)
PR vs. head, changed timers (wmo-onone)
PR vs. baseline (wmo-onone)PR vs. baseline, changed counters (wmo-onone)
PR vs. baseline, changed timers (wmo-onone)
ReleasePR vs. head (release)PR vs. head, changed counters (release)
PR vs. head, changed timers (release)
PR vs. baseline (release)PR vs. baseline, changed counters (release)
PR vs. baseline, changed timers (release)
Last baseline commit on smoketest-master-debug.csv
commit 4de0b8570ecc3a81748350219435c85f1060ac16
Author: Graydon Hoare
Date: Tue Sep 19 00:18:10 2017 -0700
Add smoketest cperf-baselines for Xcode 9.0 GM 9A235
Last baseline commit on smoketest-master-wmo-onone.csv commit 4de0b8570ecc3a81748350219435c85f1060ac16
Author: Graydon Hoare
Date: Tue Sep 19 00:18:10 2017 -0700
Add smoketest cperf-baselines for Xcode 9.0 GM 9A235
Last baseline commit on smoketest-master-release.csv commit 4de0b8570ecc3a81748350219435c85f1060ac16
Author: Graydon Hoare
Date: Tue Sep 19 00:18:10 2017 -0700
Add smoketest cperf-baselines for Xcode 9.0 GM 9A235
|
…builder. Once we compute a generic signature from a generic signature builder, all queries involving that generic signature will go through a separate (canonicalized) builder, and the original builder can no longer be used. The canonicalization process then creates a new, effectively identical generic signature builder. How silly. Once we’ve computed the signature of a generic signature builder, “register” it with the ASTContext, allowing us to move the existing generic signature builder into place as the canonical generic signature builder. The builder requires minimal patching but is otherwise fully usable. Thanks to Slava Pestov for the idea!
These only occur with canonicalized builders in invalid code; it’s not worth having them slip out.
The number of generic signature builders “registered” indicates how many times a generic signature builder (GSB) was moved in to become the canonical GSB, rather than recreating the GSB. When a GSB is “already registered”, it means that we’ve constructed a new GSB only to find that there already was a canonical GSB for that particular generic signature. These latter cases could potentially benefit from more caching.
The GSB performs repeated lookups of the same nested type (by name) within a given equivalence class. Cache the results of this lookup.
DougGregor
force-pushed
the
make-fewer-gsbs
branch
from
6cc6378 to
3a3e887
Compare
|
@swift-ci please smoke test and merge |
|
Ahhh, found the issue. It wasn't triggering on my branch. |
|
@swift-ci please smoke test and merge |
|
@swift-ci please smoke test Linux |
|
Doug, I think this might be causing the parsing test failures on the bots? |
|
I'm pulling in the pieces of this that don't break the parsing tests, separately. #12091 has the improvements that avoid creating GSBs when there's already an obvious generic signature to use. |
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.
The
GenericSignatureBuilderis slow, so instead of making it fast... build fewer of them!