test(flat): make the arena allocated-bytes assertion page-size portable

[AnkushinDaniil]

Changes

• ArenaMetricsTests.ArenaWriter_Complete_AdvancesAllocatedBytes_ByFrontierDelta_NotMappedSize sized its payload to a hard-coded 4096 bytes and expected the ArenaAllocatedBytes gauge to advance by exactly 4096. ArenaWriter.Complete pads a shared arena's frontier up to an OS page, so the gauge advances by one page, not the logical write. On 4 KiB-page hosts (Linux CI) the two coincide; on 16 KiB-page hosts (Apple Silicon) the gauge advanced by 16384 and the assertion failed locally.
• Size the payload to Environment.SystemPageSize so the delta equals exactly one page on any platform, still proving the gauge tracks the written frontier and not the 64 KiB mapped size.

Types of changes

• Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

• No

Notes on testing

Nethermind.State.Flat.Test was red locally on arm64 macOS on this one case and is green after the change (961 passed / 10 skipped / 0 failed). Unaffected on 4 KiB Linux CI.

[claude]

Claude finished @AnkushinDaniil's task in 2m 49s —— View job

---

Review: page-size-portable arena assertion

Verified the change against the production code path. The fix is correct. No Critical, High, or Medium findings — two Low nits, both about keeping the test meaningful rather than about it passing.

Why the fix is correct

PersistedSnapshotDedicatedArenaThresholdBytes defaults to 1 GiB (FlatDbConfig.cs:32) and the test doesn't override it, so CreateWriter(payloadBytes) always takes the shared path (ArenaManager.cs:161) — the one that pads:

long newFrontier = _dedicated ? dataEnd : Math.Min(PageLayout.RoundUpToOsPage(dataEnd), _file.MappedSize);

(ArenaWriter.cs:46-48)

The arena is fresh, so _startOffset == 0 and dataEnd == payloadBytes. With payloadBytes == Environment.SystemPageSize, RoundUpToOsPage(dataEnd) == dataEnd, so OnWriteCompleted's Frontier - ReportedFrontier delta is exactly the payload on any page size — the root cause described in the PR body checks out, and so does the fix. The ArenaReservationBytes assertion (line 103) is unaffected: reservations are constructed from actualSize, never the padded frontier. The teardown assertions still hold — the single reservation covers the whole live frontier, so MarkDead drops the file and returns both gauges to baseline.

Scope is right too: the neighbouring BlobArenaWriter case is genuinely page-independent (_file.Frontier = _written, no OS-page padding, and the 1 KiB record at offset 0 never crosses the fixed 4 KiB PageLayout.PageSize straddle-pad at BlobArenaWriter.cs:99), and the many arenaFileSizeBytes: 4096 fixtures elsewhere in the suite stay correct on large-page hosts because the Math.Min(..., MappedSize) cap clamps the padding. That matches the reported 961 passed / 0 failed on arm64 macOS.

No security, performance, or consensus surface — test-only, one line.

Findings

Low — the assertion degenerates on 64 KiB-page hosts (inline)
maxArenaSize remains a hard-coded 64 KiB while the payload now scales. On a 64 KiB-page host (Linux arm64 with CONFIG_ARM64_64K_PAGES, ppc64le) payloadBytes == maxArenaSize == 65536, so payload, padded frontier and MappedSize all collapse to one value. The test still passes, but it can no longer distinguish "gauge tracks the frontier" from "gauge reports the mapped size" — which is the entire thing it's named after. long maxArenaSize = Math.Max(64 * 1024, 4L * Environment.SystemPageSize); keeps 64 KiB on 4/16 KiB hosts and preserves a real gap everywhere.

