feat: add next! and prev! for in-place LazyNode traversal#59
Open
mathieu17g wants to merge 1 commit into
Open
Conversation
`next(o::LazyNode)` allocates a fresh `LazyNode` on every call, which is fine for occasional use but adds up sharply when a downstream package walks a large document — e.g. extracting all `Placemark` elements from a 50 MiB KML can allocate ~1 M `LazyNode` wrappers in the iterator alone (~38 MiB cumulative on a single benchmark run). Add a strictly-additive in-place pair, `next!(o)` / `prev!(o)`, that mutates `o` to point at the next/previous node and returns `o` (or `nothing` at the document boundary). Exported alongside `next` / `prev`. The aliasing trade-off is documented in the docstring: callers must not retain references to a previous position unless they explicitly snapshot with `LazyNode(o.raw)`. The existing `next` / `prev` methods are unchanged; this is purely opt-in API surface for hot paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
next(::LazyNode)allocates a freshLazyNodewrapper on every call. Consumers walking large documents — e.g. extracting everyPlacemarkfrom a 50 MiB KML — can churn ~1 M wrappers per traversal (~38 MiB cumulative). This PR addsnext!(o)/prev!(o)that mutateoin place and return it, ornothingat the document boundary. Functionally equivalent too = next(o), zero per-step allocation.Why this is safe
Strictly additive:
next/prevare unchanged, callers opt in. The aliasing trade-off (ois the same object across calls, so a retained reference would silently track the new position) is documented inline; the docstring points readers needing a snapshot atLazyNode(o.raw).Why it matters
Measured on FastKML.jl extracting a
DataFramefrom a 47 MiB sample KML (~1 MRawnodes traversed): the per-step allocation site atnext(::LazyNode)was contributing ~38 MiB; switching the consumer's traversal loop tonext!drops that to zero with no functional change. Independent of (and stackable with) thenext_no_xml_spacectx fix in #58.Verification
Full test suite passes (Julia 1.12), including a new
LazyNode next! / prev!testset covering: functional equivalence withnext, identity (next!(o) === o), memoization-field reset on advance,nothingat the document boundary, andprev!symmetry.