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

Avoid using moka in the SymbolProvider #1356

Merged
merged 2 commits into from
Jan 31, 2024
Merged

Conversation

Swatinem
Copy link
Member

@Swatinem Swatinem commented Jan 31, 2024

Profiling has shown that moka might be a bit overkill for the SymbolProvider. The SymbolProvider is never used for long term caching, but only as a short term HashMap. This removed moka from this code path, and also avoids a bunch of Clones and other work from the process_minidump flow.

# before
Workload 0 (concurrency: 10): 6746 operations, 224.86667 ops/s
Workload 1 (concurrency: 10): 5405 operations, 180.16667 ops/s
Workload 2 (concurrency: 10): 8014 operations, 267.13333 ops/s

# after
Workload 0 (concurrency: 10): 7610 operations, 253.66667 ops/s
Workload 1 (concurrency: 10): 6075 operations, 202.5 ops/s
Workload 2 (concurrency: 10): 8770 operations, 292.33334 ops/s

Looks like a ~10% win

#skip-changelog

@Swatinem Swatinem requested a review from loewenheim January 31, 2024 14:14
@Swatinem Swatinem self-assigned this Jan 31, 2024
Profiling has shown that `moka` might be a bit overkill for the `SymbolProvider`.
The `SymbolProvider` is never used for long term caching, but only as a short term `HashMap`.
This removed `moka` from this code path, and also avoids a bunch of Clones and other work from the `process_minidump` flow.
@Swatinem Swatinem force-pushed the swatinem/symbolprovider-nomoka branch from b69042c to 73dacd2 Compare January 31, 2024 14:17
Copy link

codecov bot commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (60dec6f) 75.88% compared to head (73dacd2) 75.93%.
Report is 1 commits behind head on master.

❗ Current head 73dacd2 differs from pull request most recent head 227b7c7. Consider uploading reports for the commit 227b7c7 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1356      +/-   ##
==========================================
+ Coverage   75.88%   75.93%   +0.04%     
==========================================
  Files          99       99              
  Lines       14764    14791      +27     
==========================================
+ Hits        11204    11231      +27     
  Misses       3560     3560              

Copy link
Contributor

@loewenheim loewenheim left a comment

Choose a reason for hiding this comment

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

One nit, looks good apart from that.

@Swatinem Swatinem merged commit 202922a into master Jan 31, 2024
13 checks passed
@Swatinem Swatinem deleted the swatinem/symbolprovider-nomoka branch January 31, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants