Add a fast-path for equality checking when the backing is empty or pointer equal #1322
Conversation
| let b1Address = b1.baseAddress! | ||
| let b2Address = b2.baseAddress! | ||
|
|
||
| if b1Address == b2Address { |
There was a problem hiding this comment.
For stylistic consistency with your edit above, could guard here and early exit :)
|
Simplified benchmark output: |
|
@swift-ci test |
|
You can see from the benchmark output above that the time is about the same now for the three differently-sized datas that are equal, and slows for larger data that is not equal (note the difference in scaling). |
parkera
force-pushed
the
parkera/data_equal_efficiency
branch
from
962d303 to
7ea7b31
Compare
|
@swift-ci test |
| guard b1Address != b2Address else { | ||
| return true | ||
| } |
There was a problem hiding this comment.
@parkera Ahh… very nice!
This looks like an impactful win and I don't want to block this from landing… but I did have some more "long tail" questions about what kind of runtime and space complexity we might be looking at from this approach.
Do what extent (if any) do we know with probability one that withUnsafeBytes can return in constant time and constant space? If withUnsafeBytes is ever performing linear work… this diff is not a "regression" (because we are currently shipping linear work in production anyway). But we are potentially losing a little bit of extra performance.
This comment from @lorentey might have some clues:
[T]he
withUnsafeBytes(of:)invocations […] often result in binaries that make several full copies of the input arguments. A clever enough optimizer might get rid of those, but I do not think ours is clever enough for that -- at least not in general.)
For a little extra context… I would eventually like to pitch a public method on Data that exposes "identity equality" on its own without falling back to a linear time comparison. That method should return in constant time… if it ever must perform a linear amount of work to determine identity equality then it's no longer impactful to have that extra method.
There was a problem hiding this comment.
The only thing I'm not sure about there is if withUnsafeBytes ever has to move bytes into some temporary storage. I don't believe that is currently the case, because Data is contiguous (although DataProtocol is not).
There was a problem hiding this comment.
This particular use of withUnsafeBytes is from the type that owns its own implementation—advice about the use of such APIs in general are not so applicable because we're not solely relying on the API guarantee; rather, the function call (which is inlinable, unless I'm mistaken) is just code organization.
There was a problem hiding this comment.
That's right. For Data, this is always O(1).
There was a problem hiding this comment.
For Data, this is always O(1).
Ahh… good to know! Thanks!
A documentation nit at that point would be to actually add a Complexity callout to Data.withUnsafeBytes reminding engineers this should operate in constant time and space.
|
@swift-ci test |
From a discussion here:
https://forums.swift.org/t/could-data-ship-a-fast-path-check-for-identity-equality/80171
Credit to vanvoorden for looking into this.