Eager string bridging #5489
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,164 +20,44 @@ import SwiftShims | |
| /// Effectively an untyped NSString that doesn't require foundation. | ||
| public typealias _CocoaString = AnyObject | ||
|
|
||
| extension String { | ||
| @inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency | ||
| public // SPI(Foundation) | ||
| init(_cocoaString: AnyObject) { | ||
| // If the NSString is actually a Swift String in disguise, | ||
| // we can just copy out the internal repr. | ||
| if let wrapped = _cocoaString as? _NSContiguousString { | ||
| self._core = wrapped._core | ||
| return | ||
| } | ||
|
|
||
| let cocoaString = unsafeBitCast(_cocoaString, to: _NSString.self) | ||
|
|
||
| let length = cocoaString.length() | ||
|
|
||
| // The 0 here is `nullRequired: false` (bad hack) | ||
| let nulTerminatedASCII = cocoaString._fastCStringContents(0) | ||
|
|
||
| let isUTF16 = (nulTerminatedASCII == nil) | ||
| let buffer = _StringBuffer(capacity: length, | ||
| initialSize: length, | ||
| elementWidth: isUTF16 ? 2 : 1) | ||
|
|
||
| // We try our darndest to just get a pointer and memcpy, falling back | ||
| // to having the string do it for us with getCharacters. | ||
| if isUTF16 { | ||
| let bufPtr = buffer.start.assumingMemoryBound(to: UInt16.self) | ||
| if let utf16Ptr = cocoaString._fastCharacterContents() { | ||
| bufPtr.assign(from: utf16Ptr, count: length) | ||
| } else { | ||
| cocoaString.getCharacters(bufPtr) | ||
| } | ||
|
|
||
| } else { | ||
| buffer.start.copyBytes(from: nulTerminatedASCII!, count: length) | ||
| } | ||
|
|
||
| self._core = _StringCore(buffer) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -187,29 +67,16 @@ extension String { | |
| // The @_swift_native_objc_runtime_base attribute | ||
| // This allows us to subclass an Objective-C class and use the fast Swift | ||
| // memory allocator. | ||
| // | ||
| // Subclasses should provide: | ||
| // * func length() -> Int | ||
| // * func characterAtIndex(_ index: Int) -> UInt16 | ||
| @objc @_swift_native_objc_runtime_base(_SwiftNativeNSStringBase) | ||
| public class _SwiftNativeNSString {} | ||
|
|
||
| /// An `NSString` built around a slice of contiguous Swift `String` storage. | ||
| public final class _NSContiguousString : _SwiftNativeNSString, _NSStringCore { | ||
| public init(_ _core: _StringCore) { | ||
| self._core = _core | ||
| super.init() | ||
| } | ||
|
|
@@ -218,16 +85,16 @@ public final class _NSContiguousString : _SwiftNativeNSString { | |
| _sanityCheckFailure("init(coder:) not implemented for _NSContiguousString") | ||
| } | ||
|
|
||
| public func length() -> Int { | ||
| return _core.count | ||
| } | ||
|
|
||
| public func characterAtIndex(_ index: Int) -> UInt16 { | ||
| return _core[index] | ||
| } | ||
|
|
||
| @inline(__always) // Performance: To save on reference count operations. | ||
| public func getCharacters( | ||
| _ buffer: UnsafeMutablePointer<UInt16>, | ||
| range aRange: _SwiftNSRange) { | ||
|
|
||
|
|
@@ -248,7 +115,7 @@ public final class _NSContiguousString : _SwiftNativeNSString { | |
| } | ||
|
|
||
| @objc | ||
| public func _fastCharacterContents() -> UnsafeMutablePointer<UInt16>? { | ||
| return _core.elementWidth == 2 ? _core.startUTF16 : nil | ||
| } | ||
|
|
||
|
|
@@ -313,11 +180,6 @@ extension String { | |
| /// Same as `_bridgeToObjectiveC()`, but located inside the core standard | ||
| /// library. | ||
| public func _stdlib_binary_bridgeToObjectiveCImpl() -> AnyObject { | ||
| return _NSContiguousString(_core) | ||
|
There was a problem hiding this comment. it would be nice if we didn't have such a performance hit here; this will destroy all tagged pointer strings which is a decent memory savings to objc, and it destroys the fast-path accessors we have in Foundation as well. it really needs to implement the _fastCStringContents: as well as fastestEncoding and smallestEncoding. Please reference rdar://problem/18525604 There was a problem hiding this comment. So the fact that we need to create an NsContiguousString is an old bug that needs to be fixed for sure, but if we fix that then Swift->Objc is free. The only impact is at the Objc -> Swift part. It's an often stated claim that we want String to become a single pointer, with tagged pointer optimizations. It's possible we can pick up objc's tagged pointer opts if we go that way? But we need someone to take point on the design here. I know @dabrahams is working on a lot of string stuff, but idk if he's been looking at this aspect of it yet. I'll look into filling in the fastpaths, thanks! There was a problem hiding this comment. Yes, I am looking into that aspect and intend to handle it. |
||
| } | ||
|
|
||
|
|
||
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
Oops, something went wrong.
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.
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.
Just so you know, this is not 100% correct. ObjCBool is only UInt8 on some platforms, and it's theoretically possible that UInt8 and Bool would have different calling conventions.
Internal Clang bug rdar://problem/21170440 may provide a solution we can use in SwiftShims.
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.
Summarizing: this is a bad hack that probably works fine in practice for the time being. When https://reviews.llvm.org/D26234 is merged, we can use that to do the strictly correct thing and provide a
#definein SwiftShims.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.
Further,
_fastCStringContentsmay need to account for the system encoding. See also #3151