Skip to content

Prevent needless key/value copies in database methods #163

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

Merged
merged 1 commit into from
Nov 5, 2019

Conversation

victorporof
Copy link
Contributor

@victorporof victorporof commented Nov 3, 2019

Signed-off-by: Victor Porof victor.porof@gmail.com

We're using std::collections::HashMap's Entry API to store keys/values in our safe mode database implementation. This is more ergonomic than using an imperative API, and more performant in the ideal case.

However, the real world isn't ideal, and we end up needlessly reallocating keys and values when interacting with the database. For example, using the Entry API to insert a key/value pair into the database results in a copy of the key when the value was already added. Similarly (in an even worse case scenario), deleting all values matching a key copies the key, and attempts clearing an already empty BTreeSet representing our values.

The Entry API is long known for having poor performance in those situations, with many attempts to fix it. We could opt into using the imperative API instead, but that just makes us less performant in other situations. Luckily, we now have a new RawEntry API, added in rust-lang/rust#54043 giving us the tools required to deal with this problem elegantly.

The RawEntry API is available in nightlies at the moment; however, we can use it in stable via std::collections::HashMap's backing store rust-lang/hashbrown.

Signed-off-by: Victor Porof <victor.porof@gmail.com>
@victorporof victorporof requested a review from ncloudioj November 3, 2019 18:56
Copy link
Member

@ncloudioj ncloudioj left a comment

Choose a reason for hiding this comment

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

LTGM!

Shall we keep a note somewhere so that we could get rid of the hashbrown dependency at some point in the future?

@victorporof
Copy link
Contributor Author

@ncloudioj Yes, sounds good!

@victorporof victorporof changed the base branch from safe-mode-managers to safe-mode November 5, 2019 11:00
@victorporof victorporof merged commit b19a905 into safe-mode Nov 5, 2019
@victorporof victorporof deleted the hashbrown branch November 5, 2019 11:02
victorporof added a commit that referenced this pull request Nov 5, 2019
Prevent needless key/value copies in database methods
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