[5.9] Eliminate non-lazy name lookup #66428
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
A recent refactoring uncovered two places where we could end up importing a C++ field declaration as a property more than once: 1. Importing the declaration context of a field in C++ mode can then go import all of the fields. In such a case, check that the field we're importing didn't happen already, and bail out early if it did. This is common practice in the Clang importer but wasn't happening here. 2. One caller to the function that imported a field from a C++ base class into its inheriting class (as a computed property) wasn't checking the cache, and therefore created a redundant version. Fix both issues. (cherry picked from commit b374c09)
Lazy member loading has been in use and the default for several years now. However, the lazy loading was disabled for any type whose primary definition was parsed even though some of its extensions could have been deserialized, e.g., from a Clang module. Moreover, the non-lazy path walked all of the extensions of such a type for all member name lookup operations. Faced with a large number of extensions to the same type (in my example, 6,000), this walk of the list of the extensions could dominate type-checking time. Eliminate all effects of the `-disable-named-lazy-member-loading` flag, and always use the "lazy" path, which effectively does no work for parsed type definitions and extensions thereof. The example with 6,000 extensions of a single type goes from type checking in 6 seconds down to type checking in 0.6 seconds, and name lookup completely disappears from the profiling trace. The deleted tests relied on the flag that is now inert. They aren't by themselves providing much value nowadays, and it's better to have the simpler (and more efficient) implementation of member name lookup be the only one. (cherry picked from commit 234534e)
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
tbkka
approved these changes
Jun 7, 2023
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.
Lazy member loading has been in use and the default for several years
now. However, the lazy loading was disabled for any type whose primary
definition was parsed even though some of its extensions could have
been deserialized, e.g., from a Clang module. Moreover, the non-lazy
path walked all of the extensions of such a type for all member name
lookup operations. Faced with a large number of extensions to the same
type (in my example, 6,000), this walk of the list of the extensions
could dominate type-checking time.
Eliminate all effects of the
-disable-named-lazy-member-loadingflag, and always use the "lazy" path, which effectively does no work
for parsed type definitions and extensions thereof. The example with
6,000 extensions of a single type goes from type checking in 6 seconds
down to type checking in 0.6 seconds, and name lookup completely
disappears from the profiling trace.
The deleted tests relied on the flag that is now inert. They aren't by
themselves providing much value nowadays, and it's better to have the
simpler (and more efficient) implementation of member name lookup be
the only one.