Skip to content
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.

Sign up for GitHub

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

Improve Guid equality checks on 64-bit platforms #35654

Closed
wants to merge 1 commit into from
Closed

Improve Guid equality checks on 64-bit platforms #35654

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 8 additions & 16 deletions src/libraries/System.Private.CoreLib/src/System/Guid.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,19 +789,15 @@ public override bool Equals(object? o)
else g = (Guid)o;

// Now compare each of the elements
return g._a == _a &&
Unsafe.Add(ref g._a, 1) == Unsafe.Add(ref _a, 1) &&
Unsafe.Add(ref g._a, 2) == Unsafe.Add(ref _a, 2) &&
Unsafe.Add(ref g._a, 3) == Unsafe.Add(ref _a, 3);
return Unsafe.As<int, long>(ref g._a) == Unsafe.As<int, long>(ref _a) &&
Unsafe.As<byte, long>(ref g._d) == Unsafe.As<byte, long>(ref _d);
}

public bool Equals(Guid g)
{
// Now compare each of the elements
return g._a == _a &&
Unsafe.Add(ref g._a, 1) == Unsafe.Add(ref _a, 1) &&
Unsafe.Add(ref g._a, 2) == Unsafe.Add(ref _a, 2) &&
Unsafe.Add(ref g._a, 3) == Unsafe.Add(ref _a, 3);
return Unsafe.As<int, long>(ref g._a) == Unsafe.As<int, long>(ref _a) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this logic need to be repeated in multiple places rather than have everything forward to == for example?

I'd hope the JIT would properly inline the check in all the cases and the code would be equivalent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, although that's a separate issue.

Unsafe.As<byte, long>(ref g._d) == Unsafe.As<byte, long>(ref _d);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that Int64-based operations would be slower than Int32-based operations on 32-bit, especially if the values weren't aligned (and that that's why @jkotas used Int32 here initially when switching these operations to be based on Unsafe). Is that not the case?

}

private int GetResult(uint me, uint them) => me < them ? -1 : 1;
Expand Down Expand Up @@ -937,17 +933,13 @@ public int CompareTo(Guid value)
}

public static bool operator ==(Guid a, Guid b) =>
a._a == b._a &&
Unsafe.Add(ref a._a, 1) == Unsafe.Add(ref b._a, 1) &&
Unsafe.Add(ref a._a, 2) == Unsafe.Add(ref b._a, 2) &&
Unsafe.Add(ref a._a, 3) == Unsafe.Add(ref b._a, 3);
Unsafe.As<int, long>(ref a._a) == Unsafe.As<int, long>(ref b._a) &&
Unsafe.As<byte, long>(ref a._d) == Unsafe.As<byte, long>(ref b._d);

public static bool operator !=(Guid a, Guid b) =>
// Now compare each of the elements
a._a != b._a ||
Unsafe.Add(ref a._a, 1) != Unsafe.Add(ref b._a, 1) ||
Unsafe.Add(ref a._a, 2) != Unsafe.Add(ref b._a, 2) ||
Unsafe.Add(ref a._a, 3) != Unsafe.Add(ref b._a, 3);
Unsafe.As<int, long>(ref a._a) != Unsafe.As<int, long>(ref b._a) ||
Unsafe.As<byte, long>(ref a._d) != Unsafe.As<byte, long>(ref b._d);

public string ToString(string? format)
{
Expand Down