perf(storage): optimize wiped storage entry collection to avoid Vec allocation #19127
Conversation
WilliamNwoke
requested review from
joshieDo,
rakita and
shekhirin
as code owners
|
Please review @yongkangc @mattsse |
There was a problem hiding this comment.
Think we could simplify this without the abstraction
ref:
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the collection of wiped storage entries by replacing an intermediate Vec allocation with a streaming iterator that processes entries directly from the database cursor.
Key Changes:
- Introduces
WipedStorageIterfor lazy iteration over wiped storage entries - Refactors storage revert writing to use the new iterator instead of collecting into a Vec
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/storage/provider/src/changesets_utils/wiped_storage.rs | New file implementing WipedStorageIter for streaming wiped storage entries |
| crates/storage/provider/src/changesets_utils/mod.rs | Exports the new WipedStorageIter type |
| crates/storage/provider/src/providers/database/provider.rs | Replaces Vec collection with WipedStorageIter in storage revert writing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mediocregopher
left a comment
mediocregopher
left a comment
There was a problem hiding this comment.
He @WilliamNwoke , sorry it took so long to look at this. I think the code could be simplified a little further, left a comment about that 🙏
|
Please review @mediocregopher @yongkangc |
|
Is this PR still active ? |
WilliamNwoke
force-pushed
the
refactor_storage
branch
from
464961b to
775d8ee
Compare
Yeah. Apologies for the late response |
|
@yongkangc @mediocregopher @mattsse Please review |
4 participants
Resolves #19077
Refactored the implementation to use a streaming iterator approach that processes wiped storage entries directly from the database cursor without intermediate collection. Key changes:
Added
StorageWipedEntriesIter: A new iterator that yields wiped storage entries on-demand from the database cursorIntegrated with
StorageRevertsIter: Combined with existing reverts processing for efficient merging