Low — comment doesn't record the coupling
Lines 99-100 still read "exactly the payload size, NOT the 64 KiB sparse MaxSize". That framing is what made the original 4096 literal look safe, and nothing in the test now says the payload must be a whole OS page. A half-line noting that Complete pads a shared frontier to a page boundary would stop this being simplified back to a literal later. (Couldn't attach inline — GitHub only accepts comments on the diff hunk.) Fix this →

Checklist

• Gather context (PR diff, changed file)
• Verify the padding path in ArenaWriter.Complete / ArenaManager.CreateWriter
• Confirm the assertion holds for 4 / 16 / 64 KiB page sizes
• Check the sibling blob test and other fixtures for the same page assumption
• Post review (inline + summary)

Not done: I could not execute dotnet test — the sandbox in this job denied the build command, so I reviewed statically. CI covers the 4 KiB Linux case and the author reports green on 16 KiB arm64 macOS.

Verdict: mergeable — both findings are Low and optional.
· branch daniil/fix-arena-metrics-parallel

[AnkushinDaniil]

@claude review

[claude]

Claude finished @AnkushinDaniil's task in 2m 19s —— View job

---

Review: page-size-portable arena assertion

Re-reviewed after ee277b1 (which applied the maxArenaSize scaling from my previous pass). Both commits are correct. No Critical, High, or Medium findings. One Low nit remains, unchanged from last time.

Current diff vs origin/master

-        const long maxArenaSize = 64 * 1024;  // 64 KiB sparse arena file
-        const int payloadBytes = 4096;
+        long maxArenaSize = Math.Max(64 * 1024, 4L * Environment.SystemPageSize);
+        int payloadBytes = Environment.SystemPageSize;

Verification

The gauge-delta assertion (line 101). PersistedSnapshotDedicatedArenaThresholdBytes defaults to 1 GiB and isn't overridden, so CreateWriter(payloadBytes) takes the shared path (ArenaManager.cs:161-164) — the padding one:

long newFrontier = _dedicated ? dataEnd : Math.Min(PageLayout.RoundUpToOsPage(dataEnd), _file.MappedSize);

(ArenaWriter.cs:46-48, with RoundUpToOsPage keyed off Environment.SystemPageSize at PageLayout.cs:40-42). Fresh arena ⇒ _startOffset == 0 ⇒ dataEnd == payloadBytes, which is now exactly one OS page, so RoundUpToOsPage is the identity and OnWriteCompleted's Frontier - ReportedFrontier delta equals the payload on any page size. Root cause and fix both check out.

The new maxArenaSize scaling is a no-op on 4 / 16 KiB hosts — Math.Max(65536, 4*4096) = Math.Max(65536, 4*16384) = 65536, so CI and the author's arm64 macOS run are byte-identical to before; only a 64 KiB-page host moves (to 256 KiB), which is what restores the payload-vs-MappedSize gap the test is named after. Overload resolution is fine (4L forces Math.Max(long, long)), and 256 KiB stays far under the tracker's uint page-key ceiling flagged at ArenaManager.cs:323.

No knock-on effects inside the test. ArenaReservationBytes (line 103) is built from actualSize, never the padded frontier. hasHeadroom is now always true (page < 4*page), so the file re-enters the mutable pool — and the teardown still passes because the single reservation's deadSize == payloadBytes == Frontier, so MarkDead hits DeadBytes >= Frontier and RemoveFullyDeadArena returns both gauges to baseline (ArenaManager.cs:265-289). Worth knowing: that makes two independent places the test depends on payloadBytes being exactly one whole page, not just the line-101 delta.

Scope is right. The sibling BlobArenaWriter case is genuinely page-independent (Frontier = _written, no OS-page padding), the fixture's own CreateArenaManager / WritePersistedArena(4096) helper stays correct on large-page hosts because Math.Min(..., MappedSize) clamps the padding and Initialize_ReportsMissingArenaEntriesWithoutReusingTheirIds asserts no byte gauges, and the arenaFileSizeBytes: 4096 fixtures elsewhere already route through TestFixtureHelpers.cs:31's Math.Max(arenaSize, Environment.SystemPageSize). Consistent with the reported 961 passed / 0 failed on arm64 macOS.

Test-only, two lines, no production, consensus, security, or performance surface. Style-wise const → local is unavoidable (Environment.SystemPageSize isn't a compile-time constant) and no var is introduced.

Findings

Low — the comment block no longer matches the code (unchanged from previous review)
Lines 99-100 still read "exactly the payload size, NOT the 64 KiB sparse MaxSize". On a 64 KiB-page host the MaxSize is now 256 KiB, and more importantly nothing in the test records the invariant that makes it work: ArenaWriter.Complete pads a shared arena's frontier to an OS page, so payloadBytes must be exactly one page — both for the line-101 delta and for the teardown's DeadBytes >= Frontier removal. That unstated coupling is exactly what made the original 4096 literal look safe; without it, someone may simplify it back. Purely optional. Fix this →

Checklist

• Gather context (diff, both commits)
• Re-verify the padding path against ArenaWriter / ArenaManager
• Check the new maxArenaSize scaling for regressions (mapping, reservations, teardown, uint page keys)
• Sweep the rest of the suite for the same page assumption
• Post review

Not done: I could not execute the build — dotnet build was denied by this job's sandbox, so this is a static review again. CI covers the 4 KiB Linux case; the author reports green on 16 KiB arm64 macOS. The 64 KiB-page path is reasoned about, not executed anywhere.

Verdict: mergeable — the sole remaining finding is Low and optional.
· branch daniil/fix-arena-metrics-parallel