fix(pipeline): MP1b review - bake CLI determinism + bounded memory

Adversarially-verified review findings 1 and 10:

(1) The bake pipeline no longer accumulates every decoded ObjectMeshData
in one ConcurrentBag before writing (multi-GB OOM risk on the full
bake), and no longer writes blobs in thread-completion order (which
violated the plan's "bakes must be byte-reproducible run-to-run").
New shape in BakeRunner: build the FULL id list, sort by PakKey, chunk
into 512-id batches; Parallel.ForEach WITHIN each batch; sort each
batch's results by key and AddBlob sequentially; release the batch.
Batches are contiguous key ranges, so the blob region lands in global
key order regardless of thread scheduling, and peak memory is one
batch's output. Side-staged particle-preload meshes drain per batch
into a key-deduped map (first instance wins — per-id extraction output
is deterministic, so instance choice cannot affect bytes) and are
written after all batches, sorted by key, skipping keys already
written.

Program.cs is now a thin arg-parsing shell over the public BakeRunner
so the new dat-gated byte-reproducibility test can drive the REAL
pipeline: tests/AcDream.Bake.Tests (new project, rule 6; registered in
slnx; no Silk.NET in its resolved dependency graph — verified) bakes
the same 9-id mixed fixture twice with DIFFERENT thread counts (8 vs
3 — thread scheduling was the nondeterminism source) and asserts the
two pak files are byte-identical. Ran for real against the dats on
this machine: green.

(10) The isSetup argument for EnvCell extraction now matches at both
call sites (BakeRunner and PakEquivalenceTests both pass false) and is
documented at each: the runtime's own request sites
(WbMeshAdapter.IncrementRefCount / EnsureLoaded) pass isSetup: false
for every MeshRef id including cell-geometry ids. The parameter is
currently dead in MeshExtractor.PrepareMeshData (dispatch is on the
resolved dat type), but two disagreeing call sites were a latent trap.

Header FormatVersion is no longer set by the bake (PakWriter stamps it
per review finding 2, previous commit).
This commit is contained in:
Erik 2026-07-05 22:14:31 +02:00
parent 84d1956d84
commit 86e0dc4655
6 changed files with 371 additions and 208 deletions

View file

@ -0,0 +1,29 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<IsPackable>false</IsPackable>
<LangVersion>latest</LangVersion>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="coverlet.collector" Version="6.0.4" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.14.1" />
<PackageReference Include="xunit" Version="2.9.3" />
<PackageReference Include="xunit.runner.visualstudio" Version="3.1.4" />
</ItemGroup>
<ItemGroup>
<Using Include="Xunit" />
</ItemGroup>
<ItemGroup>
<!-- NO Silk.NET / GL anywhere in this dependency chain (AcDream.Bake ->
AcDream.Content -> AcDream.Core are all GL-free by construction). -->
<ProjectReference Include="..\..\src\AcDream.Bake\AcDream.Bake.csproj" />
</ItemGroup>
</Project>

View file

@ -0,0 +1,79 @@
using System;
using System.Collections.Generic;
using System.IO;
using AcDream.Bake;
namespace AcDream.Bake.Tests;
/// <summary>
/// Dat-gated byte-reproducibility gate for the bake pipeline (review finding
/// 1): the plan's "bakes must be byte-reproducible run-to-run" is a whole-
/// FILE property, so it must be tested through the REAL BakeRunner — the
/// serializer-level determinism tests in Content.Tests can't see thread-
/// completion-order effects in the blob region. Skips cleanly when the dats
/// are absent (CI), same convention as
/// tests/AcDream.Core.Tests/Conformance/DatConcurrencyStressTests.
/// </summary>
public sealed class BakeDeterminismTests : IDisposable {
private readonly List<string> _tempFiles = new();
private string NewTempPakPath() {
var path = Path.Combine(Path.GetTempPath(), $"acdream-baketest-{Guid.NewGuid():N}.pak");
_tempFiles.Add(path);
return path;
}
public void Dispose() {
foreach (var f in _tempFiles) {
try { if (File.Exists(f)) File.Delete(f); } catch { /* best effort cleanup */ }
}
}
private static string? ResolveDatDir() {
// Mirrors ConformanceDats.ResolveDatDir (env var, then the well-known
// Documents fallback); duplicated because test projects can't
// reference each other's helpers.
var fromEnv = Environment.GetEnvironmentVariable("ACDREAM_DAT_DIR");
if (!string.IsNullOrWhiteSpace(fromEnv) && Directory.Exists(fromEnv)) return fromEnv;
var def = Path.Combine(
Environment.GetFolderPath(Environment.SpecialFolder.UserProfile),
"Documents", "Asheron's Call");
return Directory.Exists(def) ? def : null;
}
[Fact]
public void Bake_SameIdSet_DifferentThreadCounts_ByteIdenticalPaks() {
var datDir = ResolveDatDir();
if (datDir is null) return; // dats absent (CI) — skip, matching suite convention
// Small mixed fixture: GfxObjs (incl. the #119 tricky ids), Setups
// (door setup carries emitters -> exercises the side-staged preload
// path), and a Holtburg EnvCell. Big enough to span multiple asset
// types; small enough to run in seconds.
var ids = new HashSet<uint> {
0x01000001u, 0x010002B4u, 0x010008A8u, 0x010014C3u,
0x02000001u, 0x020019FFu, 0x020005D8u,
0xA9B40100u, 0xA9B40101u,
};
var pathA = NewTempPakPath();
var pathB = NewTempPakPath();
// DIFFERENT thread counts on purpose: thread-completion order was the
// nondeterminism source the sorted-batching fix removes — identical
// bytes must hold regardless of parallelism.
int rcA = BakeRunner.Run(new BakeOptions { DatDir = datDir, OutPath = pathA, IdFilter = ids, Threads = 8 });
int rcB = BakeRunner.Run(new BakeOptions { DatDir = datDir, OutPath = pathB, IdFilter = ids, Threads = 3 });
Assert.Equal(0, rcA);
Assert.Equal(0, rcB);
var bytesA = File.ReadAllBytes(pathA);
var bytesB = File.ReadAllBytes(pathB);
Assert.True(bytesA.Length == bytesB.Length,
$"pak sizes differ between runs: {bytesA.Length} vs {bytesB.Length} bytes");
Assert.True(bytesA.AsSpan().SequenceEqual(bytesB),
"two bakes of the same id set produced different bytes — the sorted-batching determinism guarantee is broken");
}
}

View file

@ -59,7 +59,11 @@ public sealed class PakEquivalenceTests {
var work = new List<(PakAssetType Type, uint FileId, ulong ExtractorId, bool IsSetup)>();
work.AddRange(gfxObjIds.Select(id => (PakAssetType.GfxObjMesh, id, (ulong)id, false)));
work.AddRange(setupIds.Select(id => (PakAssetType.SetupMesh, id, (ulong)id, true)));
work.AddRange(envCellIds.Select(id => (PakAssetType.EnvCellMesh, id, id | 0x1_0000_0000UL, true)));
// isSetup: false for EnvCell — matches the runtime's own request sites
// (WbMeshAdapter.IncrementRefCount/EnsureLoaded pass isSetup: false for
// every MeshRef id, incl. cell-geometry ids) and BakeRunner's call site.
// (Review finding 10 — the two call sites previously disagreed.)
work.AddRange(envCellIds.Select(id => (PakAssetType.EnvCellMesh, id, id | 0x1_0000_0000UL, false)));
// ---- LIVE extraction (golden) ----
var golden = new Dictionary<(PakAssetType, uint), ObjectMeshData>();