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.

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

Vectorize Guid equality. #66889

Merged
merged 2 commits into from
Apr 25, 2022
Merged

Conversation

madelson
Copy link
Contributor

@madelson madelson commented Mar 19, 2022

I was looking into some of the new Vector capabilities and came across this potential use-case.

When I originally benchmarked this change I was using a copy of the current EqualsCore method compared to the new method on .NET 6. There I saw substantial improvement (75%) on both the equal and not equal cases. However, with the dotnet/performance repo running directly against my local build of corerun.exe the results are much less impressive:

Before

Method Mean Error StdDev Median Min Max Allocated
EqualsSame 2.839 ns 0.0255 ns 0.0226 ns 2.836 ns 2.806 ns 2.883 ns -
EqualsDifferent 1.813 ns 0.0202 ns 0.0179 ns 1.810 ns 1.792 ns 1.856 ns -
EqualsOperator 3.350 ns 0.0259 ns 0.0216 ns 3.345 ns 3.323 ns 3.397 ns -
NotEqualsOperator 3.326 ns 0.0388 ns 0.0324 ns 3.332 ns 3.237 ns 3.360 ns -

After

Method Mean Error StdDev Median Min Max Allocated
EqualsSame 1.814 ns 0.0321 ns 0.0268 ns 1.822 ns 1.747 ns 1.839 ns -
EqualsDifferent 2.388 ns 0.1811 ns 0.2085 ns 2.309 ns 2.160 ns 2.886 ns -
EqualsOperator 2.459 ns 0.1477 ns 0.1700 ns 2.435 ns 2.245 ns 2.841 ns -
NotEqualsOperator 2.243 ns 0.0391 ns 0.0347 ns 2.238 ns 2.169 ns 2.298 ns -

Benchmark Info

BenchmarkDotNet=v0.13.1.1716-nightly, OS=Windows 10 (10.0.19043.1586/21H1/May2021Update)
Intel Core i7-3632QM CPU 2.20GHz (Ivy Bridge), 1 CPU, 8 logical and 4 physical cores
.NET SDK=7.0.100-preview.2.22153.17
  [Host]     : .NET 7.0.0 (7.0.22.15202), X64 RyuJIT
  Job-NGABWR : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT

Is it worth it?

This shows EqualsSame improving by 36%, while EqualsDifferent gets worse by 32%. This makes sense because the previous implementation would short-circuit if the first 32 bits didn't match.

I'm curious which case matters more. For Guids used as dictionary keys, I'd think that EqualsSame should dominate. Are there other common scenarios for equating Guids?

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Mar 19, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Mar 19, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

I was looking into some of the new Vector capabilities and came across this potential use-case.

Using BenchmarkDotNet on my machine (Win 10 x64 Ivy bridge) this results in a ~75% speedup vs. the current approach whether or not the Guids are equal.

Author: madelson
Assignees: -
Labels:

area-System.Runtime, community-contribution

Milestone: -

@gfoidl
Copy link
Member

gfoidl commented Mar 20, 2022

For reference: #53012 (comment)

@danmoseley
Copy link
Member

Nice. Is this ready for review? Could you share the benchmark numbers you gathered?

@madelson
Copy link
Contributor Author

madelson commented Mar 20, 2022

Nice. Is this ready for review? Could you share the benchmark numbers you gathered?

@danmoseley I had created as a draft since I wanted to see if this triggered any CI failures. I ran some ad-hoc benchmarks but I plan go through the suggested benchmarking workflow and update with the numbers from that before removing the draft tag.

Given the suggestions on unaligned reads I'll want to rerun my numbers anyway in case that affects things.

Update: ready now, benchmarks posted.

Feedback from dotnet#66889 (comment)

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@tannergooding
Copy link
Member

tannergooding commented Apr 25, 2022

I should've caught that the CI run was slightly out of date.

The removal of using System.Numerics; is a build break since IComparisonOperators was moved to this namespace.

I have a fix up here: #68494

@AndyAyersMS
Copy link
Member

Improvements seen on windows arm64: dotnet/perf-autofiling-issues#4993

@ghost ghost locked as resolved and limited conversation to collaborators May 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants