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

Data as arc#15714

Closed
jeffwashington wants to merge 2 commits intosolana-labs:masterfrom
jeffwashington:data_as_arc
Closed

Data as arc#15714
jeffwashington wants to merge 2 commits intosolana-labs:masterfrom
jeffwashington:data_as_arc

Conversation

@jeffwashington
Copy link
Copy Markdown
Contributor

@jeffwashington jeffwashington commented Mar 4, 2021

Problem
We don't want to copy account and account data around unnecessarily.
Account structure is fixed for abi. It needs to remain.

Summary of Changes
Step 1 is creating a new AccountSharedData struct which we can manipulate. That is PR #15691
Step 2 is this PR. This is to make AccountSharedData.data hold data: Arc<Vec>. The major changes have comments next to them in the source below.

@jeffwashington
Copy link
Copy Markdown
Contributor Author

Processing a ledger with ledger-tool on a recent mainnet snapshot with this change produced this difference vs master:
ledger processed in 1 minute and 21 seconds. 7124 MB allocated. root slot is 67566632 2 forks at 67566647 67566674 with 42 frozen banks
ledger processed in 1 minute and 31 seconds. 8053 MB allocated. root slot is 67566632 2 forks at 67566647 67566674 with 42 frozen banks

@jeffwashington
Copy link
Copy Markdown
Contributor Author

jeffwashington commented Mar 4, 2021

note this is based off #15691
That pr is intended to have as much as possible the bulk, boring code changes due to replacing a struct (while preserving the previous one)
This pr is intended to have only changes related to changing data to be stored in an arc.

Comment thread sdk/src/account.rs
Comment thread runtime/src/accounts_cache.rs Outdated
#[derive(Debug, Clone)]
pub struct CachedAccount {
pub account: Account,
pub account: AccountNoData,
Copy link
Copy Markdown
Contributor Author

@jeffwashington jeffwashington Mar 4, 2021

Choose a reason for hiding this comment

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

The root place this change impacts many code paths is here. The cache now holds AccountNoData, which shares its data behind an Arc. So, as an item is loaded from the cache, the data is never copied until it needs to be modified by someone. Cloning the account clones the non-data fields and addrefs the Arc to the data.

A subsequent change will attempt to move this AccountNoData struct to AccountNoDataInner and make AccountNoData have an Arc to an AccountNoDataInner. This would reduce copying of the rest of the Account contents (lamports, executable, owner, rent_epoch)

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 5, 2021

Codecov Report

Merging #15714 (be7b618) into master (efcb580) will decrease coverage by 0.0%.
The diff coverage is 82.7%.

@@            Coverage Diff            @@
##           master   #15714     +/-   ##
=========================================
- Coverage    80.1%    80.0%   -0.1%     
=========================================
  Files         407      407             
  Lines      105449   105962    +513     
=========================================
+ Hits        84488    84867    +379     
- Misses      20961    21095    +134     

@jeffwashington jeffwashington force-pushed the data_as_arc branch 2 times, most recently from a1d2dfc to c08da33 Compare March 6, 2021 08:00
@stale
Copy link
Copy Markdown

stale Bot commented Mar 19, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale Bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Mar 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

stale [bot only] Added to stale content; results in auto-close after a week.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants