Account -> AccountSharedData

[jeffwashington]

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 in creating a new AccountSharedData struct which holds data in an Arc.
This change intends to be as much as possible just a rename since the rename touches many files.
Subsequent changes will change the implementation of AccountSharedData and handle its fallout.
I accomplished this by creating a parallel AccountSharedData which is a duplicate of Account. This let me produce something that should be identical in performance and allow us to review the mass rename changes separately from what should be interesting changes.
#15714
Fixes #

[jeffwashington]

I ran ledger-tool verify on a colo machine against a recent mainnet snapshot. This change should have no or minimal effect.
master:
ledger processed in 1 minute and 31 seconds. 8053 MB allocated. root slot is 67566632 2 forks at 67566647 67566674 with 42 frozen banks
this pr:
ledger processed in 1 minute and 31 seconds. 7982 MB allocated. root slot is 67566632 2 forks at 67566647 67566674 with 42 frozen banks

[codecov]

Codecov Report

Merging #15691 (ed16a59) into master (a7d5645) will increase coverage by 0.0%.
The diff coverage is 90.1%.

@@           Coverage Diff            @@
##           master   #15691    +/-   ##
========================================
  Coverage    80.1%    80.1%            
========================================
  Files         407      407            
  Lines      105422   105849   +427     
========================================
+ Hits        84457    84828   +371     
- Misses      20965    21021    +56     

[jeffwashington]

Some archived discussion here:
#15639

[jeffwashington]

@CriesofCarrots @jackcmay : @carllin suggested I call this type of rpc change out to you two in case we needed to do something different here.

[CriesofCarrots]

Thanks for the heads up! Is there anything in particular in the client or rpc tooling about which you have questions?

[jeffwashington]

@CriesofCarrots: @carllin and I expressed some lack of knowledge in the rpc area. @sakridge raised rpc as a concern early on with this refactoring and rename. So, we need an expert to make sure we can add methods here and things will work ok and to see if we missed something.

[jeffwashington]

I like shared of the choices ;) Sent from my phone.

…

On Mar 5, 2021, at 7:21 PM, sakridge ***@***.***> wrote:  @sakridge commented on this pull request. In sdk/src/account.rs: > @@ -21,24 +21,184 @@ pub struct Account { pub rent_epoch: Epoch, } +/// An Account with data that is stored on chain +/// This will become a new in-memory representation of the 'Account' struct data. +/// The existing 'Account' structure cannot easily change due to downstream projects. +/// This struct will shortly rely on something like the AnAccount trait for access to the fields. +#[derive(Serialize, Deserialize, PartialEq, Eq, Clone, Default, AbiExample)] +pub struct AccountNoData { I think AccountRefData or AccountSharedData might be better. AccountNoData sounds to me like there's no data at all, but I think it just means it has a reference to the data instead of owning it outright. Does it sound right? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[CriesofCarrots]

Is the intent to deprecate Account altogether? I am wondering if the rpc_client changes are necessary, or if downstream clients should still be operating using Account (with AccountSharedData just an internal node concept). The account will always have to be copied to be returned from RPC anyway, so could use an AccountSharedData::into<Account>() method.

[jeffwashington]

Is the intent to deprecate Account altogether? I am wondering if the rpc_client changes are necessary, or if downstream clients should still be operating using Account (with AccountSharedData just an internal node concept). The account will always have to be copied to be returned from RPC anyway, so could use an AccountSharedData::into<Account>() method.

The intuition of @carllin , @sakridge , @jackcmay appears to be that we will need to keep the existing Account struct around for things like 'downstream projects' that will have to be modified if we make the fields of 'Account' private and abstract away how the conceptual account is actually stored in memory. For example, if lamports becomes private, and we require traits to access ie. account.lamports**()** or account.set_lamports(new_lamports), then existing code in downstream projects or 3rd parties could break.

I tried to walk the right line for things like rpc where we could at the beginning get an account in either format (Account or AccountSharedData). The existing function call (.get_account()) still needed to return Account. This would require us to litter in various places AccountSharedData::from(_.get_account()). This at a minimum could result in extra struct copying and such on every account load. Perhaps this is inlined and optimized away. I went with the approach that in the few origins of Account[SharedData], the caller could be explicit about what storage they wanted initially, which should result in higher performance code. It didn't seem there were too many places where we needed to replicate the 'get' functions. I am happy to consider other options.

[jeffwashington]

I just pushed commits to respond to pr feedback and rebase on master.

[jeffwashington]

bench-tps #s
master: Average TPS: 52101.16
shareddata: Average TPS: 52483.273

colo machine, single node testnet

[jeffwashington]

I reworked a lot, squashed it all, rolled back some changes. Squashed it.

[carllin]

@behzadnouri does this look ok now?

[behzadnouri]

the issue was changing Serialize/Deserialize impl which is now reverted.
changes to new_rand_vote_account also seem redundant but i guess it does not make a difference.