Skip to content

yaml: fold merge-key property budget into alias-expansion budget - #32385

Open
robobun wants to merge 4 commits into
mainfrom
farm/9d093b8c/yaml-merge-budget
Open

yaml: fold merge-key property budget into alias-expansion budget#32385
robobun wants to merge 4 commits into
mainfrom
farm/9d093b8c/yaml-merge-budget

yaml: drop redundant merge-key property budget

5262ac4
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jun 16, 2026 in 23m 13s

Code review found 1 important issue

Found 3 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/parsers/yaml.rs:3284-3285 Nested inline merge keys can amplify memory beyond alias_expansion_budget
🟡 Nit test/js/bun/yaml/yaml.test.ts:4513 120s test timeout — workload could be reshaped to fit 30s convention

Annotations

Check failure on line 3285 in src/parsers/yaml.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Nested inline merge keys can amplify memory beyond alias_expansion_budget

Removing `merge_props_budget` opens a memory-amplification gap: nested inline merge wrappers around a single alias — `{<<: {<<: ... {<<: *big}}}` — re-materialize the anchor's K properties at every nesting level D, allocating D×K `G::Property` structs while `alias_expansion_budget` is charged only once (~2K). With K=100,000 and D in the hundreds (bounded only by `StackCheck`), a ~1.5 MB document produces tens of millions of arena allocations that the old 1M cap rejected. Consider decrementing `a

Check warning on line 4513 in test/js/bun/yaml/yaml.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

120s test timeout — workload could be reshaped to fit 30s convention

nit: the 120s timeout is 4× the file's 30s convention, and CLAUDE.md says "Don't raise per-test timeouts to make a slow test pass; shrink the workload." The test only needs to materialize >1,048,576 merged properties to guard against regression — e.g. `keyCount = 1024` × `mergeCount = 1030` crosses the same threshold with ~16× fewer mappings/JS objects and a ~10× smaller document, and should fit comfortably under 30s on debug+ASAN.