Skip to content

Replacement of KeyValueStoreView by a direct version. #4057

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

MathieuDutSik
Copy link
Contributor

Motivation

The existing KeyValueStoreView is a scalable design for implementing a Key-Value store, which is needed by the EVM smart contracts and the Wasm smart contracts. However, it is a slow design that is overkill for 99% of the use cases.

Replacing it with an enum Big/Small might be a solution but it presents an inconvenient that we cannot easily transition from a Big sdesign to a small design if the storage use decreases. Also:

  • It creates a strong discontinuity in the usage performance near the chosen threshold.
  • Anyway, when we compute the hash, we have to access the whole set of key/values of the KeyValueStoreView.

Proposal

The following is done:

  • The whole Key/value store is replaced by a single BTreeMap<Vec<u8>, vec<u8>>.
  • Therefore, the API becomes a RegisterView, and this container is chosen accordingly. Note that this doubles the memory usage, but this was already the case for the small container in the now-closed PR.
  • We cannot use the derive macro in linera-views. Therefore, the file key_value_store_view.rs is moved to linera-execution.
  • The ViewContainer is eliminated and with it the corresponding tests.

Test Plan

The CI was adequate for identifying problems with the design.

Release Plan

  • Nothing to do / These changes follow the usual release cycle.

Links

None.

@MathieuDutSik MathieuDutSik marked this pull request as ready for review June 4, 2025 18:39
@ma2bd
Copy link
Contributor

ma2bd commented Jun 4, 2025

The existing KeyValueStoreView is a scalable design for implementing a Key-Value store

Except that hashing is going to force reading everything anyway

@ma2bd ma2bd self-requested a review June 4, 2025 21:10
// Copyright (c) Zefchain Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

//! We implement a `KeyValueStoreView` implements View and the function of `KeyValueStore`.
Copy link
Contributor

@ma2bd ma2bd Jun 4, 2025

Choose a reason for hiding this comment

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

This doesn't belong here, does it? Especially if we want to keep the unit tests.

Copy link
Contributor

@ma2bd ma2bd Jun 4, 2025

Choose a reason for hiding this comment

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

Also it could be called SmallKeyValueStoreView if we want to keep the old implementation around for performance comparison (which are not done in this PR)

Copy link
Contributor

@ma2bd ma2bd left a comment

Choose a reason for hiding this comment

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

As discussed, we'd like to

  • keep the old KVSView in place
  • have the new "small" data-structure in linera-view as well (if possible -- otherwise maybe that's ok for now).
  • make the change in linera-execution later, when we have benchmarks

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