High memory consumption while syncing a supernode

[tbenr]

In fusaka devnet 2 we can easily see supernode memory usage to around 8-9gb while syncing.

I managed to reduce the peak significantly by reducing the "inflight" blocks so that the number of columns to deal with go down significantly via:

--Xp2p-sync-batch-size=10
--Xp2p-sync-max-pending-batches=2

We may need to thing about changing these defaults (25 and 5 respectively) to avoid too much work happening in parallel in background.

[rolfyone]

it does appear that I fixed this, but its probably controversial.

Xrust-kzg-enabled: true

with CKZG we were peaking out at nearly 6G mem (all subnets custodied) and the ValidatoingProxy.onResponse was like 71% of all cpu time we spent

With rust kzg we were around 2G memory with all the same settings, and the same onResponse was about 20% of all cpu time...

I wonder if there's a reason that rust KZG isnt the default? To be fair it probably mostly means that the memory in use is just off heap, but its interesting that it so heavily impacts CPU time too...

@zilm13 would be interested in seeing your take...

[rolfyone]

data_availability_check times did seem good but I'd probably need to run a better comparison on that. the best case does appear lower than the best I saw with CKZG.

[rolfyone]

over 58 samples each, their times quoted in the late block import lines were basically the same (30ms dif over 2.5 seconds)

[jtraglia]

I made this PR for this. If this is just a peak (not a leak) it might be a different issue.

• In Java bindings, delete more local references ethereum/c-kzg-4844#589

[tbenr]

@jtraglia it was peak mem usage, not a leak, so I have the impression it might be something different.

[jtraglia]

I believe this is due to flattenBytes and flattenProofs which are only used with ckzg. This is essentially duplicating all of the cells, commitments, and proofs. I'm not sure there's anything we can do without big changes to ckzg's java bindings.

teku/infrastructure/kzg/src/main/java/tech/pegasys/teku/kzg/CKZG4844.java

Lines 183 to 203 in 2773391

	public boolean verifyCellProofBatch(
	final List<KZGCommitment> commitments,
	final List<KZGCellWithColumnId> cellWithIdList,
	final List<KZGProof> proofs) {
	if (commitments.size() != cellWithIdList.size() || cellWithIdList.size() != proofs.size()) {
	throw new KZGException("Cells, proofs and commitments sizes should match");
	}
	return CKZG4844JNI.verifyCellKzgProofBatch(
	CKZG4844Utils.flattenBytes(
	commitments.stream()
	.map(kzgCommitment -> (Bytes) kzgCommitment.getBytesCompressed())
	.toList(),
	commitments.size() * BYTES_PER_COMMITMENT),
	cellWithIdList.stream()
	.mapToLong(cellWithIds -> cellWithIds.columnId().id().longValue())
	.toArray(),
	CKZG4844Utils.flattenBytes(
	cellWithIdList.stream().map(cellWithIds -> cellWithIds.cell().bytes()).toList(),
	cellWithIdList.size() * BYTES_PER_CELL),
	CKZG4844Utils.flattenProofs(proofs));
	}

teku/infrastructure/kzg/src/main/java/tech/pegasys/teku/kzg/RustKZG.java

Lines 164 to 177 in 2773391

	public boolean verifyCellProofBatch(
	final List<KZGCommitment> commitments,
	final List<KZGCellWithColumnId> cellWithIdList,
	final List<KZGProof> proofs) {
	return library.verifyCellKZGProofBatch(
	commitments.stream().map(KZGCommitment::toArrayUnsafe).toArray(byte[][]::new),
	cellWithIdList.stream()
	.mapToLong(cellWithIds -> cellWithIds.columnId().id().longValue())
	.toArray(),
	cellWithIdList.stream()
	.map(cellWithIds -> cellWithIds.cell().bytes().toArrayUnsafe())
	.toArray(byte[][]::new),
	proofs.stream().map(KZGProof::toArrayUnsafe).toArray(byte[][]::new));
	}

[tbenr]

if i'm not mistaken the flattening in rustKZG happens on the native binding:

https://github.com/crate-crypto/rust-eth-kzg/blob/d49e9057ebce035e6a6024f32ae920fb20819ff8/bindings/java/rust_code/src/lib.rs#L136-L167

so on one side we got flattening happening on Java heap, on the other side on the native side, off java heap.

I don't recall when I was measuring this if I was looking at Total process memory or just Java heap.

if we want a faster flattening on cKZG we could implement it there but it would be a big change. At the end the mem consumption is still there if the underlining logic requires everything to be flattened.

[rolfyone]

I'm not sure what we want to do here given above? if its mostly backfill related we could slow it down..

[rolfyone]

  @Override
  public boolean verifyCellProofBatch(
      final List<KZGCommitment> commitments,
      final List<KZGCellWithColumnId> cellWithIdList,
      final List<KZGProof> proofs) {
    if (commitments.size() != cellWithIdList.size() || cellWithIdList.size() != proofs.size()) {
      throw new KZGException("Cells, proofs and commitments sizes should match");
    }

    final int size = commitments.size();
    // Pre-allocate arrays with exact sizes
    final byte[] flattenedCommitments = new byte[size * BYTES_PER_COMMITMENT];
    final long[] columnIds = new long[size];
    final byte[] flattenedCells = new byte[size * BYTES_PER_CELL];

    for (int i = 0; i < size; i++) {
      final byte[] commitmentBytes = commitments.get(i).getBytesCompressed().toArrayUnsafe();
      System.arraycopy(
          commitmentBytes, 0, flattenedCommitments, i * BYTES_PER_COMMITMENT, BYTES_PER_COMMITMENT);
      columnIds[i] = cellWithIdList.get(i).columnId().id().longValue();
      byte[] cellBytes = cellWithIdList.get(i).cell().bytes().toArray();
      System.arraycopy(cellBytes, 0, flattenedCells, i * BYTES_PER_CELL, BYTES_PER_CELL);
    }
    return CKZG4844JNI.verifyCellKzgProofBatch(
        flattenedCommitments, columnIds, flattenedCells, CKZG4844Utils.flattenProofs(proofs));
  }

so i had a play, but memory didn't seem that noticeably different...