Repository navigation
perf(yq): build a write's pristine presentation tree without the subtrees that say nothing (#6880) - #6944
Conversation
…rees that say nothing (#6880) A YAML write builds the document's presentation tree and then a second one for the result: a node for every node of the document, even where there is no comment, style, anchor, tag or spelling to carry. The pristine snapshot now holds the one-word empty tree for each subtree that says nothing, collapsed by the parent as the walk finishes the child (so `- # note` is attached first), and reconcile_presentation answers an empty pristine container of the same kind with the empty tree instead of walking it. Not when something reads or writes the tree by position or shape: a document with an alias, a metadata write, a tag write, or a plain = whose right side is not a literal.
…irst version's slowdown
…ld for its first member (#6880 review) -P defers a flow collection's trailing comment to the next item's first member, and head_comment_first_member only matched Object and Array trees, so a silent block item (now one empty leaf) dropped the comment. A leaf over a non-empty collection now grows the one node that holds it, which also fixes the same loss for an item a write changed the kind of, where yq keeps the comment. Also: one named is_silent_leaf predicate, a SilentSubtrees enum in place of a bool next to under_alias, collapse_if_silent private, depth-guard tests that recurse the dense arms again, one test snapshot helper, and CLI pins in tests/yq_cli/i6880.rs.
…override for the faster write row (#6880 review) users_yq_del_select does about 11% less work (instruction counts, both architectures), which the guard fails on in either direction. Unit pins for assign_reads_tree_by_shape and for the shortcut having fired, one CLI row for the metadata-write gate, and the changelog and plan claims restated (9-25% faster, and the one output change).
…#6880 review) A Leaf over a collection is also what a write that changed a node's kind leaves, and the two are not the same to head_comment_first_member: growing every such Leaf fixed the -P comment loss but changed output where main matched yq by accident (a held comment that did not reach a non-adjacent kind-changed item), found by a 29k-run structural fuzz classified against yq. CommentTree::Silent(SilentKind) stands for the dense, all-empty tree of the collection and unfolds on a write, so every site that matches the tree's shape is now a compile error to decide, the reconcile shortcut is exact, and a Leaf behaves as before.
…ir its plan bullet
679e4d3 to
3552c3f
Compare
…pshot is taken, drop dead Silent arms, narrow visibility The prune gate walked the expression on every call, reads included; it is now a closure the snapshot calls. Silent arms that no gated pass can reach are gone (exhaustive patterns stay), materialize and is_silent are private and set_key_comment only unfolds an object, the perf-guard bound for users_yq_del_select is 15 (it moved -11%), the test row carries whether it expects a silent part, and the changelog says 8-17% faster, 25% with -P.
CoverageTotal: 92.98% ⚪ 0.01 pp vs Comparing
🔇 0 ignored region(s), 683 tolerated region(s)
643 more not shown: the full report is in the Touched files absent from every coverage report (5)
These files may contain no executable code or may have been omitted from the coverage run. Patch coveragePatch: 96.91% (314/324 new lines covered)
Uncovered new lines (10)
Indirect coverage changes🔴 0 lines lost coverage, 🟢 3 lines gained coverage on unchanged code. Indirect changes
📦 Full per-file coverage summary · run summary Baseline: the report published for |
CoverageTotal: 93.03% ⚪ 0.01 pp vs Comparing
🔇 0 ignored region(s), 682 tolerated region(s)
642 more not shown: the full report is in the Touched files absent from every coverage report (5)
These files may contain no executable code or may have been omitted from the coverage run. Patch coveragePatch: 96.91% (314/324 new lines covered)
Uncovered new lines (10)
Indirect coverage changes🔴 0 lines lost coverage, 🟢 3 lines gained coverage on unchanged code. Indirect changes
📦 Full per-file coverage summary · run summary Baseline: the report published for |
Summary
A YAML write builds the document's presentation tree (comments, styles, anchors) and then a second one for its result: a node for every node of the document, even where there is nothing to carry. This holds one word,
CommentTree::Silent, for each collection that says nothing.to_owned_with_comments_pruned), after the comments that belong to it are attached (- # notekeys on the tree lining up with its value).reconcile_presentationreturnsSilentfor a collection whose pristine tree isSilentand whose result is a collection of the same kind, instead of walking it, so the result's tree is never built either. The nesting-depth guard still runs over the result.style =,head_comment =,tag =), and a plain=whose right side is not a closed literal (.c = .acopies the source subtree's presentation).Silentis its own variant and not aLeaf: aLeafover a collection is also what a write that changed a node's kind leaves, and the two are not the same to-P's deferral of a flow collection's trailing comment (head_comment_first_member). ReusingLeafwas this PR's first review finding; the variant makes every site that matches the tree's shape a compile error to decide.Output is byte-identical to
main(see the gates below). Fixes #6880.Measurements
Interleaved release A/B against
main@13def7c58(builds fromgit archive, an A/A control of identical source built twice),.zzz = 1with-o yamlunless noted. Peak RSS undermemcap.py, median of 3 (5 at 20 MiB on the M4 Pro); times are medians of 5.users20 MiBusers20 MiB,-Pusers20 MiB,del(.users[0])users20 MiB,|= "X"users100 MiBcomprehensive100 MiBnested100 MiBCaveats, stated rather than buried:
users: 10.38 s to 13.43 s) while saving the same memory: it collapsed the finished tree in a pass, which freed millions of cold nodes. Collapsing each child as it is finished (this version) frees it while it is cache-hot and is 10-25% faster on the 7950X and 8-14% on the M4 Pro..fast path, JSON-output write,anchors,flow,strings,sequencesand the gated.c = .users[0]copy are within the 5 MiB that two builds of the same source differ by.navigationsaves 3%: it carries a quoted timestamp in every record, so no record is silent (perf(yq): a record with one field that says something keeps a presentation node for every field of it (#6880 follow-up) #6955).nested(the same binary read 267-271 or 365-378 MiB at 20 MiB, and 1145 to 1553 at 100 MiB). macOS keeps freed pages resident until the kernel purges them; read the 7950X column, whose runs agree to a MiB, for the size of the saving. The M4 Pro also carried other sessions' load while timed (load average 1-2), so its times are interleaved ratios, not quiet-box figures.Review of this PR (
/code-review max) and what came of itA real defect, fixed: with
-P, a flow collection's trailing comment was dropped when the next item was a silent block collection. EverySilentsite is now decided by the compiler,head_comment_first_membergrows the one node that holds the comment, andtests/yq_cli/i6880.rspins it (it fails with the growth disabled; verified by mutation). Also done: the test module'sOutputConfigliteral no longer breaks on a new field (one helper, after rebasing onto #6935), the depth-guard tests recurse the dense arms again (a tree that says something at every level), theperf-guard.pyoverride forusers_yq_del_select(-11% instructions on both architectures), a named predicate and an enum in place of aboolbesideunder_alias, and the changelog restated. The identity claim the review falsified is replaced by the gates below.My own follow-up to that review: a 29k-run structural fuzz, classified against yq, showed that growing every
Leaf(the first fix) changed ten outputsmaingets right by accident; that is whySilentexists. The cases wheremainis wrong are #6954.Test plan
cargo test --features cli,simd,regex,serde(full suite) on the final commitscripts/pre-push.sh --full: ledger, changelog, fmt, release build, three clippy variants, rustdoc x3,cargo test --no-default-featuresdel,|=,+=,*=, copies, constructions,-P, metadata writes, reads) x 15 hand-written documents (plain, head comment, line comment, quoted, flow, tagged, anchored, multi-document, empty, keys) and 8 generated 1 MiB ones x 6 flag sets: 0 differences; and 16,200 runs of a structural fuzz (flow collections with trailing comments beside silent block items,- # noteelements, nested sequences, 7 writes,-Pand not): 0 differencesSilentreads as the dense tree and unfolds on a write, the pruned snapshot against the dense one throughreconcile_presentationandemit_yaml_value(and that the shortcut fired), the gate predicate, the- # noteelement, the depth guardstests/yq_cli/i6880.rs, rows captured from yq v4.53.3 (mutation-checked: the growth, and the metadata-write gate)/code-review ultrafor the deepest pass, if a human wants one