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

Conversation

@jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented May 22, 2021

Problem

2 different use cases have emerged where it is useful to have storages sorted by slot.

  1. merging write cache data with store data so we can use this scan in more places. Write cache data is organized by slot.
  2. perf improvements for scanning accounts. If slots are in order, we don't have to keep track of a slot (or a write version) per slot, which is a large data savings.

Summary of Changes

Sort storages by slot.

Fixes #

@jeffwashington jeffwashington force-pushed the flatten3 branch 2 times, most recently from 700cd92 to ca216bf Compare May 22, 2021 17:56
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

I'm assuming the sorting helps speed up the scan? Also assuming the speed up to the scan is greater than the time to sort, yes?

Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

My comments are mostly questions :)

@jeffwashington jeffwashington force-pushed the flatten3 branch 14 times, most recently from 282c98a to 95da609 Compare May 28, 2021 21:28
@codecov
Copy link

codecov bot commented May 29, 2021

Codecov Report

Merging #17411 (0621672) into master (72bb271) will increase coverage by 0.0%.
The diff coverage is 95.4%.

@@           Coverage Diff            @@
##           master   #17411    +/-   ##
========================================
  Coverage    82.7%    82.8%            
========================================
  Files         428      429     +1     
  Lines      119964   120063    +99     
========================================
+ Hits        99325    99435   +110     
+ Misses      20639    20628    -11     

@jeffwashington jeffwashington marked this pull request as ready for review June 1, 2021 04:33
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Looks good! Just the two questions/comments.

brooksprumo
brooksprumo previously approved these changes Jun 1, 2021
@jeffwashington jeffwashington added the automerge Merge this Pull Request automatically once CI passes label Jun 1, 2021
@mergify mergify bot dismissed brooksprumo’s stale review June 1, 2021 18:21

Pull request has been modified.

Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Re-approving since mergify removed it.

@jeffwashington jeffwashington removed the automerge Merge this Pull Request automatically once CI passes label Jun 1, 2021
@jeffwashington jeffwashington merged commit ef5169f into solana-labs:master Jun 1, 2021
mergify bot pushed a commit that referenced this pull request Jun 3, 2021
* sort storages by slot before scan

* fix return value

(cherry picked from commit ef5169f)

# Conflicts:
#	runtime/src/accounts_db.rs
jeffwashington added a commit that referenced this pull request Jun 3, 2021
* sort storages by slot before scan

* fix return value

(cherry picked from commit ef5169f)

# Conflicts:
#	runtime/src/accounts_db.rs
mergify bot added a commit that referenced this pull request Jun 4, 2021
* sort storages by slot before scan

* fix return value

(cherry picked from commit ef5169f)

# Conflicts:
#	runtime/src/accounts_db.rs

Co-authored-by: Jeff Washington (jwash) <[email protected]>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
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.

2 participants