[Swift-ObjC-Interop] Speculative cyclic lookup fix #66195
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
The lookup to resolve an ObjC forward declaration to its potential native Swift definition within a mixed module is becoming cyclic somehow. The old uncached lookup still works, so it seems this is an issue with cache invalidation somehow. Until the route of the issue, use the old uncached lookup method.
|
@swift-ci Please test |
slavapestov
approved these changes
May 30, 2023
|
@swift-ci Please Test Source Compatibility |
DougGregor
approved these changes
Jun 6, 2023
|
Thank you! |
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.
Explanation:
In certain scenarios (ie. mixed modules), we must be able to associate an ObjC forward declaration with a corresponding
@objcexposed native Swift declaration. There is no existing look up table to facilitate lookups of Swift declaration via@objcname.Prior to 61606, the behavior used
getTopLevelDeclsWhereAttributesMatchto fetch all Swift declarations with a matching@objcname. In an attempt to make this faster, I createdlookupTopLevelDeclsByObjCNamewhich does the same, but finds all declarations that have any@objcname and puts them into a cache. This lookup deserializes too many declarations.This patch reverts to the old lookup method, which due to a narrowed predicate, does far less deserialization.
Issue:
Deserialization was taking too long on some framework @slavapestov can provide more detail.
Risk:
This is a partial revert, and the existing lookup behavior was used in Swift5.8 and before, so should be low risk. Since the other changes are gated behind a flag, without the flag on, this patch is likely equivalent to reverting 61606 in its entirety.
Testing:
CI