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

Review bigint usage #5892

Open
twoeths opened this issue Aug 16, 2023 · 3 comments
Open

Review bigint usage #5892

twoeths opened this issue Aug 16, 2023 · 3 comments
Labels
meta-bug Issues that identify a bug and require a fix.

Comments

@twoeths
Copy link
Contributor

twoeths commented Aug 16, 2023

Describe the bug

As noted here using bigint causes more memory (rss + process_heap_bytes) in lodestar

ChainSafe/js-libp2p-gossipsub#327 (comment)

Expected behavior

we should review all of bigint usages in lodestar and do not use it unless it's really necessary

Steps to reproduce

No response

Additional context

No response

Operating system

Linux

Lodestar version or commit hash

unstable

@twoeths
Copy link
Contributor Author

twoeths commented Nov 23, 2023

Some tests using BigInt vector vs Number vector BeaconState.slashings in updated processSlashingsAllForks.test.ts in #6121 using node v20.5.0:

Number version

export const Slashings = new VectorBasicType(UintNum64, EPOCHS_PER_HISTORICAL_VECTOR);

BigInt version (as in unstable up until today), or refer to v1.12 https://github.com/ChainSafe/lodestar/blob/v1.12.0/packages/types/src/phase0/sszTypes.ts#L251

export const Slashings = new VectorBasicType(Gwei, EPOCHS_PER_SLASHINGS_VECTOR);
function getProcessSlashingsTestData(indicesToSlashLen: number): {
  state: CachedBeaconStateAllForks;
  cache: EpochTransitionCache;
} {
  const state = generatePerfTestCachedStatePhase0({goBackOneSlot: true});
  const cache = beforeProcessEpoch(state);
  state.slashings.set(0, indicesToSlashLen * MAX_EFFECTIVE_BALANCE);
  for (let i = 1; i < state.slashings.length; i++) {
    state.slashings.set(i, MAX_EFFECTIVE_BALANCE);
  }
  state.commit();

  cache.indicesToSlash = linspace(indicesToSlashLen);

  return {
    state,
    cache,
  };
}
EPOCHS_PER_SLASHINGS_VECTOR BigInt Number
8192 2.3ms/op 1.3ms/op
10M 2.3s/op 289.6655 ms/op
20M 7.59s/op 1.14s/op
30M Out of memory 1.9s/op

This is an example showing both memory and performance issue using BigInt

@dapplion
Copy link
Contributor

@tuyennhv thanks! results definitely show a penalty of using bigints. However I find it hard to believe that we have millions of instances of bigints in a Lodestar BeaconNode. Can you try to proof and count what's the upper bound of bigint instances possible?

If we use a regular number to represent total slashed balance the max ETH slashed we can represent is 9007199254740991 / 1e9 = 9,007,199 or 281474 validators. If for some reason more than those validators are slashed in a single epoch, Lodestar will suffer a consensus failure.

@twoeths
Copy link
Contributor Author

twoeths commented Nov 24, 2023

results definitely show a penalty of using bigints. However I find it hard to believe that we have millions of instances of bigints in a Lodestar BeaconNode. Can you try to proof and count what's the upper bound of bigint instances possible?

@dapplion the main thing of this issue is to prove the memory and performance issue of bigint and I used unrealistic number to do it. Yes I also don't believe we have that many instances of bigins in our beacon node.

If we use a regular number to represent total slashed balance the max ETH slashed we can represent is 9007199254740991 / 1e9 = 9,007,199 or 281474 validators. If for some reason more than those validators are slashed in a single epoch, Lodestar will suffer a consensus failure.

we'll discuss this in #6121 but thanks for the idea, will investigate it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta-bug Issues that identify a bug and require a fix.
Projects
None yet
Development

No branches or pull requests

2 participants