Skip to content

Optimize IndexOf lookups#19963

Merged
mattleibow merged 2 commits intodotnet:mainfrom
symbiogenesis:indexof-performance
Jan 19, 2024
Merged

Optimize IndexOf lookups#19963
mattleibow merged 2 commits intodotnet:mainfrom
symbiogenesis:indexof-performance

Conversation

@symbiogenesis
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

/azp run

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

Azure Pipelines successfully started running 3 pipeline(s).

@mattleibow mattleibow merged commit 9dd1ce2 into dotnet:main Jan 19, 2024
@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 perf/general 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 perf/general 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