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-6826] [StringGuts] Side-table deallocation #49375
Comments
This lies at the intersections of ObjC, Swift, and the ObjC runtime. /// A shadow for the "core operations" of NSString.
///
/// Covers a set of operations everyone needs to implement in order to
/// be a useful `NSString` subclass.
@objc
public protocol _NSStringCore : _NSCopying /* _NSFastEnumeration */ { ... } // At runtime, this class is derived from `_SwiftNativeNSStringBase`,
// which is derived from `NSString`.
//
// The @_swift_native_objc_runtime_base attribute
// This allows us to subclass an Objective-C class and use the fast Swift
// memory allocator.
@_fixed_layout // FIXME(sil-serialize-all)
@objc @_swift_native_objc_runtime_base(_SwiftNativeNSStringBase)
public class _SwiftNativeNSString {
@_versioned // FIXME(sil-serialize-all)
@objc
internal init() {}
deinit {}
} And our storage classes inherit (through 1 intermediary) from _SwiftNativeNSString and conform to _NSStringCore. swift_deallocClassInstance calls objc_destructInstance which calls objc_object::sidetable_clearDeallocating(). @belkadan, @gparker42, or @DougGregor Any ideas how we can inherit from NSString for fast bridging, but not go down this deallocation slow path? |
@swift-ci create |
Nothing from me. I figure this is required if someone's done something terrible like stuck an associated object or a weak reference to the string backing, but I would hope we could avoid it in the common case. This is ultimately a Greg question, though—it might mean conditionally avoiding calling objc_destructInstance altogether. |
@Catfish-Man or @mikeash, either of you interested in holding on to this? My understanding is that this can be improved with OS integration. |
Possibly yes! It's tricky though. Mike has been more directly involved, but if he doesn't want it I'll take it. |
I think the right solution here will be for the ObjC runtime to track has-sidetableness (there has to be a better term for that) on a class-by-class basis for classes that don't use non-pointer ISA. (That's tracked at rdar://problem/45661834 for those of you who can view it.) Right now, we track this at the class level on platforms that don't support non-pointer ISA, and at the object level on platforms that do. Tracking it at the object level is a lot better, but doesn't work for classes that opt out of non-pointer ISA, which includes all Swift classes. This scheme doesn't currently fall back to tracking at the class level, but it probably should. David, please feel free to take a shot at this. I'd like to do it but I have a lot of other stuff that needs doing first. |
Thoughts after brainstorming with Karoy and Michael: * String backing stores are probably safe to set the existing forbid-assoc-objects bit on: evidence being that anyone relying on being able to set associated objects on arbitrary NSStrings has already likely had their code broken by tagged pointer strings, and having to hit such a code path with a bridged Swift String just makes it all the less likely * Non-verbatim-bridged collection backing stores are definitely safe to set the existing forbid-assoc-objects bit on, but unfortunately they currently share a class with verbatim-bridged ones. We can likely split those cases up, but will have to be careful about back deployment. The nice thing about these two cases is that, assuming we're correct, they work without requiring another bit in the class. We do still need objc runtime changes to actually check that bit in the dealloc path though, and we need to be careful not to slow down the other cases too much. |
Hm. Why is the latter okay? Can't people rely on the identity of an NSArray once they have it? |
Non-verbatim-bridged Array buffers are not accessible from ObjC (we make a new object when bridging). That said, I may have found a better approach that sidesteps this. |
(Specifically: we have half a bit left in the refcount) |
Fix landed. Requires changes to libobjc to take effect. |
Additional Detail from JIRA
md5: 7066f482bb79190efe2bba3a5936697a
Issue Description:
Regression from #14046
This might be ABI affecting.
When we destroy our storage classes, we spend time consulting side-tables in the objc runtime. This is likely because we want to bridge to NSString fast and our classes derive from NSString at runtime. However, we shouldn’t’ have to pay this cost.
Applies to all StringBuilder benchmarks,
ErrorHandling
This might not be the only culprit of slowdown here. Append may just be slower, and there might be something to explore there. If net regressions persist, please file a new JIRA for them.
The text was updated successfully, but these errors were encountered: