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

Optimize IndexOf lookups #19963

Merged
merged 2 commits into from
Jan 19, 2024
Merged

Conversation

symbiogenesis
Copy link
Contributor

This extension method in Core is referenced in at least 19 places.

With this change, the performance in some cases is orders of magnitude better.

https://dotnetfiddle.net/Anb4rA

@symbiogenesis symbiogenesis requested a review from a team as a code owner January 18, 2024 06:37
@ghost ghost added the community ✨ Community Contribution label Jan 18, 2024
@ghost
Copy link

ghost commented Jan 18, 2024

Hey there @symbiogenesis! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@jfversluis jfversluis added the legacy-area-perf Startup / Runtime performance label Jan 18, 2024
jsuarezruiz
jsuarezruiz previously approved these changes Jan 18, 2024
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

It looks like you wrote a manual benchmark instead of using BenchmarkDotNet.

Can you add a benchmark for this change? and add it to this PR?

https://github.com/dotnet/maui/tree/main/src/Core/tests/Benchmarks/Benchmarks

Then let us know your results before/after. Thanks!

@symbiogenesis
Copy link
Contributor Author

symbiogenesis commented Jan 18, 2024

Before:

Method Mean Error StdDev Gen0 Allocated
IndexOfList 5.500 ms 0.1088 ms 0.1990 ms 5093.7500 45.78 MB
IndexOfArray 5.245 ms 0.1018 ms 0.1524 ms 5093.7500 45.78 MB
IndexOfHashSet 5.820 ms 0.1139 ms 0.2595 ms 5093.7500 45.78 MB

After:

Method Mean Error StdDev Gen0 Allocated
IndexOfList 89.60 us 1.777 us 1.975 us - -
IndexOfArray 89.87 us 1.551 us 1.375 us - -
IndexOfHashSet 5,479.40 us 107.325 us 173.310 us 5093.7500 48000043 B

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you!

I will put this on the list for some far future blog post. 😄

@mattleibow
Copy link
Member

/azp run

@mattleibow mattleibow enabled auto-merge (squash) January 19, 2024 07:47
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@mattleibow mattleibow merged commit 9dd1ce2 into dotnet:main Jan 19, 2024
47 checks passed
@symbiogenesis symbiogenesis deleted the indexof-performance branch January 19, 2024 10:31
@github-actions github-actions bot locked and limited conversation to collaborators Feb 19, 2024
@Eilon Eilon added the t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) label May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community ✨ Community Contribution fixed-in-8.0.7 fixed-in-9.0.100-preview.1.9973 legacy-area-perf Startup / Runtime performance t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants