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

Optimized RocksDBStore.IterateIndexes() for pruned chain storage. #4004

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

Conversation

greymistcube
Copy link
Contributor

@greymistcube greymistcube commented Nov 29, 2024

See #4003 for context.

Should follow #4001.
This assumes a transition from a pruned storage to an unpruned storage is never allowed.
Together with #3999 and automatic pruning in RocksDBStore() constructor and further removal of chain id manipulating methods, the implementation should become much simpler.

I haven't tested but I don't think Iterator.SeekToPrev() would be any faster. This is still slower than the original bugged implementation of IterateIndexes(), but the performance degradation is much more tolerable.

@greymistcube greymistcube force-pushed the refactor/pruned-indexer branch 3 times, most recently from b2e4672 to 49ae37f Compare November 29, 2024 05:20
@greymistcube
Copy link
Contributor Author

Added 49ae37f for further optimization.

The point is to terminate iteration early at the exact point it should terminate.
Although not a proper benchmark, a preliminary benchmark shows a vastly improved indexing speed.

For unpruned Odin chain:
Took 28 ms to access last 1000 hashes

For pruned Odin chain:
Took 2 ms to access last 1000 hashes

Just another reason why pruning should be done. 😗

@greymistcube greymistcube force-pushed the refactor/pruned-indexer branch from 49ae37f to 3eaa387 Compare November 29, 2024 06:25
@greymistcube greymistcube marked this pull request as ready for review November 29, 2024 06:27
@greymistcube
Copy link
Contributor Author

Unless we want to support partial chains, the removed test is invalid.

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.

1 participant