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

fix: improve forkchoice updateHead() time #5867

Merged
merged 8 commits into from
Aug 11, 2023
17 changes: 11 additions & 6 deletions packages/fork-choice/src/protoArray/computeDeltas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,33 +21,38 @@ export function computeDeltas(
): number[] {
const deltas = Array.from({length: indices.size}, () => 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

This can also be improved, it's faster to do a regular for loop than array.from

Copy link
Member

Choose a reason for hiding this comment

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

What about Array.fill performance?

Copy link
Contributor

Choose a reason for hiding this comment

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

Last time i checked regular for loop in the fastest way to populate an array. There should be benchmarks in the state transition package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  array creation
Array.from(() => 0): 9.6100125 ms
    ✔ Array.from(() => 0)
Array.from().fill(0): 8.389225 ms
    ✔ Array.from().fill(0)
Array.from(): 15.170741699999999 ms
    ✔ Array.from()
new Array(): 0.8613583 ms
    ✔ new Array()
new Array(); for loop: 1.2474166 ms
    ✔ new Array(); for loop

@dapplion I'll do new Array() and a for loop in in #5882 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you benchmark just pushing to an empty array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not yet, how important is it? in this case there are always more than 0 protonodes in forkchoice

const zeroHash = HEX_ZERO_HASH;
// avoid creating new variables in the loop to potentially reduce GC pressure
let oldBalance, newBalance: number;
let currentRoot, nextRoot: string;
let currentDeltaIndex, nextDeltaIndex: number | undefined;
for (let vIndex = 0; vIndex < votes.length; vIndex++) {
const vote = votes[vIndex];
// There is no need to create a score change if the validator has never voted or both of their
// votes are for the zero hash (genesis block)
if (vote === undefined) {
continue;
}
const {currentRoot, nextRoot} = vote;
currentRoot = vote.currentRoot;
nextRoot = vote.nextRoot;
if (currentRoot === zeroHash && nextRoot === zeroHash) {
continue;
}

// IF the validator was not included in the _old_ balances (i.e. it did not exist yet)
// then say its balance was 0
const oldBalance = oldBalances[vIndex] ?? 0;
oldBalance = oldBalances[vIndex] ?? 0;

// If the validator's vote is not known in the _new_ balances, then use a balance of zero.
//
// It is possible that there was a vote for an unknown validator if we change our justified
// state to a new state with a higher epoch that is on a different fork because that fork may have
// on-boarded fewer validators than the prior fork.
const newBalance = newBalances[vIndex] ?? 0;
newBalance = newBalances[vIndex] ?? 0;

if (equivocatingIndices.size > 0 && equivocatingIndices.has(vIndex)) {
// this function could be called multiple times but we only want to process slashing validator for 1 time
if (currentRoot !== zeroHash) {
const currentDeltaIndex = indices.get(currentRoot);
currentDeltaIndex = indices.get(currentRoot);
if (currentDeltaIndex !== undefined) {
if (currentDeltaIndex >= deltas.length) {
throw new ProtoArrayError({
Expand All @@ -65,7 +70,7 @@ export function computeDeltas(
if (currentRoot !== nextRoot || oldBalance !== newBalance) {
// We ignore the vote if it is not known in `indices .
// We assume that it is outside of our tree (ie: pre-finalization) and therefore not interesting
const currentDeltaIndex = indices.get(currentRoot);
currentDeltaIndex = indices.get(currentRoot);
if (currentDeltaIndex !== undefined) {
if (currentDeltaIndex >= deltas.length) {
throw new ProtoArrayError({
Expand All @@ -77,7 +82,7 @@ export function computeDeltas(
}
// We ignore the vote if it is not known in `indices .
// We assume that it is outside of our tree (ie: pre-finalization) and therefore not interesting
const nextDeltaIndex = indices.get(nextRoot);
nextDeltaIndex = indices.get(nextRoot);
twoeths marked this conversation as resolved.
Show resolved Hide resolved
if (nextDeltaIndex !== undefined) {
if (nextDeltaIndex >= deltas.length) {
throw new ProtoArrayError({
Expand Down
123 changes: 44 additions & 79 deletions packages/fork-choice/test/perf/protoArray/computeDeltas.test.ts
Original file line number Diff line number Diff line change
@@ -1,98 +1,63 @@
import {itBench, setBenchOpts} from "@dapplion/benchmark";
import {expect} from "chai";
import {
CachedBeaconStateAltair,
computeStartSlotAtEpoch,
EffectiveBalanceIncrements,
getEffectiveBalanceIncrementsZeroed,
} from "@lodestar/state-transition";
import {TIMELY_SOURCE_FLAG_INDEX} from "@lodestar/params";
// eslint-disable-next-line import/no-relative-packages
import {generatePerfTestCachedStateAltair} from "../../../../state-transition/test/perf/util.js";
import {EffectiveBalanceIncrements, getEffectiveBalanceIncrementsZeroed} from "@lodestar/state-transition";
import {VoteTracker} from "../../../src/protoArray/interface.js";
import {computeDeltas} from "../../../src/protoArray/computeDeltas.js";
import {computeProposerBoostScoreFromBalances} from "../../../src/forkChoice/forkChoice.js";

/** Same to https://github.com/ethereum/eth2.0-specs/blob/v1.1.0-alpha.5/specs/altair/beacon-chain.md#has_flag */
const TIMELY_SOURCE = 1 << TIMELY_SOURCE_FLAG_INDEX;
function flagIsTimelySource(flag: number): boolean {
return (flag & TIMELY_SOURCE) === TIMELY_SOURCE;
}

describe("computeDeltas", () => {
let originalState: CachedBeaconStateAltair;
const indices: Map<string, number> = new Map<string, number>();
let oldBalances: EffectiveBalanceIncrements;
let newBalances: EffectiveBalanceIncrements;

const oldRoot = "0x32dec344944029ba183ac387a7aa1f2068591c00e9bfadcfb238e50fbe9ea38e";
const newRoot = "0xb59f3a209f639dd6b5645ea9fad8d441df44c3be93bd1bbf50ef90bf124d1238";

before(function () {
this.timeout(2 * 60 * 1000); // Generating the states for the first time is very slow

originalState = generatePerfTestCachedStateAltair({goBackOneSlot: true});

const previousEpochParticipationArr = originalState.previousEpochParticipation.getAll();
const currentEpochParticipationArr = originalState.currentEpochParticipation.getAll();

const numPreviousEpochParticipation = previousEpochParticipationArr.filter(flagIsTimelySource).length;
const numCurrentEpochParticipation = currentEpochParticipationArr.filter(flagIsTimelySource).length;

expect(numPreviousEpochParticipation).to.equal(250000, "Wrong numPreviousEpochParticipation");
expect(numCurrentEpochParticipation).to.equal(250000, "Wrong numCurrentEpochParticipation");

oldBalances = getEffectiveBalanceIncrementsZeroed(numPreviousEpochParticipation);
newBalances = getEffectiveBalanceIncrementsZeroed(numPreviousEpochParticipation);

for (let i = 0; i < numPreviousEpochParticipation; i++) {
oldBalances[i] = 32;
newBalances[i] = 32;
}
for (let i = 0; i < 10000; i++) {
indices.set("" + i, i);
}
indices.set(oldRoot, 1001);
indices.set(newRoot, 1001);
});

setBenchOpts({
minMs: 30 * 1000,
maxMs: 40 * 1000,
});

itBench({
id: "computeDeltas",
beforeEach: () => {
const votes: VoteTracker[] = [];
const epoch = originalState.epochCtx.currentShuffling.epoch;
const committee = originalState.epochCtx.getBeaconCommittee(computeStartSlotAtEpoch(epoch), 0);
for (let i = 0; i < 250000; i++) {
if (committee.includes(i)) {
// 2 first numbers are respective to number of validators in goerli, mainnet as of Aug 2023
for (const numValidator of [500_000, 750_000, 1_400_000, 2_100_000]) {
before(function () {
this.timeout(2 * 60 * 1000);
oldBalances = getEffectiveBalanceIncrementsZeroed(numValidator);
newBalances = getEffectiveBalanceIncrementsZeroed(numValidator);

for (let i = 0; i < numValidator; i++) {
oldBalances[i] = 32;
newBalances[i] = 32;
}
for (let i = 0; i < 10000; i++) {
indices.set("" + i, i);
}
indices.set(oldRoot, 1001);
indices.set(newRoot, 1002);
});

setBenchOpts({
minMs: 30 * 1000,
maxMs: 40 * 1000,
});

itBench({
id: `computeDeltas ${numValidator} validators`,
beforeEach: () => {
const votes: VoteTracker[] = [];
const epoch = 100_000;
for (let i = 0; i < numValidator; i++) {
votes.push({
currentRoot: oldRoot,
nextRoot: newRoot,
nextEpoch: epoch,
});
} else {
votes.push({
currentRoot: oldRoot,
nextRoot: oldRoot,
nextEpoch: epoch - 1,
});
}
}
return votes;
},
fn: (votes) => {
computeDeltas(indices, votes, oldBalances, newBalances, new Set());
},
});

itBench({
id: "computeProposerBoostScoreFromBalances",
fn: () => {
computeProposerBoostScoreFromBalances(newBalances, {slotsPerEpoch: 32, proposerScoreBoost: 70});
},
});
return votes;
},
fn: (votes) => {
computeDeltas(indices, votes, oldBalances, newBalances, new Set());
},
});

itBench({
id: `computeProposerBoostScoreFromBalances ${numValidator} validators`,
fn: () => {
computeProposerBoostScoreFromBalances(newBalances, {slotsPerEpoch: 32, proposerScoreBoost: 70});
},
});
}
});
Loading