Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions docs/compliance/yq/limitations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions src/jq/eval_generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17872,6 +17872,17 @@ fn getpath_walk_cursor<S: EvalSemantics, V: DocumentValue>(
// 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,
Expand Down
69 changes: 69 additions & 0 deletions tests/yq_cli_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Loading