-
Notifications
You must be signed in to change notification settings - Fork 190
Support NS/CFURL re-core in Swift #1238
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
Conversation
@swift-ci please test |
@swift-ci please test |
I understand the goals of this change and I haven't looked at the detailed implementation but the following sounds problematic to me:
Since this PR also introduces differences between platforms. Is there any resulting difference in top-level-API behaviour between platforms? |
#if FOUNDATION_FRAMEWORK | ||
private static var _type: any _URLProtocol.Type { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the performance impacts of this and other existential on Darwin platforms?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

I tested the URL benchmarks in this repo before and after the change. Creating a URL
from a valid URL string, e.g.
blackHole(URL(string: validURLString))
is 47% faster, partly since we now lazily create an NSURL
if we're actually going to bridge instead of up front.
Interestingly, getting encoded components, e.g.
blackHole(encodedURL.scheme)
blackHole(encodedURL.user())
blackHole(encodedURL.password())
blackHole(encodedURL.host())
blackHole(encodedURL.path())
blackHole(encodedURL.query())
blackHole(encodedURL.fragment())
is 195% faster now, even with just calling nearly identical code on the stored any _URLProtocol & AnyObject
. So in these cases, I think the existential may have performance benefit over the old architecture.
Parsing an invalid port URL to return nil
is likely regressed in part due to an additional compatibility check for URL
to allow an empty port. Maybe we're also allocating the class before we know we're just going to return nil
? Regardless, parsing an invalid URL string is probably something we don't need to optimize for.
We could try storing some ranges/data directly in the
There's no differences in behavior for |
…stPathComponent edge cases
@swift-ci please test |
@swift-ci please test |
@swift-ci please test |
@swift-ci please test |
Thank you! |
Will merge after Swift smoke tests pass with this branch at swiftlang/swift#75354 |
Changes to allow
NSURL
andCFURL
to use the SwiftURL
implementation. With this refactoring, the behavior ofURL
should be identical, except for a few minor compatibility/bug fixes to the parser and URL functions, revealed by NS/CFURL test failures with the re-core enabled. The overall architecture has a bit of complexity in order to support:NSURL
s, but notURL
s,NSURL
s, andNSURL
to Swift while maintaining the same object pointer returned by functions like.absoluteURL
(returningself
) and.baseURL
, which is the previous behavior, and one I believe could have implications for reference counting in ObjC/C land.This PR changes
struct URL
to wrap a single class that implements each of its methods. InFOUNDATION_FRAMEWORK
, the inner class types ofURL
conform to a new_URLProtocol
. OutsideFOUNDATION_FRAMEWORK
, only_SwiftURL
is used, so the protocol is not needed.Note: Except for
baseURL
, a nilURL?
return value in the protocol means thatstruct URL
should returnself
.class _SwiftURL
provides the new Swift implementation forURL
, using the same parser andURLParseInfo
asURLComponents
, but with a few compatibility behaviors.class _BridgedURL
wraps anNSURL
reference. Its methods use the old implementations, which call directly intoNSURL
methods._BridgedURL
is used when anNSURL
subclass is bridged to Swift, allowing us to 1) return the same subclass object when bridging back to ObjC and 2) call methods that are overridden by theNSURL
subclass like we did before._BridgedURL
also helps us to easily compare the results of the old and newURL
implementation.Note: If the
NSURL
subclass does not override a method,NSURL
will call into the underlying_SwiftURL
implementation.class _BridgedNSSwiftURL
wraps an_NSSwiftURL
, which is the Swift subclass ofNSURL
._BridgedNSSwiftURL
is used when an_NSSwiftURL
is bridged to Swift, allowing us to return the same object (pointer) when bridging back to ObjC, such as in cases where.absoluteURL
should returnself
, or.baseURL
should return a pointer to the sameNSURL
from initialization. At the same time, this still allows us to use the new_SwiftURL
forNSURL
s bridged to Swift.