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

reduces std::mem::size_of::<gossip::CrdsData>() #4391

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

behzadnouri
Copy link

Problem

Because ContactInfo.cache is huge, currently

std::mem::size_of::<gossip::CrdsData>()

stands at 560 bytes. However only a small % of entries in gossip CRDS table are ContactInfo so this wastes a lot of memory.

Summary of Changes

In order to reduce the size of CrdsData enum, the commit adds type parameter for ContactInfo.cache. For the ContactInfo stored in CRDS table we use

ContactInfo.cache: Box<[SocketAddr; N]>

whereas outside gossip CRDS table we avoid Box<...> overhead by just using the plain array:

ContactInfo.cache: [SocketAddr; N]

Doing so,

  • std::mem::size_of::<gossip::CrdsData>() reduces from 560 bytes to 176 bytes.
  • Accessing other fields of ContactInfo does not incur Box<...> overhead.
  • Outside gossip CRDS table, we avoid Box<...> overhead entirely.

@behzadnouri behzadnouri force-pushed the box-contact-info-cache branch 2 times, most recently from 974fd85 to 23038ae Compare January 10, 2025 17:36
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jan 10, 2025
anza-xyz#4391
adds type parameter to ContactInfo.cache but we don't want to expose
that to outside of gossip crate.

In order to encapsulate that change in gossip crate, this commit adds an
alias for:
    Fn(&ContactInfo) -> R
which can be used outside of gossip crate.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Jan 10, 2025
anza-xyz#4391
adds type parameter to ContactInfo.cache but we don't want to expose
that implementation detail to outside of the gossip crate.

In order to encapsulate that change in gossip crate, this commit adds an
alias for:
    Fn(&ContactInfo) -> R
which can be used outside of the gossip crate.
@behzadnouri behzadnouri force-pushed the box-contact-info-cache branch from 23038ae to 03c984f Compare January 10, 2025 19:01
anza-xyz#4391
adds type parameter to gossip ContactInfo, but CrdsData::ContactInfo(_)
can only be initialized with a specific type.

In order to simplify the change and avoid verbosity, this commit
implements From<ContactInfo> for CrdsData which allows to generically
handle different types without changing the call sites.
anza-xyz#4391
adds type parameter to ContactInfo.cache but we don't want to expose
that implementation detail to outside of the gossip crate.

In order to encapsulate that change in gossip crate, this commit adds an
alias for:
    Fn(&ContactInfo) -> R
which can be used outside of the gossip crate.
We no longer use LegacyContactInfo and don't need to push updates over
gossip.
Because ContactInfo.cache is huge, currently
    std::mem::size_of::<gossip::CrdsData>()
stands at 560 bytes. However only a small % of entries in gossip CRDS
table are ContactInfo so this wastes a lot of memory.

In order to reduce the size of CrdsData enum, the commit adds type
parameter for ContactInfo.cache. For the ContactInfo stored in CRDS
table we use
    ContactInfo.cache: Box<[SocketAddr; N]>
whereas outside gossip CRDS table we avoid Box<...> overhead by just
using the plain array:
    ContactInfo.cache: [SocketAddr; N]

Doing so,
  * std::mem::size_of::<gossip::CrdsData>() reduces from 560 bytes to
    176 bytes.
  * Accessing other fields of ContactInfo does not incur Box<...>
    overhead.
  * Outside gossip CRDS table, we avoid Box<...> overhead entirely.
@behzadnouri behzadnouri force-pushed the box-contact-info-cache branch from 03c984f to c3cbfe1 Compare January 10, 2025 19:48
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.

1 participant