-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[SR-6197] Swift subclass of NSTextStorage is slow because of Swift bridging #48749
Comments
Hmm. The bridging peephole addresses some instances of this class of problem, but not ObjC subclassing or protocol implementation, and it won't be available until Swift 4.1 anyway. So this has to stay open for now. @belkadan, we don't allow methods to be implemented/overridden with their original types, right? |
No, we don't. I suppose we could (by checking bridging conversions, not by checking the original type). |
That gets tricky for further Swift overrides, though. |
I think it would basically require Swift overrides to stick with the original types, which is an unfortunate leak of what's really an implementation detail. I don't know how to do this without substantially changing the code-generation model for Swift overrides/implementations of ObjC methods, though. |
Comment by Ian McDowell (JIRA) Hi guys. Any progress or ideas here? I am also trying to implement a NSTextStorage subclass in an all-swift app, and seeing it taking around 1.5 seconds to update for a new page of text. The vast majority of that time is spent bridging to and from NSDictionary. If I implement the following method in Objective-C, and the rest in a Swift subclass, the execution time for the same operation is under 200 milliseconds. - (NSDictionary<NSAttributedStringKey,id> *)attributesAtIndex:(NSUInteger)location effectiveRange:(NSRangePointer)range; |
We don't have a short-term solution for this, and unfortunately we're likely to stay busy on other things until at least after the fall release. I don't have a better suggestion for you than just implementing the one method in Objective-C. |
There’s a reference to this perhaps being possible to improve once Swift 4.1 arrived. Would now be a good time to revisit this? |
The only reasonable solution to this that I can see is to allow Swift subclasses to define overrides using the original, unbridged types. Unfortunately, it's way too late to be adding significant new generalizations like that in 5.0 — it's very likely that we'll end up chasing the long tail of unanticipated interaction problems in the new feature. |
I think this should be considerably better on current trunk. Not "different big O" better like would be needed to really fix it, but I expect a nice incremental improvement. |
Note that this is not just an issue with subclasses, but also delegate methods, such as NSLayoutManagerDelegate shouldUseTemporaryAttributes. What about the following:
With "just" that I believe the issue can be worked around by developers, for the aforementioned NSTextStorage as follows: Define a protocol and forwarding base class in objc: @interface MyTextStorageImplProtocol
- (NSDictionary<NSAttributedStringKey,id> *)impl_attributesAtIndex:(NSUInteger)location effectiveRange:(NSRangePointer)range NS_SWIFT_UNBRIDGED;
@end
@interface MyTextStorageBase : NSTextStorage
@property (nonatomic, weak) id<MyTextStorageImplProtocol> myImpl;
- (NSDictionary<NSAttributedStringKey,id> *)attributesAtIndex:(NSUInteger)location effectiveRange:(NSRangePointer)range; // @implementation not showed here calls corresponding myImpl method
@end In swift: class MyTextStorage: MyTextStorageBase <MyTextStorageImplProtocol> {
init() {
super.myImpl = self
}
func impl_attributes(at location: Int, effectiveRange range: NSRangePointer?) -> NSDictionary {
// Do your thing
}
...
} How is this better than just defining the full subclass in Obiective-C? Well, assuming the rest of the app is written in Swift, your attributesAtIndex method can now access all your non-objc bridgeable Swift types. While of course not ideal, this provides a escape hatch that is better than waiting for a possible fix in Swift 6. Edit: I realised that a poor-mans version of NS_SWIFT_UNBRIDGED, called NS_NON_BRIDGED already exists in https://github.com/apple/swift/blob/master/stdlib/public/SwiftShims/FoundationShimSupport.h: #ifndef NS_NON_BRIDGED
#define NS_NON_BRIDGED(type) NSObject *
#endif |
Sure, if you're willing to write some ObjC stubs there's definitely a continuum of workarounds. I don't think that changes what the right language direction here is, though. |
Attachment: Download
Environment
Apple Swift version 4.0 (swiftlang-900.0.65.2 clang-900.0.37)
Target: x86_64-apple-macosx10.9
macOS 10.13
Additional Detail from JIRA
md5: d13ea9787b0d44984c429de46b2d6547
Issue Description:
`NSTextStorage` vs. `TextStorageSwiftSubclass`
Calling `-[NSTextStorage attributesAtIndex:effectiveRange:]` is nearly 3 times as slow for `TextStorageSwiftSubclass` compared to `NSTextStorage`. Time profiling shows that this is caused by the bridging of the Objective-C dictionary return value to Swift. Effectively, this means we cannot use Swift if we want to subclass `NSTextStorage`.
Sample project and time profiler trace are attached.
The text was updated successfully, but these errors were encountered: