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.

Sign up for GitHub

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

db: use ByteView in KV StateCache #2141

Merged
merged 1 commit into from
Jun 25, 2024
Merged

db: use ByteView in KV StateCache #2141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions silkworm/db/kv/api/base_transaction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,13 @@ Task<Bytes> BaseTransaction::get_one_impl_with_cache(const std::string& table, B
if (table == db::table::kPlainStateName) {
std::shared_ptr<StateView> view = state_cache_->get_view(*this);
if (view != nullptr) {
// TODO(canepat) remove key copy changing DatabaseReader interface
const auto value = co_await view->get(silkworm::Bytes{key.data(), key.size()});
const auto value = co_await view->get(key);
co_return value ? *value : silkworm::Bytes{};
}
} else if (table == db::table::kCodeName) {
std::shared_ptr<StateView> view = state_cache_->get_view(*this);
if (view != nullptr) {
// TODO(canepat) remove key copy changing DatabaseReader interface
const auto value = co_await view->get_code(silkworm::Bytes{key.data(), key.size()});
const auto value = co_await view->get_code(key);
co_return value ? *value : silkworm::Bytes{};
}
}
Expand Down
26 changes: 14 additions & 12 deletions silkworm/db/kv/api/state_cache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ namespace silkworm::db::kv::api {

CoherentStateView::CoherentStateView(Transaction& txn, CoherentStateCache* cache) : txn_(txn), cache_(cache) {}

Task<std::optional<Bytes>> CoherentStateView::get(const Bytes& key) {
Task<std::optional<Bytes>> CoherentStateView::get(ByteView key) {
co_return co_await cache_->get(key, txn_);
}

Task<std::optional<Bytes>> CoherentStateView::get_code(const Bytes& key) {
Task<std::optional<Bytes>> CoherentStateView::get_code(ByteView key) {
co_return co_await cache_->get_code(key, txn_);
}

Expand Down Expand Up @@ -156,7 +156,7 @@ void CoherentStateCache::process_storage_change(CoherentStateRoot* root, StateVi
}
}

bool CoherentStateCache::add(const KeyValue& kv, CoherentStateRoot* root, StateViewId view_id) {
bool CoherentStateCache::add(KeyValue&& kv, CoherentStateRoot* root, StateViewId view_id) {
auto [it, inserted] = root->cache.insert(kv);
SILK_DEBUG << "Data cache kv.key=" << to_hex(kv.key) << " inserted=" << inserted << " view=" << view_id;
std::optional<KeyValue> replaced;
Expand All @@ -173,7 +173,7 @@ bool CoherentStateCache::add(const KeyValue& kv, CoherentStateRoot* root, StateV
state_evictions_.remove(*replaced);
SILK_DEBUG << "Data evictions removed replaced.key=" << to_hex(replaced->key);
}
state_evictions_.push_front(kv);
state_evictions_.push_front(std::move(kv));

// Remove the longest unused key-value pair when size exceeded
if (state_evictions_.size() > config_.max_state_keys) {
Expand All @@ -186,7 +186,7 @@ bool CoherentStateCache::add(const KeyValue& kv, CoherentStateRoot* root, StateV
return inserted;
}

bool CoherentStateCache::add_code(const KeyValue& kv, CoherentStateRoot* root, StateViewId view_id) {
bool CoherentStateCache::add_code(KeyValue&& kv, CoherentStateRoot* root, StateViewId view_id) {
auto [it, inserted] = root->code_cache.insert(kv);
SILK_DEBUG << "Code cache kv.key=" << to_hex(kv.key) << " inserted=" << inserted << " view=" << view_id;
std::optional<KeyValue> replaced;
Expand All @@ -203,7 +203,7 @@ bool CoherentStateCache::add_code(const KeyValue& kv, CoherentStateRoot* root, S
code_evictions_.remove(*replaced);
SILK_DEBUG << "Code evictions removed replaced.key=" << to_hex(replaced->key);
}
code_evictions_.push_front(kv);
code_evictions_.push_front(std::move(kv));

// Remove the longest unused key-value pair when size exceeded
if (code_evictions_.size() > config_.max_code_keys) {
Expand All @@ -216,7 +216,7 @@ bool CoherentStateCache::add_code(const KeyValue& kv, CoherentStateRoot* root, S
return inserted;
}

Task<std::optional<Bytes>> CoherentStateCache::get(const Bytes& key, Transaction& tx) {
Task<std::optional<Bytes>> CoherentStateCache::get(ByteView key, Transaction& tx) {
std::shared_lock read_lock{rw_mutex_};

const auto view_id = tx.view_id();
Expand All @@ -225,7 +225,7 @@ Task<std::optional<Bytes>> CoherentStateCache::get(const Bytes& key, Transaction
co_return std::nullopt;
}

KeyValue kv{key};
KeyValue kv{Bytes{key}};
auto& cache = root_it->second->cache;
const auto kv_it = cache.find(kv);
if (kv_it != cache.end()) {
Expand All @@ -252,12 +252,13 @@ Task<std::optional<Bytes>> CoherentStateCache::get(const Bytes& key, Transaction
read_lock.unlock();
std::unique_lock write_lock{rw_mutex_};

add({key, value}, root_it->second.get(), view_id);
kv.value = value;
add(std::move(kv), root_it->second.get(), view_id);

co_return value;
}

Task<std::optional<Bytes>> CoherentStateCache::get_code(const Bytes& key, Transaction& tx) {
Task<std::optional<Bytes>> CoherentStateCache::get_code(ByteView key, Transaction& tx) {
std::shared_lock read_lock{rw_mutex_};

const auto view_id = tx.view_id();
Expand All @@ -266,7 +267,7 @@ Task<std::optional<Bytes>> CoherentStateCache::get_code(const Bytes& key, Transa
co_return std::nullopt;
}

KeyValue kv{key};
KeyValue kv{Bytes{key}};
auto& code_cache = root_it->second->code_cache;
const auto kv_it = code_cache.find(kv);
if (kv_it != code_cache.end()) {
Expand All @@ -293,7 +294,8 @@ Task<std::optional<Bytes>> CoherentStateCache::get_code(const Bytes& key, Transa
read_lock.unlock();
std::unique_lock write_lock{rw_mutex_};

add_code({key, value}, root_it->second.get(), view_id);
kv.value = value;
add_code(std::move(kv), root_it->second.get(), view_id);

co_return value;
}
Expand Down
16 changes: 8 additions & 8 deletions silkworm/db/kv/api/state_cache.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ class StateView {
public:
virtual ~StateView() = default;

virtual Task<std::optional<Bytes>> get(const Bytes& key) = 0;
virtual Task<std::optional<Bytes>> get(ByteView key) = 0;

virtual Task<std::optional<Bytes>> get_code(const Bytes& key) = 0;
virtual Task<std::optional<Bytes>> get_code(ByteView key) = 0;
};

class StateCache {
Expand Down Expand Up @@ -95,9 +95,9 @@ class CoherentStateView : public StateView {
CoherentStateView(const CoherentStateView&) = delete;
CoherentStateView& operator=(const CoherentStateView&) = delete;

Task<std::optional<Bytes>> get(const Bytes& key) override;
Task<std::optional<Bytes>> get(ByteView key) override;

Task<std::optional<Bytes>> get_code(const Bytes& key) override;
Task<std::optional<Bytes>> get_code(ByteView key) override;

private:
Transaction& txn_;
Expand Down Expand Up @@ -134,10 +134,10 @@ class CoherentStateCache : public StateCache {
void process_code_change(CoherentStateRoot* root, StateViewId view_id, const remote::AccountChange& change);
void process_delete_change(CoherentStateRoot* root, StateViewId view_id, const remote::AccountChange& change);
void process_storage_change(CoherentStateRoot* root, StateViewId view_id, const remote::AccountChange& change);
bool add(const KeyValue& kv, CoherentStateRoot* root, StateViewId view_id);
bool add_code(const KeyValue& kv, CoherentStateRoot* root, StateViewId view_id);
Task<std::optional<Bytes>> get(const Bytes& key, Transaction& txn);
Task<std::optional<Bytes>> get_code(const Bytes& key, Transaction& txn);
bool add(KeyValue&& kv, CoherentStateRoot* root, StateViewId view_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend to pass by value by default (and move at call sites), unless there's a need to disambiguate between overloads (e.g. && vs const&).

bool add_code(KeyValue&& kv, CoherentStateRoot* root, StateViewId view_id);
Task<std::optional<Bytes>> get(ByteView key, Transaction& txn);
Task<std::optional<Bytes>> get_code(ByteView key, Transaction& txn);
CoherentStateRoot* get_root(StateViewId view_id);
CoherentStateRoot* advance_root(StateViewId view_id);
void evict_roots(StateViewId next_view_id);
Expand Down
Loading