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.

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(persistence): reuse cursor for updating history indices #13622

Merged
merged 1 commit into from
Jan 9, 2025
Merged
Changes from all commits
Commits
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
22 changes: 12 additions & 10 deletions crates/storage/provider/src/providers/database/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -801,15 +801,17 @@ impl<TX: DbTxMut + DbTx + 'static, N: NodeTypes> DatabaseProvider<TX, N> {

/// Load shard and remove it. If list is empty, last shard was full or
/// there are no shards at all.
fn take_shard<T>(&self, key: T::Key) -> ProviderResult<Vec<u64>>
fn take_shard<T>(
&self,
cursor: &mut <TX as DbTxMut>::CursorMut<T>,
key: T::Key,
) -> ProviderResult<Vec<u64>>
where
T: Table<Value = BlockNumberList>,
{
let mut cursor = self.tx.cursor_read::<T>()?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This created a new cursor per account or storage slot change.

let shard = cursor.seek_exact(key)?;
if let Some((shard_key, list)) = shard {
if let Some((_, list)) = cursor.seek_exact(key)? {
// delete old shard so new one can be inserted.
self.tx.delete::<T>(shard_key, None)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This created another cursor and re-seeked even when cursor is already on the needed key.

cursor.delete_current()?;
let list = list.iter().collect::<Vec<_>>();
return Ok(list)
}
Expand All @@ -832,23 +834,23 @@ impl<TX: DbTxMut + DbTx + 'static, N: NodeTypes> DatabaseProvider<TX, N> {
P: Copy,
T: Table<Value = BlockNumberList>,
{
let mut cursor = self.tx.cursor_write::<T>()?;
for (partial_key, indices) in index_updates {
let mut last_shard =
self.take_shard::<T>(sharded_key_factory(partial_key, u64::MAX))?;
self.take_shard::<T>(&mut cursor, sharded_key_factory(partial_key, u64::MAX))?;
Comment on lines +837 to +840
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this makes sense @rkrasiuk

last_shard.extend(indices);
// Chunk indices and insert them in shards of N size.
let indices = last_shard;
let mut chunks = indices.chunks(sharded_key::NUM_OF_INDICES_IN_SHARD).peekable();
let mut chunks = last_shard.chunks(sharded_key::NUM_OF_INDICES_IN_SHARD).peekable();
while let Some(list) = chunks.next() {
let highest_block_number = if chunks.peek().is_some() {
*list.last().expect("`chunks` does not return empty list")
} else {
// Insert last list with `u64::MAX`.
u64::MAX
};
self.tx.put::<T>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This created another cursor per chunk.

cursor.insert(
Copy link
Member

Choose a reason for hiding this comment

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

@shekhirin @joshieDo any difference between tx.put and cursor.insert?

Copy link
Collaborator

@joshieDo joshieDo Jan 8, 2025

Choose a reason for hiding this comment

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

when inserting multiple values, generally yes: #1130 (comment)

sharded_key_factory(partial_key, highest_block_number),
BlockNumberList::new_pre_sorted(list.iter().copied()),
&BlockNumberList::new_pre_sorted(list.iter().copied()),
)?;
}
}
Expand Down
Loading