Skip to content

Cache the last ObjC bridging conformance we looked up #81545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 16, 2025

Conversation

Catfish-Man
Copy link
Contributor

@Catfish-Man Catfish-Man commented May 15, 2025

Fixes rdar://151475392

(cherry picked from commit 3d9e8db)
@Catfish-Man Catfish-Man self-assigned this May 15, 2025
@Catfish-Man
Copy link
Contributor Author

@swift-ci please Apple Silicon benchmark

@Catfish-Man
Copy link
Contributor Author

@swift-ci please Apple Silicon benchmark

}

static const _ObjectiveCBridgeableWitnessTable *
findBridgeWitness(const Metadata *T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire function is duplicated between DynamicCast.cpp and Casting.cpp, along with its memoization variables. There's also a separate memoization of String stuff in DynamicCast.cpp. I'm unifying the behavior of these (yes they had behavior differences), but saving actually unifying the implementations for later.

@Catfish-Man
Copy link
Contributor Author

1.23x improvement in ObjectiveCBridgeToNSDictionary :) still waiting on my local build to verify the improvement in the new test

@Catfish-Man Catfish-Man marked this pull request as ready for review May 16, 2025 00:12
@Catfish-Man
Copy link
Contributor Author

@swift-ci please test

@Catfish-Man Catfish-Man requested a review from tbkka May 16, 2025 00:21
@Catfish-Man Catfish-Man changed the title Experimentally try caching the last ObjC bridging conformance we looked up Cache the last ObjC bridging conformance we looked up May 16, 2025
@tbkka
Copy link
Contributor

tbkka commented May 16, 2025

Looks fine.

@Catfish-Man
Copy link
Contributor Author

@swift-ci please benchmark

@Catfish-Man Catfish-Man merged commit e795eb0 into main May 16, 2025
6 of 7 checks passed
@Catfish-Man Catfish-Man deleted the doubleword-atomics-are-my-bestie-2 branch May 16, 2025 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants