Severity: Low (perf only; no incorrect output). Found during code review of #2575.
Summary
LazySeq::fold_one (src/jq/eval_generic.rs) always allocates a fresh vec![elem] per element, even when self.instructions is empty (no map stage to run). Iterator::next() then stores that 1-item Vec into self.pending and immediately pops it back out. For an instruction-free source (LazySource::Cursors with no stages — map(.)'s own bare identity map, #724/#725; and, since #2575, [.[]]/[.[] | select(f)] printed directly rather than consumed by length/first/.[n]), every single element pays one avoidable heap allocation it has no stage to justify.
materialize_atomic (src/jq/eval_generic.rs) already got its own instruction-free-Cursors fast path as part of #2575, bypassing this entirely for the "materialize into OwnedValue::Array" case. drain_atomic — used by src/bin/succinctly/jq_runner.rs's own GenericResult::LazySeq print arm (~line 4769), specifically because it needs to preserve each element's own cursor identity for streaming output rather than build an OwnedValue — still goes through the plain Iterator protocol and so still pays this cost.
Root cause
LazySeq::fold_one (eval_generic.rs):
fn fold_one(&self, elem: LazyElem<V>) -> Result<Vec<LazyElem<V>>, Control> {
let mut items = vec![elem];
for instr in self.instructions.iter().flat_map(|rc| rc.iter()) {
...
}
Ok(items)
}
Iterator::next() calls this unconditionally, whatever self.instructions holds, and stores the result into self.pending before popping the single item back out.
Why this is separate from #2575
#2575 made [...] return a LazySeq for a cursor-shaped inner result, which is a real, measured win for length/first/.[n]-shaped consumers (materialize_atomic's own new fast path, and fold_lazy_seq_stage's Phase 2 arms). But it also means more queries now reach drain_atomic's pre-existing (not introduced by #2575) per-element allocation when the array is printed directly with no further consumer — a cost map(.)'s own bare-print case already paid, unchanged, since #724/#725 landed. Fixing fold_one/Iterator::next() itself is a generally-applicable optimization benefiting both callers, not something scoped to #2575's own array-construction laziness — a separate, standalone change.
Suggested fix direction
Give LazySeq's Iterator impl (or drain_atomic/materialize_atomic directly) a fast path mirroring #2575's own materialize_atomic one: when self.instructions.is_none() and self.pending.is_empty(), next()/drain_atomic() can pull straight from self.source.advance() without ever routing through fold_one's per-element Vec wrapping — a LazySource-level "am I randomly-indexable and instruction-free" property, generalized beyond just LazySource::Cursors (LazySource::IndexRange has the identical precondition and could benefit too — [keys_unsorted] | last-shaped queries, noted separately during the same review).
Risks / open questions
Touches LazySeq's core Iterator impl, used by every consumer of the type, not just the two named here — needs the same care #2575 itself took (interleaved A/B on the pinned boxes, output-identity gated) before landing, since a subtle change to iteration order/error-propagation timing here would be a correctness bug, not just a missed optimization.
Class: Sonnet — a scoped, well-understood optimization to a single already-tested function, with an existing precedent (#2575's own materialize_atomic fast path) to mirror.
Dependencies / related: #2575 (widened exposure to this pre-existing cost), #724/#725 (map(.)'s own original LazySeq landing, which already pays this cost today).
Severity: Low (perf only; no incorrect output). Found during code review of #2575.
Summary
LazySeq::fold_one(src/jq/eval_generic.rs) always allocates a freshvec![elem]per element, even whenself.instructionsis empty (nomapstage to run).Iterator::next()then stores that 1-itemVecintoself.pendingand immediately pops it back out. For an instruction-free source (LazySource::Cursorswith no stages —map(.)'s own bare identity map, #724/#725; and, since #2575,[.[]]/[.[] | select(f)]printed directly rather than consumed bylength/first/.[n]), every single element pays one avoidable heap allocation it has no stage to justify.materialize_atomic(src/jq/eval_generic.rs) already got its own instruction-free-Cursorsfast path as part of #2575, bypassing this entirely for the "materialize intoOwnedValue::Array" case.drain_atomic— used bysrc/bin/succinctly/jq_runner.rs's ownGenericResult::LazySeqprint arm (~line 4769), specifically because it needs to preserve each element's own cursor identity for streaming output rather than build anOwnedValue— still goes through the plainIteratorprotocol and so still pays this cost.Root cause
LazySeq::fold_one(eval_generic.rs):Iterator::next()calls this unconditionally, whateverself.instructionsholds, and stores the result intoself.pendingbefore popping the single item back out.Why this is separate from #2575
#2575 made
[...]return aLazySeqfor a cursor-shaped inner result, which is a real, measured win forlength/first/.[n]-shaped consumers (materialize_atomic's own new fast path, andfold_lazy_seq_stage's Phase 2 arms). But it also means more queries now reachdrain_atomic's pre-existing (not introduced by #2575) per-element allocation when the array is printed directly with no further consumer — a costmap(.)'s own bare-print case already paid, unchanged, since #724/#725 landed. Fixingfold_one/Iterator::next()itself is a generally-applicable optimization benefiting both callers, not something scoped to #2575's own array-construction laziness — a separate, standalone change.Suggested fix direction
Give
LazySeq'sIteratorimpl (ordrain_atomic/materialize_atomicdirectly) a fast path mirroring #2575's ownmaterialize_atomicone: whenself.instructions.is_none()andself.pending.is_empty(),next()/drain_atomic()can pull straight fromself.source.advance()without ever routing throughfold_one's per-elementVecwrapping — aLazySource-level "am I randomly-indexable and instruction-free" property, generalized beyond justLazySource::Cursors(LazySource::IndexRangehas the identical precondition and could benefit too —[keys_unsorted] | last-shaped queries, noted separately during the same review).Risks / open questions
Touches
LazySeq's coreIteratorimpl, used by every consumer of the type, not just the two named here — needs the same care #2575 itself took (interleaved A/B on the pinned boxes, output-identity gated) before landing, since a subtle change to iteration order/error-propagation timing here would be a correctness bug, not just a missed optimization.Class: Sonnet — a scoped, well-understood optimization to a single already-tested function, with an existing precedent (#2575's own
materialize_atomicfast path) to mirror.Dependencies / related: #2575 (widened exposure to this pre-existing cost), #724/#725 (
map(.)'s own originalLazySeqlanding, which already pays this cost today).