Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Switch confirmed_unrooted_slots from Vec<_> to HashSet<_> #33311

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

steviez
Copy link
Contributor

@steviez steviez commented Sep 19, 2023

Problem

The container is only used to check for inclusion of slots with
the .contains() method. This method is O(n) on a Vec<> but O(1) on a
HashSet<
>.

The container should be fairly small given that it is populated with confirmed slots that are newer than the most recent root. So, in normal network conditions, should be around the lockout length. Regardless, still a more proper change to make

@steviez steviez force-pushed the bstore_vec_to_hashset branch 2 times, most recently from 9c10ad4 to bd6300c Compare September 19, 2023 18:56
@yihau yihau added the CI Pull Request is ready to enter CI label Sep 20, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Sep 20, 2023
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Seems like a fair trade-off. Bump me when this is past CI and I'll approve

ledger/src/blockstore.rs Show resolved Hide resolved
The container is only used to check for inclusion of slots with
the .contains() method. This method is O(n) on a Vec<_> but O(1) on a
HashSet<_>.
@steviez steviez force-pushed the bstore_vec_to_hashset branch from bd6300c to 89dbed9 Compare September 21, 2023 06:09
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Merging #33311 (89dbed9) into master (357eabd) will increase coverage by 0.0%.
The diff coverage is 94.4%.

@@           Coverage Diff           @@
##           master   #33311   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         796      796           
  Lines      215745   215752    +7     
=======================================
+ Hits       176789   176809   +20     
+ Misses      38956    38943   -13     

@steviez
Copy link
Contributor Author

steviez commented Sep 21, 2023

Seems like a fair trade-off. Bump me when this is past CI and I'll approve

Shoot, my bad; I thought I had seen greens before adding you but that obviously wasn't the case. Rebase on tip of master did the trick

@steviez steviez merged commit 3428333 into solana-labs:master Sep 22, 2023
@steviez steviez deleted the bstore_vec_to_hashset branch September 22, 2023 05:15
SwenSchaeferjohann pushed a commit to ananas-block/solana that referenced this pull request Sep 23, 2023
Switch confirmed_unrooted_slots from Vec<_> to HashSet<_> (solana-labs#33311)

The container is only used to check for inclusion of slots with
the .contains() method. This method is O(n) on a Vec<_> but O(1) on a
HashSet<_>.

add Restart structs for disk index (solana-labs#33361)

use bytemuck for disk bucket restart (solana-labs#33371)

disk bucket: init restart path (solana-labs#33375)

simple cleanup in bucket map (solana-labs#33376)

add disk bucket get_restart_file (solana-labs#33373)

* add disk bucket get_restart_file

* add get_restartable_buckets

pass RestartableBucket through disk index (solana-labs#33377)

data bucket holds RestartableBucket (solana-labs#33381)

Feature - better error codes for tx lamport check (solana-labs#33343)

Replaces `TransactionError::InstructionError(0, InstructionError::UnbalancedInstruction)` with `TransactionError::UnbalancedTransaction`.

Co-authored-by: Alexander Meißner <AlexanderMeissner@gmx.net>

DiskIdx: reuse disk bucket file if possible (solana-labs#33379)

diskidx: stats for created vs reused (solana-labs#33385)

solana-program - altbn128: add g1 & g2 compression

still fixing tests for point of infinity

feat: proof compression syscall working

add rust test to ci

remove prints

added c test

added sycall pricing

fixed ci checks

refactored altbn128 and compression
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Sep 25, 2023
…s#33311)

The container is only used to check for inclusion of slots with
the .contains() method. This method is O(n) on a Vec<_> but O(1) on a
HashSet<_>.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants