Remove redundant copies from the asset parse path

[briaguya0]

Two redundant copies in the asset parse path. Extracting Banjo-Kazooie US v1.0 goes from 28.3s to 3.0s.

SegmentConfig deep copy — GetFileOffsetFromSegmentedAddr and GetFileOffsetFromCompressedSegmentedAddr both did auto segments = this->gConfig.segment;, copying four unordered_maps (one keyed by every file) on every address resolution. The copy only existed so operator[] could be used from a const method; find() on the member does the same job. 28.3s → 18.9s.

Dead ROM copy — AddAsset did auto rom = this->GetRomData();. GetRomData() returns std::vector<uint8_t>&, but auto deduces a value, so this deep-copied the whole 16MB ROM once per asset. rom was never read. -Wunused-variable stays quiet because std::vector has a non-trivial ctor/dtor. 18.9s → 3.0s.

No behaviour change: o2r archives compared by per-entry CRC and uncompressed size (byte comparison is useless — zip timestamps), 16,937 entries identical before and after.

Full benchmark + profiling writeup

Asset extraction benchmark (2026-08-12)

Environment: Linux x86-64, 32 cores, GCC, glibc malloc.
ROM: Banjo-Kazooie US v1.0 (1fe1632098865f639e22c11b9a81ee8f29c75d7a).

What's being measured

The target is the in-app extraction users hit on first launch:
GameExtractor::GenerateOTR (src/port/Extractor/GameExtractor.cpp:240), run on a
thread pool from Engine.cpp:781. Its actual work is Companion::Init(ExportType::Binary, ...)
(GameExtractor.cpp:366).

We measure it via the standalone torch o2r <rom> binary, which calls the same
Companion::Init. On Linux the in-app path is only reachable through GUI popups
(ES_EXTRACT → PS_FILE_CHECK → PS_FIRST), so it isn't scriptable without code
changes. The proxy was matched to the app on both axes that could skew it:

• Compiler flags — the app links torch via add_subdirectory(Torch)
  (CMakeLists.txt:509). Torch/CMakeLists.txt:243 sets CMAKE_CXX_FLAGS_RELEASE "-O3"
  in its own directory scope, so a Release app build compiles torch at -O3 —
  identical to build/torch-bench.
• Factory set — the app forces BUILD_BK64=ON and every other game plus
  BUILD_NAUDIO=OFF (CMakeLists.txt:500-508); tools/bench-extract.sh passes the
  same. Measured: no effect on runtime (28.28 s vs 28.37 s with everything on).

Remaining in-app-only cost, verified negligible: GenerateOTR walks the asset YAMLs
to size the progress bar (GameExtractor.cpp:246-313) — that's 3 files / 296 KB
under assets/yaml/us/rev0.

Timings

Release -O3 (== app Release), hyperfine, 5 runs each:

	wall
baseline	28.28 s ± 0.36
after fix 1 (SegmentConfig copy)	18.90 s ± 0.35 — 33% faster
after fix 2 (dead ROM copy)	3.00 s ± 0.01 — 9.4× faster than baseline

Peak RSS ~738 MB. 99% CPU on a 32-core box — extraction is single-threaded.
The one thread pool in Torch/src/Companion.cpp:1735 covers only the modding-export
path, not the Binary/o2r export the game uses. (BKAssetFactory.cpp:367 does run a
threaded pre-decompression pass, but it's a small slice of wall time.)

Fix 1 — SegmentConfig deep copy (done)

Torch/src/Companion.cpp, GetFileOffsetFromCompressedSegmentedAddr (hot, 35%
cumulative) and GetFileOffsetFromSegmentedAddr (same pattern, cold) both did:

auto segments = this->gConfig.segment;   // deep copy, every call
if (segments.compressed[this->gCurrentFile].contains(segment)) { ... }

SegmentConfig (Companion.h:57) holds four unordered_maps, one a map-of-maps
keyed by every file. The copy existed only so operator[] could be used from a
const method. Replaced with find() on the member directly.

28.28 s → 18.90 s. Bigger than the ~20% the profile predicted, because the copy
was also driving nearly all allocator traffic: malloc fell 20.7% → 1.5% and
free dropped off the profile entirely.

Verified behaviour-preserving: o2r archives can't be compared byte-wise (zip
timestamps), so compare per-entry CRCs and sizes from the zip directory — 16,937
entries, all identical.

Fix 2 — dead per-asset copy of the entire ROM (done)

Companion::AddAsset, Torch/src/Companion.cpp:2540:

auto rom = this->GetRomData();   // 16 MB copy, per asset, never used

GetRomData() returns std::vector<uint8_t>&, but auto deduces a value, so
this deep-copied the whole 16 MB ROM. rom is never read anywhere in AddAsset
(lines 2514-2578) — it was pure dead weight. GCC doesn't warn because std::vector
has a non-trivial ctor/dtor, so -Wunused-variable stays quiet.

18.90 s → 3.00 s. Deleting one line.

How it was found

perf couldn't attribute it (inline frames at -O2 were nonsense), but the shape of
the profile gave it away: 14.3 s of memmove against only 1.5% malloc. Many small
vector copies would show proportional malloc traffic — so it had to be a few very
large copies. At ~10 GB/s, 14.3 s ≈ 140 GB moved; against ~16.9k assets that's about
one 16 MB ROM per asset. From there grep GetRomData() found the auto-by-value bug
directly, no heaptrack needed.

Ruled out along the way: BaseFactory::parse takes the buffer by reference
(BaseFactory.h:104), the call site passes the member (Companion.cpp:493), and
Decompressor::AutoDecode returns cached raw pointers (Decompressor.h:26).

Profile after both fixes (build/bench/report-*.txt)

%	symbol
42.3%	tdefl_compress (miniz)
13.9%	shared_ptr refcount release
6.1%	yaml-cpp node_data::get linear scan
5.3%	__libc_malloc2
4.9%	mz_crc32
1.1%	__memmove_avx512_unaligned_erms

Roughly half the run is now genuine zip compression (tdefl_* + mz_crc32), and
memmove has collapsed from 76% to 1%. This is a healthy profile.

Remaining leads, in rough order of size:

1. Compression, ~50%. Now the dominant cost. Either parallelize archive writing
   or drop the deflate level — worth checking what miniz level the o2r writer uses.
2. shared_ptr refcount churn, ~17% (release + addref). Atomic refcounting on
   IParsedData passed around by shared_ptr; reducible by passing raw/const&
   through the hot path.
3. Threading. Still single-threaded; at 3 s the payoff is much smaller than it
   looked at 28 s.

Side note: the ExtractAssets cmake target

Not the path users take, but CMakeLists.txt:743 ExternalProject_Add(TorchExternal ...)
passes no CMAKE_BUILD_TYPE, so make ExtractAssets builds an unoptimized torch:
measured 43.3 s vs 28.3 s. Adding -DCMAKE_BUILD_TYPE=Release to those
CMAKE_ARGS would cut it ~34%.

Reproducing

Harness (benchmark/profile script + the o2r content comparison used to verify these
changes): https://gist.github.com/briaguya0/135467a5243c4b28b94c16b2c6b4fdeb

Needs hyperfine and perf. Run from a Lighthouse checkout root:

./bench-extract.sh build      # release + profiling torch binaries
./bench-extract.sh bench 5    # hyperfine timing
./bench-extract.sh profile    # flat perf report
./bench-extract.sh callgraph  # cumulative/caller report
./bench-extract.sh flamegraph # perf script for speedscope

Reports are written to build/bench/. REUSE=1 re-renders them from existing
perf.data without re-recording.

To check a change is behaviour-preserving, compare o2r archives by per-entry CRC and
size (compare-o2r-contents.py), not by file hash — zip timestamps make byte
comparison useless.

---

🤖 Generated with Claude Code

[Malkierian]

Confirmed reduction of ~97% of extraction time on Windows release (from ~1:35 to ~3.5s) and for debug, a ~91% decrease (from ~4:30 to ~24s).