Avoid account index entry Arc clone in shrinking

[HaoranYi]

Problem

In shrinking, we use get() to find the index entry and update refcount. However, getting account index entry will result in an Arc clone, which can be avoided by passing the addref closure directly to the other get_interal API.

Similar to https://github.com/solana-labs/solana/pull/34918

Summary of Changes

• use get_internal and addref closure to avoid account's index entry Arc clone in shrinking.

Fixes #

[codecov[bot]]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (b1f8a89) 81.6% compared to head (70f480e) 81.6%. Report is 28 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #35010   +/-   ##
=======================================
  Coverage    81.6%    81.6%           
=======================================
  Files         830      830           
  Lines      224841   224847    +6     
=======================================
+ Hits       183486   183505   +19     
+ Misses      41355    41342   -13     

[jeffwashington]

this is fine, but in practice we never hit this code path. Hitting this would indicate we have miscalculated the alive bytes in an append vec and would be a bug in clean or shrink or append vecs. It does remove an easy use of the self ref struct. You may want to use the scan fn with Iter::once instead of this internal fn, though. or we should make a new single pubkey method that does the bin calc? Just seems non-ideal to use this internal method. I imagine we do it elsewhere, though.

[brooksprumo]

Hitting this would indicate we have miscalculated the alive bytes in an append vec and would be a bug in clean or shrink or append vecs.

@yhchiang-sol and I were just talking about alive bytes w.r.t. tiered storage. Since cold storage will compress account data, and hot storage with deduplicate owners, decrementing alive bytes after cleaning may need to be conservative/use a heuristic, and thus alive bytes may not be 100% accurate. Maybe this means we'll end up hitting this when using tiered storage?

[HaoranYi]

You may want to use the scan fn with Iter::once instead of this internal fn, though.

👍 Yes, updated.

[HaoranYi]

Maybe this means we'll end up hitting this when using tiered storage?

I added a WARN log when we hit this. We will find out if this is the case for cold storage.

[HaoranYi]

I also update the comments for scan function.

[jeffwashington]

Hitting this would indicate we have miscalculated the alive bytes in an append vec and would be a bug in clean or shrink or append vecs.

@yhchiang-sol and I were just talking about alive bytes w.r.t. tiered storage. Since cold storage will compress account data, and hot storage with deduplicate owners, decrementing alive bytes after cleaning may need to be conservative/use a heuristic, and thus alive bytes may not be 100% accurate. Maybe this means we'll end up hitting this when using tiered storage?

I think tiered storage will make this algorithm change significantly.