Call experimental new clock cache HyperClockCache #10684
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: This change establishes a distinctive name for the experimental new lock-free clock cache (originally developed by @guidotag and revamped in PR facebook#10626). A few reasons: * We want to make it clear that this is a fundamentally different implementation vs. the old clock cache, to avoid people saying "I already tried clock cache." * We want to highlight the key feature: it's fast (especially under parallel load) * Because it requires an estimated charge per entry, it is not drop-in API compatible with old clock cache. This estimate might always be required for highest performance, and giving it a distinct name should reduce confusion about the distinct API requirements. * We might develop a variant requiring the same estimate parameter but with LRU eviction. In that case, using the name HyperLRUCache should make things more clear. (FastLRUCache is just a prototype that might soon be removed.) Some API detail: * To reduce copy-pasting parameter lists, etc. as in LRUCache construction, I have a `MakeSharedCache()` function on `HyperClockCacheOptions` instead of `NewHyperClockCache()`. * Changes -cache_type=clock_cache to -cache_type=hyper_clock_cache for applicable tools. I think this is more consistent / sustainable for reasons already stated. I'm not yet announcing this in HISTORY.md because it is considered experimental and I haven't put all the advantages and caveats in the public API yet. Test Plan: no interesting functional changes; tests updated
|
@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
How about OverclockCache then? :) |
|
@pdillinger has updated the pull request. You must reimport the pull request before landing. |
|
@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary: This change establishes a distinctive name for the experimental new lock-free clock cache (originally developed by @guidotag and revamped in PR #10626). A few reasons:
Some API detail:
MakeSharedCache()function onHyperClockCacheOptionsinstead ofNewHyperClockCache().For performance tests, see #10626
Test Plan: no interesting functional changes; tests updated