[SE-0494][StdLib] Add isTriviallyIdentical(to:) Methods to Array, ArraySlice, and ContiguousArray

[vanvoorden]

Background

SE-0494

We propose new isTriviallyIdentical(to:) instance methods to the following concrete types for determining in constant-time if two instances must be equal by-value:

• String
• Substring
• Array
• ArraySlice
• ContiguousArray
• Dictionary
• Set

Instead of “one big diff”… we can try and keep the diffs grouped together by similar functionality:

• String, Substring
• Array, ArraySlice, ContiguousArray
• Dictionary, Set

Changes

Our Array already performs a "fast path" for equality checking in our == operator.¹ We can implement a similar check for isIdentical:

extension Array { @_alwaysEmitIntoClient public func isTriviallyIdentical(to other: Self) -> Bool { self._buffer.identity == other._buffer.identity } }

There are similar fast paths in ArraySlice and ContiguousArray.²³

Test Plan

New tests were added for Array, ArraySlice, and ContiguousArray.

Benchmarks

New benchmarks were added for Array, ArraySlice, and ContiguousArray.

Footnotes

1. https://github.com/swiftlang/swift/blob/swift-6.1.2-RELEASE/stdlib/public/core/Array.swift#L1816-L1824 ↩

2. https://github.com/swiftlang/swift/blob/swift-6.1.2-RELEASE/stdlib/public/core/ArraySlice.swift#L1398-L1406 ↩

3. https://github.com/swiftlang/swift/blob/swift-6.1.2-RELEASE/stdlib/public/core/ContiguousArray.swift#L1335-L1343 ↩

[Azoy]

I would prefer this (and everything else in this PR) to be @_alwaysEmitIntoClient instead.

[vanvoorden]

@Azoy TBH… I have zero experience with either option. From reading through SE-0376 it sounds like backDeployed came with some advantages:

While @_alwaysEmitIntoClient can be used to back deploy APIs, there are some drawbacks to using it. Since a copy of the function is always emitted, there is code size overhead for every client even if the client's deployment target is new enough that the library API would always be available at runtime. Additionally, if the implementation of the API were to change in order to improve performance, fix a bug, or close a security hole then the client would need to be recompiled against a new SDK before users benefit from those changes.

I don't see much discussion in that proposal over when a library maintainer would still prefer _alwaysEmitIntoClient.

I did find this discussion from @lorentey:

#75433 (comment)

It sounds like one issue here was that debugDescription was used to conform to a protocol. Our isIdentical function here would not be used to conform to a protocol. It sounds like that might make safer shipping backDeployed?

But we did leave a FIXME comment:

e39613b

That we eventually want to make this backDeployed.

Hmm… would you have any more specific ideas why we prefer _alwaysEmitIntoClient here for these changes?

[Azoy]

Because @backDeployed commits this as the stdlib's ABI vs. @_aEIC which does not. If we find we need to replace this in the future with some more generalized thing or such, we pay the price of having to maintain this forever instead of just being able to update the definition. @_aEIC is the best attribute in my opinion because it is the "pay for what you use" attribute both for the stdlib and the client. The stdlib doesn't have to take the code size hit (unless it started using it in its own opaque implementation) or the ABI hit, and clients don't pay for anything unless they use it themselves or use something that uses it.

There's also the fact that @backDeployed introduces a runtime availability check for some configurations which does have a performance cost vs. @_aEIC which does not.

[vanvoorden]

@Azoy SGTM. I'll make the changes and push a new commit. Thanks!

[vanvoorden]

@Azoy would you have any opinion about #82055 and #82439? Same tradeoff for preferring aEIC on those?

[vanvoorden]

@lorentey Did we want to ship these opaque?

[lorentey]

That would prevent generic specialization, so the answer is no, sadly.

[vanvoorden]

@lorentey I think we have to check the count here… or else we could have the same buffer with two different ranges that compare as identical. But if the buffer identity itself already defends against that then I can remove this extra check.

[lorentey]

This does appear sound for all cases I've seen in practice -- the buffer identity is simply the address of its first element, and including the count guarantees that the two slices are backed by the same region of memory.

But two slices being backed by the same memory does not necessarily imply that they must be identical slices. It does leave me worried that this shortcut may misfire in some exotic/hypothetical cases, such as if two distinct bridged read-only NSArray instances somehow end up getting backed by overlapping memory regions, and therefore two array slices may be backed by the same memory, but still be distinguishable -- for example, by looking at their startIndex. I'm not aware of any actual NSArray subclass that would do that, but I do believe it is technically possible.

To avoid this (and (perhaps more practical) similar cases I may be missing), I think we should just follow best practice and simply compare all stored properties, with no clever shortcuts, like in the draft below:

extension _SliceBuffer { @_alwaysEmitIntoClient internal func isTriviallyIdentical(to other: Self) -> Bool { self.owner == other.owner && self.subscriptBaseAddress == other.subscriptBaseAddress && self.startIndex == other.startIndex && self.endIndexAndFlags == other.endIndexAndFlags } } extension ArraySlice { @_alwaysEmitIntoClient public func isTriviallyIdentical(to other: Self) -> Bool { self._buffer.isTriviallyIdentical(to: other._buffer) } } 

(Note that this only applies to array slices; I believe comparing _buffer.identity will suffice for Array and ContiguousArray. In the bridged case, the identity is simply the object identity of the NSArray instance.)

[vanvoorden]

@lorentey Ahh… that's a good idea! I'll push a new commit. Thanks!

[vanvoorden]

Hmm… I'm running into trouble now trying to compare self.owner to other.owner because of the Builtin.NativeObject type…

[vanvoorden]

https://github.com/swiftlang/swift/blob/swift-6.2-RELEASE/stdlib/public/core/Builtin.swift#L135-L140

I think I need this one… but for some reason I'm not able to call it without a compiler error…

[vanvoorden]

@swift-ci Please smoke test