Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disk bucket allocated bit can be not in header #30834

Merged
merged 1 commit into from
Mar 22, 2023

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Mar 21, 2023

Problem

See #30711

If we store the single bit for 'allocated' status somewhere other than the header of each element, we save 8 bytes per element. Also remember the disk index is possibly 2x over-allocated.

Storage options include:

  1. in-mem bitfield per bucket
  2. in high bits stolen from ref_count's high bits
  3. in some other clever place (such as pubkey being a random pubkey which is unlikely to ever be used)

Summary of Changes

Placeholders for removing the allocated bit from the header.

Fixes #

@jeffwashington jeffwashington force-pushed the mm21 branch 3 times, most recently from 64be54c to e48e2fc Compare March 22, 2023 15:15
@jeffwashington jeffwashington changed the title wip: disk bucket allocated bit not in header disk bucket allocated bit can be not in header Mar 22, 2023
@jeffwashington jeffwashington marked this pull request as ready for review March 22, 2023 15:49
InHeader,
}

const FLAG_LOCATION: IsAllocatedFlagLocation = IsAllocatedFlagLocation::InHeader;
Copy link
Contributor

Choose a reason for hiding this comment

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

When I see FLAG_LOCATION by itself in the code, I first think "which flag?". Right now I have the context in my head, so the lookup is fast. And this constant is in the same file, so it'll fast later too. My preferences tend to be more verbose, maybe something along these lines is pleasing to you too?

Suggested change
const FLAG_LOCATION: IsAllocatedFlagLocation = IsAllocatedFlagLocation::InHeader;
const IS_ALLOCATED_FLAG_LOCATION: IsAllocatedFlagLocation = IsAllocatedFlagLocation::InHeader;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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.

lgtm

@codecov
Copy link

codecov bot commented Mar 22, 2023

Codecov Report

Merging #30834 (41862eb) into master (4285cb2) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #30834   +/-   ##
=======================================
  Coverage    81.4%    81.4%           
=======================================
  Files         723      723           
  Lines      203533   203538    +5     
=======================================
+ Hits       165845   165872   +27     
+ Misses      37688    37666   -22     

@jeffwashington jeffwashington merged commit 9f3381c into solana-labs:master Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants