-
Notifications
You must be signed in to change notification settings - Fork 29
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
perf: Entry API POC #251
base: main
Are you sure you want to change the base?
perf: Entry API POC #251
Conversation
d8e5612
to
ff97e3d
Compare
|
Should we maintain backward compatibility by adding a new function like |
I'm concerned that it'd make the API quite convoluted: Every time we add a new function which returns an iterator, we'd need to duplicate it (not even mentioning variants corresponding to I was considering a single iterator API with iterator transformers, like |
Hey David, we've discussed this internally, and we're in favor of adopting this approach, as it clearly brings a significant performance improvement. Do you think it makes sense to discuss the new API with a wider group of developers? |
This is a proposal for preferring iterating over Entries rather than
(K, V)
tuples. Entries provide lazy access to values which means that values don't need to be loaded and deserialized when the caller doesn't explicitly need them (by callingentry.value()
).As the benchmarks show, there is a considerable speed gain in scans, i.e. if some code only needs to access certain values depending on the key. If values are large / deserialization of values is slow, the speed gain can be even higher.
However, this is a breaking change.
Before:
After:
Affected functions:
range()
,iter()
,iter_upper_bound()
The backwards-compatible alternative would be introducing the API under new method names, however that would convolute the API surface.
This PR is not complete yet, tests have been disabled for the time of the discussion.