Skip to content

[Runtime] Let CF provide us with state, avoiding runtime lookup. rdar://111104451 #66606

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 12 commits into from
Jun 21, 2023

Conversation

Catfish-Man
Copy link
Contributor

@Catfish-Man Catfish-Man commented Jun 13, 2023

We can avoid a bunch of calls to objc_lookUpClass() if we have CF pass in the information we need, which it already knows.

Fixes rdar://111104451

@Catfish-Man Catfish-Man self-assigned this Jun 13, 2023
@Catfish-Man
Copy link
Contributor Author

@swift-ci please smoke test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please smoke test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please smoke test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please benchmark

@Catfish-Man Catfish-Man marked this pull request as ready for review June 16, 2023 15:57
@Catfish-Man
Copy link
Contributor Author

@swift-ci please test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please test

1 similar comment
@Catfish-Man
Copy link
Contributor Author

@swift-ci please test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please test

@Catfish-Man Catfish-Man requested a review from al45tair June 21, 2023 01:49
@Catfish-Man
Copy link
Contributor Author

ok! Tests green. Mike noticed a few things to tweak, so retesting with those now.

@Catfish-Man
Copy link
Contributor Author

@swift-ci please test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@al45tair al45tair left a comment

Choose a reason for hiding this comment

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

Like it :-)

static inline void initializeBridgingFunctions() {
swift_once(&initializeBridgingFuncsOnce,
static inline bool initializeBridgingFunctions() {
swift_once(&initializeBridgingStateOnce,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: In runtime code you can use swift::once as well; the latter has a slightly nicer interface (the context argument is optional and defaults to nullptr, and swift::once also supports arbitrary C++ callables, so lambdas and things will work with it). swift_once is the ABI stable entry point for the compiler to generate calls to.

@@ -87,22 +87,6 @@ swift_stdlib_NSObject_isEqual(id lhs,
return (lhs == rhs) || [lhs isEqual:rhs];
}

SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_SPI
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, getting rid of that is really nice :-) That halves the number of class lookup-by-string calls we're doing.

@al45tair al45tair changed the title Provide a hook for CF to tell us about the state we need from it, rather than us having to look it up at runtime [Runtime] Let CF provide us with state, avoiding runtime lookup. Jun 21, 2023
@al45tair
Copy link
Contributor

BTW, I tweaked the description of the PR a little; the only thing missing is a Radar number :-) Really nice change.

@Catfish-Man Catfish-Man changed the title [Runtime] Let CF provide us with state, avoiding runtime lookup. [Runtime] Let CF provide us with state, avoiding runtime lookup. rdar://111104451 Jun 21, 2023
@Catfish-Man Catfish-Man merged commit 7eb8283 into swiftlang:main Jun 21, 2023
Catfish-Man added a commit to Catfish-Man/swift that referenced this pull request Jun 22, 2023
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.

2 participants