Skip to content

Commit 4fe86ae

Browse files
authored
fix: avoid Set.has() in computeDeltas() (#8525)
**Motivation** - while investigating #8519 I found a performance issue with `computeDeltas()` where we check for `Set.has()` for every item in the big validator index loop **Description** - sort the `equivocatingIndices` set then track equivocatingValidatorIndex to avoid `Set.has()` there - fix perf test to include some equivocating indices **Benchmark on my local environment** - NodeJS: it's 1.53x faster before ``` computeDeltas ✔ computeDeltas 1400000 validators 300 proto nodes 73.65370 ops/s 13.57705 ms/op - 931 runs 30.1 s ✔ computeDeltas 1400000 validators 1200 proto nodes 73.44709 ops/s 13.61524 ms/op - 922 runs 30.0 s ✔ computeDeltas 1400000 validators 7200 proto nodes 73.59195 ops/s 13.58844 ms/op - 937 runs 30.0 s ✔ computeDeltas 2100000 validators 300 proto nodes 49.27426 ops/s 20.29457 ms/op - 623 runs 30.1 s ✔ computeDeltas 2100000 validators 1200 proto nodes 49.11422 ops/s 20.36070 ms/op - 614 runs 30.1 s ✔ computeDeltas 2100000 validators 7200 proto nodes 48.75805 ops/s 20.50943 ms/op - 619 runs 30.1 s ``` after ``` computeDeltas ✔ computeDeltas 1400000 validators 300 proto nodes 113.6256 ops/s 8.800830 ms/op - 1076 runs 30.1 s ✔ computeDeltas 1400000 validators 1200 proto nodes 112.0909 ops/s 8.921329 ms/op - 1079 runs 30.0 s ✔ computeDeltas 1400000 validators 7200 proto nodes 111.5792 ops/s 8.962247 ms/op - 1068 runs 30.1 s ✔ computeDeltas 2100000 validators 300 proto nodes 75.48259 ops/s 13.24809 ms/op - 727 runs 30.1 s ✔ computeDeltas 2100000 validators 1200 proto nodes 74.93052 ops/s 13.34570 ms/op - 707 runs 30.1 s ✔ computeDeltas 2100000 validators 7200 proto nodes 74.82280 ops/s 13.36491 ms/op - 751 runs 30.0 s ``` - Bun: it's 3.88x faster before ``` computeDeltas ✔ computeDeltas 1400000 validators 300 proto nodes 103.6817 ops/s 9.644905 ms/op x1.578 1791 runs 30.0 s ✔ computeDeltas 1400000 validators 1200 proto nodes 103.4132 ops/s 9.669949 ms/op x1.580 1800 runs 30.1 s ✔ computeDeltas 1400000 validators 7200 proto nodes 103.7312 ops/s 9.640297 ms/op x1.578 1745 runs 30.1 s ✔ computeDeltas 2100000 validators 300 proto nodes 68.86443 ops/s 14.52128 ms/op x1.583 1188 runs 30.0 s ✔ computeDeltas 2100000 validators 1200 proto nodes 68.66082 ops/s 14.56435 ms/op x1.585 1195 runs 30.1 s ✔ computeDeltas 2100000 validators 7200 proto nodes 68.49115 ops/s 14.60043 ms/op x1.592 1194 runs 30.1 s ``` after ``` computeDeltas ✔ computeDeltas 1400000 validators 300 proto nodes 407.0697 ops/s 2.456582 ms/op x0.255 3117 runs 30.1 s ✔ computeDeltas 1400000 validators 1200 proto nodes 402.2402 ops/s 2.486077 ms/op x0.257 2838 runs 30.0 s ✔ computeDeltas 1400000 validators 7200 proto nodes 401.5803 ops/s 2.490162 ms/op x0.258 2852 runs 30.0 s ✔ computeDeltas 2100000 validators 300 proto nodes 265.5509 ops/s 3.765757 ms/op x0.259 1988 runs 30.1 s ✔ computeDeltas 2100000 validators 1200 proto nodes 267.6306 ops/s 3.736494 ms/op x0.257 2026 runs 30.0 s ✔ computeDeltas 2100000 validators 7200 proto nodes 266.0949 ops/s 3.758058 ms/op x0.257 2035 runs 30.1 s ``` Co-authored-by: Tuyen Nguyen <[email protected]>
1 parent a2a22d6 commit 4fe86ae

File tree

2 files changed

+9
-4
lines changed

2 files changed

+9
-4
lines changed

packages/fork-choice/src/protoArray/computeDeltas.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ export function computeDeltas(
2828
// avoid creating new variables in the loop to potentially reduce GC pressure
2929
let oldBalance: number, newBalance: number;
3030
let currentIndex: number | null, nextIndex: number | null;
31+
// sort equivocating indices to avoid Set.has() in the loop
32+
const equivocatingArray = Array.from(equivocatingIndices).sort((a, b) => a - b);
33+
let equivocatingIndex = 0;
34+
let equivocatingValidatorIndex = equivocatingArray[equivocatingIndex];
3135

3236
for (let vIndex = 0; vIndex < votes.length; vIndex++) {
3337
const vote = votes[vIndex];
@@ -50,7 +54,7 @@ export function computeDeltas(
5054
// on-boarded fewer validators than the prior fork.
5155
newBalance = newBalances === oldBalances ? oldBalance : (newBalances[vIndex] ?? 0);
5256

53-
if (equivocatingIndices.size > 0 && equivocatingIndices.has(vIndex)) {
57+
if (vIndex === equivocatingValidatorIndex) {
5458
// this function could be called multiple times but we only want to process slashing validator for 1 time
5559
if (currentIndex !== null) {
5660
if (currentIndex >= numProtoNodes) {
@@ -62,6 +66,8 @@ export function computeDeltas(
6266
deltas[currentIndex] -= oldBalance;
6367
}
6468
vote.currentIndex = null;
69+
equivocatingIndex++;
70+
equivocatingValidatorIndex = equivocatingArray[equivocatingIndex];
6571
continue;
6672
}
6773

packages/fork-choice/test/perf/protoArray/computeDeltas.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ describe("computeDeltas", () => {
1010
const oneHourProtoNodes = (60 * 60) / 12;
1111
const fourHourProtoNodes = 4 * oneHourProtoNodes;
1212
const oneDayProtoNodes = 24 * oneHourProtoNodes;
13-
// 2 first numbers are respective to number of validators in goerli, mainnet as of Aug 2023
14-
const numValidators = [500_000, 750_000, 1_400_000, 2_100_000];
13+
const numValidators = [1_400_000, 2_100_000];
1514
for (const numValidator of numValidators) {
1615
beforeAll(
1716
() => {
@@ -47,7 +46,7 @@ describe("computeDeltas", () => {
4746
return votes;
4847
},
4948
fn: (votes) => {
50-
computeDeltas(numProtoNode, votes, oldBalances, newBalances, new Set());
49+
computeDeltas(numProtoNode, votes, oldBalances, newBalances, new Set([1, 2, 3, 4, 5]));
5150
},
5251
});
5352
}

0 commit comments

Comments
 (0)