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

Benchmarks Introduction #135

Merged
merged 8 commits into from
Oct 21, 2020

Conversation

svengeance
Copy link
Contributor

@svengeance svengeance commented Sep 27, 2020

Fixes #127

This PR uses BenchmarkDotNet to introduce benchmarks for the public API of IAppCache, in addition to a couple "integration" scenarios that tie together what should be common use-cases of the program.

Something I purposefully didn't test is the eviction process - since we currently are tied to .NET's MemoryCache for Cache Entry compaction and eviction, I didn't feel it was necessarily our job to test that. Just my 2c though, and I can add it if you'd like.

Lastly, I wrote the README from a generic owner point of view - please modify it as you see fit to fit your preferred style, particularly the Contributing section.

Copy link
Contributor

@jnyrup jnyrup left a comment

Choose a reason for hiding this comment

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

Really impressive work 👍

An idea for future refinements.
Would it be interesting to distinguish the two cases?

  • The key already exists in the cache
  • The key does not exist in the cache

LazyCache.Benchmarks/MemoryCacheBenchmarks.cs Outdated Show resolved Hide resolved
LazyCache.Benchmarks/MemoryCacheBenchmarks.cs Outdated Show resolved Hide resolved
svengeance and others added 3 commits September 27, 2020 13:58
@svengeance
Copy link
Contributor Author

Updated benchmarks to incorporate distinctions between cache hits & misses. Notably, there is a ~20% difference in performance between a hit/miss for the Get methods, however the other methods have a negligible difference.

@alastairtree
Copy link
Owner

This looks great, will try and merge this week.

@svengeance
Copy link
Contributor Author

@alastairtree Anything else you need on this?

@alastairtree
Copy link
Owner

No it's awesome, thanks. Just not got round to merging!

@alastairtree alastairtree merged commit 8d4b269 into alastairtree:master Oct 21, 2020
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.

Add benchmarks using benchmark dot net
3 participants