From 9db90e280dc0e007b77b840df8dc44a0cdc35a01 Mon Sep 17 00:00:00 2001 From: John Ky Date: Fri, 18 Sep 2026 15:39:14 +1000 Subject: [PATCH] docs(jq,yq): record getpath slice's alias cost as an accepted gap (#2641) getpath(["items", {slice}])'s array fast path (getpath_walk_cursor, src/jq/eval_generic.rs) is O(2^N) on #1804's alias fan-out shape when the sliced range contains the aliased element, because it materializes the element's expanded value the same way to_owned_cursor always has -- #1804's is_alias() short-circuit is a validation-walk property (only needs to know a node exists) and cannot transfer to a walk that has to build the value itself. Every other value-producing route pays the identical cost (eval_slice_expr, array construction), so this is inherent to the copy model, not a bug in this arm. Records the rule and the measured cost in docs/compliance/yq/limitations.md's #1804/#2476/#2173 history block, notes it at the slice arm itself, and pins the two cases that do stay flat (out-of-range slice, navigated index) as timing-guard tests so a future regression back to O(2^N) on those shapes is caught. Fixes #2641 --- docs/compliance/yq/limitations.md | 25 +++++++++++ src/jq/eval_generic.rs | 11 +++++ tests/yq_cli_tests.rs | 69 +++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+) diff --git a/docs/compliance/yq/limitations.md b/docs/compliance/yq/limitations.md index 7086b688d..1fca093a0 100644 --- a/docs/compliance/yq/limitations.md +++ b/docs/compliance/yq/limitations.md @@ -257,6 +257,31 @@ on #1804's fan-out shape (`aN: &aN [*a(N-1), *a(N-1)]`) the ambient materializat `O(2^N)`, measured at 0.33 s / 1.31 s / 5.38 s / 22.70 s for N=18/20/22/24 and flat 0.00 s after. +**Accepted gap ([#2641](https://github.com/rust-works/succinctly/issues/2641)): a +value-producing read of an alias-bearing node costs its expanded size, on every route.** +#1804/#2476's `is_alias()` short-circuits are a property of *validation* walks, which only +ever need to know a node exists, never what it contains — they do not, and cannot, transfer +to a walk that has to build the value itself. `getpath(["items", {"start":s,"end":e}])`'s +array-slice fast path (`getpath_walk_cursor`, `src/jq/eval_generic.rs`, added by #2604) calls +`to_owned_cursor` once per sliced element, and `to_owned_cursor_at_depth` expands an alias +into a copy of its target (the copy model this file's alias section describes) — on #1804's +fan-out shape, the expanded value of `*aN` *is* `2^N` leaves, so materializing it costs +`2^N` regardless of how directly the walk reaches it. The same cost is paid, identically, by +every other route that materializes the same element: a literal-bound slice +(`.items[(0+0):(1+0)]`, via `eval_slice_expr` → `slice_one_generic` → `to_owned_all`) and +array construction (`[.items[]]`, #2575's territory). None of this is a walk bug to fix — +only a *navigating* read (`getpath(["items", 0])`, a `length`/`select` truthiness test) stays +flat, because it never has to copy the aliased subtree; a read that hands back the subtree's +own value cannot avoid paying for its expanded size. Measured on the same fan-out shape, +release binary: the in-range slice `getpath(["items",{"start":0,"end":1}])` (aliased element +inside the range) took 0.02 s / 0.06 s / 0.24 s / 0.96 s at N=14/16/18/20 — the same `×2`- +per-level curve as #1804's walk cost, before that walk was ever involved — while the +out-of-range slice `getpath(["items",{"start":1,"end":3}])` and the navigated read +`getpath(["items", 0])` both stayed at 0.00 s through N=20. Real `yq` v4.53.3 answers the +in-range case in 0.01 s at N=20 regardless, because go-yaml keeps `*aN` as a reference and +never expands it — closing this gap for succinctly would need an `OwnedValue` able to hold a +shared subtree instead of a copy (the direction ADR-0017 and #1416 point at), not a walk fix. + **Resolved ([#1350](https://github.com/rust-works/succinctly/issues/1350)).** `enforce_anchor_soundness` takes a `sort_keys` argument and has always handled it correctly on the DOM path; the cursor-streaming path used to never call it, reproducing the unsound diff --git a/src/jq/eval_generic.rs b/src/jq/eval_generic.rs index 144a9372f..188e02a30 100644 --- a/src/jq/eval_generic.rs +++ b/src/jq/eval_generic.rs @@ -17872,6 +17872,17 @@ fn getpath_walk_cursor( // all 300K elements first, the exact whole-container cost // this function exists to avoid (see its own doc comment // above). + // + // #2641: this loop still costs the *expanded* size of a + // YAML-aliased element inside the sliced range -- `to_owned_cursor` + // copies through an alias the same way #1804's fan-out shape + // makes exponential, and unlike #1804's `select`/`if` walk this + // one cannot short-circuit at the alias, because it has to + // hand back the subtree's own value, not just test that it + // exists. Accepted as inherent to the copy model, not a bug in + // this arm -- see docs/compliance/yq/limitations.md's + // #1804/#2476/#2173 history block for the measured cost and + // why every other value-producing route pays the same price. if let Some(elements) = v.as_array() { let len = match elements.len_checked() { Ok(len) => len, diff --git a/tests/yq_cli_tests.rs b/tests/yq_cli_tests.rs index be58913ad..817105717 100644 --- a/tests/yq_cli_tests.rs +++ b/tests/yq_cli_tests.rs @@ -48227,3 +48227,72 @@ fn test_yq_length_of_owned_boolean_3093() -> Result<()> { Ok(()) } + +/// #2641: `getpath_walk_cursor`'s array-slice fast path (`src/jq/eval_generic.rs`, +/// added by #2604) stays `O(doc)` -- flat, not exponential -- on #1804's +/// alias fan-out shape (`aN: &aN [*a(N-1), *a(N-1)]`) as long as the sliced +/// range does not contain the aliased element. This is the accepted-gap half +/// of the story: unlike #1804's `select`/`if` walk, this one *cannot* +/// short-circuit at an alias when the range does include it, because a slice +/// has to hand back the element's own value, not just test that it exists +/// (see docs/compliance/yq/limitations.md's #1804/#2476/#2173 history block). +/// N=30 is deep enough that a regression back to `O(2^N)` would not finish in +/// any reasonable bound, not merely run slow -- same margin rationale as +/// `test_select_and_if_truthiness_flat_over_alias_fanout_1804`. +#[test] +fn test_getpath_slice_out_of_range_flat_over_alias_fanout_2641() -> Result<()> { + let mut doc = String::from("a0: &a0 [1]\n"); + for i in 1..=30 { + doc.push_str(&format!("a{i}: &a{i} [*a{}, *a{}]\n", i - 1, i - 1)); + } + doc.push_str("items: [*a30, 2, 3]\n"); + + let start = std::time::Instant::now(); + let (stdout, stderr, code) = run_yq_stdin_with_stderr( + r#"getpath(["items",{"start":1,"end":3}]) | length"#, + &doc, + &["--jq-extensions"], + )?; + let elapsed = start.elapsed(); + assert_eq!(code, 0, "stderr: {stderr:?}"); + assert_eq!(stdout.trim(), "2"); + assert!( + elapsed < std::time::Duration::from_secs(15), + "an out-of-range getpath slice over a 30-level alias fan-out took {elapsed:?} \ + -- O(2^N) blowup regressed into the range this slice never touches" + ); + + Ok(()) +} + +/// #2641's other half: a *navigated* `getpath` read (a plain index, not a +/// slice) never materializes anything -- it walks to the element and hands +/// back a cursor -- so it stays flat over the same fan-out shape even when +/// the path runs straight through the aliased element, unlike the slice case +/// above whose in-range cost is exponential by construction (not tested here +/// for that reason -- see docs/compliance/yq/limitations.md). +#[test] +fn test_getpath_navigated_index_flat_over_alias_fanout_2641() -> Result<()> { + let mut doc = String::from("a0: &a0 [1]\n"); + for i in 1..=30 { + doc.push_str(&format!("a{i}: &a{i} [*a{}, *a{}]\n", i - 1, i - 1)); + } + doc.push_str("items: [*a30, 2, 3]\n"); + + let start = std::time::Instant::now(); + let (stdout, stderr, code) = run_yq_stdin_with_stderr( + r#"getpath(["items", 0]) | length"#, + &doc, + &["--jq-extensions"], + )?; + let elapsed = start.elapsed(); + assert_eq!(code, 0, "stderr: {stderr:?}"); + assert_eq!(stdout.trim(), "2"); + assert!( + elapsed < std::time::Duration::from_secs(15), + "a navigated getpath index over a 30-level alias fan-out took {elapsed:?} \ + -- O(2^N) blowup regressed" + ); + + Ok(()) +}