-
Notifications
You must be signed in to change notification settings - Fork 207
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? |
| internal static let fileIDPrefix = Array("/.file/id=".utf8) | ||
|
|
||
| #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
NSURLandCFURLto use the SwiftURLimplementation. With this refactoring, the behavior ofURLshould 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:NSURLs, but notURLs,NSURLs, andNSURLto 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 URLto wrap a single class that implements each of its methods. InFOUNDATION_FRAMEWORK, the inner class types ofURLconform to a new_URLProtocol. OutsideFOUNDATION_FRAMEWORK, only_SwiftURLis used, so the protocol is not needed.Note: Except for
baseURL, a nilURL?return value in the protocol means thatstruct URLshould returnself.class _SwiftURLprovides the new Swift implementation forURL, using the same parser andURLParseInfoasURLComponents, but with a few compatibility behaviors.class _BridgedURLwraps anNSURLreference. Its methods use the old implementations, which call directly intoNSURLmethods._BridgedURLis used when anNSURLsubclass 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 theNSURLsubclass like we did before._BridgedURLalso helps us to easily compare the results of the old and newURLimplementation.Note: If the
NSURLsubclass does not override a method,NSURLwill call into the underlying_SwiftURLimplementation.class _BridgedNSSwiftURLwraps an_NSSwiftURL, which is the Swift subclass ofNSURL._BridgedNSSwiftURLis used when an_NSSwiftURLis bridged to Swift, allowing us to return the same object (pointer) when bridging back to ObjC, such as in cases where.absoluteURLshould returnself, or.baseURLshould return a pointer to the sameNSURLfrom initialization. At the same time, this still allows us to use the new_SwiftURLforNSURLs bridged to Swift.