-
Notifications
You must be signed in to change notification settings - Fork 191
Add a fast-path for equality checking when the backing is empty or pointer equal #1322
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2709,14 +2709,36 @@ public struct Data : Equatable, Hashable, RandomAccessCollection, MutableCollect | |
/// Returns `true` if the two `Data` arguments are equal. | ||
@inlinable // This is @inlinable as emission into clients is safe -- the concept of equality on Data will not change. | ||
public static func ==(d1 : Data, d2 : Data) -> Bool { | ||
// See if both are empty | ||
switch (d1._representation, d2._representation) { | ||
case (.empty, .empty): | ||
return true | ||
default: | ||
// Continue on to checks below | ||
break | ||
} | ||
|
||
let length1 = d1.count | ||
if length1 != d2.count { | ||
let length2 = d2.count | ||
|
||
// Unequal length data can never be equal | ||
guard length1 == length2 else { | ||
return false | ||
} | ||
|
||
if length1 > 0 { | ||
return d1.withUnsafeBytes { (b1: UnsafeRawBufferPointer) in | ||
return d2.withUnsafeBytes { (b2: UnsafeRawBufferPointer) in | ||
return memcmp(b1.baseAddress!, b2.baseAddress!, b2.count) == 0 | ||
// If they have the same base address and same count, it is equal | ||
let b1Address = b1.baseAddress! | ||
let b2Address = b2.baseAddress! | ||
|
||
guard b1Address != b2Address else { | ||
return true | ||
} | ||
Comment on lines
+2736
to
+2738
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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 This comment from @lorentey might have some clues:
For a little extra context… I would eventually like to pitch a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only thing I'm not sure about there is if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This particular use of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's right. For There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ahh… good to know! Thanks! A documentation nit at that point would be to actually add a |
||
|
||
// Compare the contents | ||
return memcmp(b1Address, b2Address, b2.count) == 0 | ||
} | ||
} | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.