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

Remove double lookups on SizeLimitedCache<K, V>, improve performance #9949

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

h3xds1nz
Copy link
Contributor

@h3xds1nz h3xds1nz commented Oct 13, 2024

Fixes #4828

Description

While checking out SizeLimitedCache<K, V> yesterday, I've noticed the double lookups on the dictionary and today I've found there's already a corresponding issue for it, hence we should fix it for optimal performance as this is used with glyphs/fonts.

  • Use throw helpers over creating new instance of ArgumentNullException directly.
  • Remove had even three lookups, so now it just has one.
  • Fields that could be readonly have been adjusted.
  • Sealed the class, proper naming conventions.
  • Node uses auto properties now.

10 capacity, 15 unique additions first, 20 lookups

Method Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size [B] Allocated [B]
Original 988.4 ns 9.39 ns 7.84 ns 0.1183 2,045 B 1992 B
PR_EDIT 809.3 ns 9.18 ns 8.14 ns 0.1183 1,820 B 1992 B

Customer Impact

Improved perfomance.

Regression

No.

Testing

Local build, tests with the class.

Risk

Low, just basically compiler warnings.

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz requested review from a team as code owners October 13, 2024 11:33
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SizeLimitedCache should use TryGetValue
1 participant