* Make diff() account for member order in ordered_json objects
diff() compared source/target objects purely by key set, ignoring
relative member order. For ordered_json (insertion-ordered, vector-
backed object_t), two objects that differ only in member order are
unequal via operator==, but diff() never emitted any patch operation
to fix the order, so source.patch(diff(source, target)) == target
could fail to hold.
Fix by detecting when common keys appear in a different relative
order in source vs. target (or when a new key would need to land
somewhere other than the end), and in that case removing and
re-adding the affected keys in target's order, which relies on
patch()'s "add" op appending new keys at the end of an ordered_map.
For plain json (std::map-backed, always key-sorted iteration) this
is a no-op and the original minimal per-key diff path is unchanged.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Avoid redundant lookups in diff()'s object-order tracking
The previous fix for ordered_json member order re-derived common-key
order and suffix information with extra target.find()/source.find()
calls layered on top of the pre-existing removed/added-key passes,
instead of reusing those same passes. This roughly tripled the number
of map lookups per diff() call for every object, including plain
`json`, where the reordering path is never taken.
Piggyback the order tracking (and the "add" op construction for new
keys) onto the two passes the algorithm already needs to detect
removed/added keys, and walk the fast path's recursion in lockstep
with the precomputed common-key list instead of re-querying `target`.
This restores diff() to its pre-existing lookup count; benchmarked at
n=1000 keys, ordered_json::diff() was roughly 2x slower than baseline
before this change and is back within noise of baseline after it.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Preserve diff()'s original op ordering and fix a slow-path deletion gap
Splitting removed-key detection and common-key recursion into separate
passes (for the earlier lookup-count fix) changed the emitted patch's
op order: all "remove" ops now came before all recursive per-key diffs,
instead of interleaved in source's iteration order as the original
implementation did. This broke docs/mkdocs/docs/examples/diff.output's
exact-match CI check (ci_test_examples) even though the patch was still
semantically correct.
Defer "remove" emission into the same walk that does the recursive
diffs, so common keys and deleted keys are interleaved in source order
again, matching historical output.
While restructuring that walk, the reordering ("slow path") branch was
only emitting "remove" for keys common to both objects, never for keys
present in source but genuinely absent from target -- a key deleted
alongside an actual reorder would silently survive the patch. Fixed by
removing every source key in the slow path (both deleted and common
keys need removing there; common keys are then re-added in target's
order). Verified with a targeted reorder+deletion case and a fresh
20,000-case round-trip fuzz run (0 failures).
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
---------
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
basic_json's copy constructor copied objects and arrays by handing the
container to its own copy constructor, which copy-constructs every element
and so reaches this constructor again, once per nesting level. A value
nested deeply enough exhausted the call stack and terminated the process
with a segmentation fault - no exception, nothing the caller could catch.
Parsing such a value works, as the parser is iterative, and so does
destroying one, as #1436 made destruction iterative.
Bound how far the copy descends rather than take the call stack away from
it. The first levels are copied exactly as they were - the containers copy
their own elements, which is by far the fastest way to fill them - and only
once the copy has descended 128 levels is the value below it finished
without the call stack, through an explicit worklist. Copying can therefore
no longer exhaust the stack, however deeply a value is nested, while a value
nested less deeply than the bound - all but a vanishing minority - is copied
by the very same code as before and pays only for one counter.
That counter lives in thread_local storage, as one shared between threads
would be raced. JSON_NO_THREAD_LOCAL switches it off for toolchains without
thread_local; copying then goes through the worklist right away, which
yields the same values but is measurably slower.
The deferred values are completed before the copy they belong to returns, so
a value copied while another copy is going on - by a custom base class, say -
is unaffected by the copy it is nested in.
operator= takes its argument by value, so copy assignment is fixed as well.
Copying is as fast as it was, within measurement noise (medians of 9
interleaved runs, clang -O3): -1.3% for an array of strings, +0.0% for a
flat object, +0.1% for a flat array of numbers, +0.3% for nested arrays,
+0.6% for nested objects and +1.2% for a twitter-like document. Copying a
three-key object costs about ten nanoseconds more, the counter. Deferring
every level instead, rather than only those below the bound, measured
between 3% and 9% slower depending on the shape of the value.
This fixes#5387 for the copy constructor. dump() is still recursive.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
When iteration_proxy_value<iter_impl<ordered_json>> appears in a context
that requires it to be complete (function or lambda parameter), the
compiler instantiates basic_json<ordered_map> and walks into
set_parents(iterator, typename iterator::difference_type)
while iterator is still incomplete, failing with "invalid use of
incomplete type".
basic_json::difference_type is already std::ptrdiff_t, so just naming
the underlying type directly avoids the dependent lookup. Behavior and
ABI are unchanged. This was the approach suggested in the issue thread.
Added a regression case in unit-ordered_json.cpp using the same trigger
pattern (lambda parameter naming the proxy type).
Fixes#3732
Signed-off-by: Akhilesh Arora <akhildawra@gmail.com>
Niels Lohmann
Akhilesh Arora
