Diff deeply nested values without recursing per nesting level (#5548)

* Diff deeply nested values without recursing per nesting level

diff() descended into both values once per nesting level, and compared
them with operator== on every level on the way, which recurses as well.
Values nested deeply enough - 25,000 levels on an 8 MiB stack - exhausted
the call stack and terminated the process, although parse() accepts
them without complaint. On such a chain the per-level comparisons and
path strings also made diff() quadratic in time and memory.

Both the recursion and operator== only descend as far as the source is
nested. So diff() first checks, recursing at most diff_depth_limit()
(128) levels, whether the source is nested more deeply than that. If not
- all but a vanishing minority of values - the recursive algorithm
diffs it exactly as before, now as diff_recursively(). Otherwise
diff_iteratively() walks the two values on an explicit stack, emitting
the same operations in the same order. It does not compare arrays and
objects with operator== up front (equal ones yield no operations
anyway), keeps the path in one buffer instead of a new string per
level, and hands every subtree that is not nested too deeply back to
diff_recursively(), so equal parts are still skipped quickly.

The check costs one pass over the source. On a 3,000-object document
that is about 30% of diffing two equal values (which is just an
operator== call), about 10% of diffing values that differ in a few
places, and noise when arrays change length. Once operator== no longer
recurses (#5390), the check can go.

Tests check that the patch reproduces the target at every depth up to
300, for json and ordered_json, including reordered members. They also
check the exact operation for a difference deep inside, and diff values
nested 100,000 levels deep.

Fixes #5393 for diff().

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Make diff_frame a member struct that declares its special members

GCC's -Weffc++ (an error in CI) asks a class with pointer members, a
user constructor and a non-trivial destructor to declare its copy
constructor and copy assignment; diff_frame's vector and basic_json
members make its destructor non-trivial. Declare all five as defaulted,
which also satisfies clang-tidy's special-member-functions check. Leave
their exception specifications implicit: GCC 4.8 rejects an explicit
one that differs from the implicit one, as it does for flatten_task in
#5517.

The converting constructor cannot throw, and is now declared noexcept
for GCC's -Wnoexcept, which flags the emplace_back() under C++26
otherwise. The struct also moves from diff_iteratively() into the class,
like dump_frame in the serializer.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Use the shared recursion limit in diff()

diff_depth_limit() is gone in favor of detail::recursion_depth_limit().

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Diff fewer nesting depths so the test does not time out under Valgrind

Checking every depth up to 300 made test-json_patch exceed the 1500 s ctest
timeout in ci_test_valgrind. Check the depths up to 16, those around the
recursion limit of 128, and 300 instead.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Mark the diff frame's value-initialized members for clang-tidy

The braces are kept for GCC's -Weffc++, as in json_sax.hpp.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Bound diff()'s descent with a depth count instead of scanning the source

Now that operator== no longer recurses (#5390), diff() can keep its per-level
equality shortcut all the way down. It diffs recursively for the first
detail::recursion_depth_limit() levels, as merge_patch() does, and hands
anything deeper to diff_iteratively(). The nesting_exceeds() scan, which
cost about 30% on equal documents, is gone, and diff() is on par with
develop again.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Note that the diff frame reference is invalidated by pop_back() too

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Keep diff()'s recursive levels small and its result elided

diff_recursively built every patch operation in place from initializer
lists. Unoptimized builds give each of those temporaries its own stack
slot, so every level of the bounded descent cost kilobytes of stack
(about 6 KB with clang -O0), and the 128 recursive levels overflowed the
1 MB stack of MSVC Debug in the "deeply nested values" test. The
operations and the key comparison of two objects are now built by
separate functions, which diff_iteratively shares, and both diff
functions append to one result instead of returning a patch per level
that the caller copies. With clang -O0, diffing values nested 300 levels
deep now peaks at about 190 KB of stack instead of 880 KB.

Since diff() now owns the only returned value, clang's -Wnrvo no longer
reports the returns of diff_recursively, which alternated between the
local patch and diff_iteratively's result.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Copy the diff frame's members instead of holding a reference to it

The loop in diff_iteratively held a reference to the top frame, which
enter() invalidates when it pushes and the end of the loop invalidates
when it pops. Nothing used it afterwards, but a later change could. As in
the other iterative walks, the members the loop reads are now copied out
as constants and the ones it advances are changed through stack.back().
The frame as a whole is not copied: it holds the common keys and the
"add" operations of an object.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This commit is contained in:
Niels Lohmann
2026-10-01 07:29:32 +02:00
committed by GitHub
parent a32f61eb98
commit 18dd5663b0
3 changed files with 995 additions and 364 deletions

View File

@@ -15,8 +15,65 @@ using nlohmann::json;
#endif
#include <fstream>
#include <string>
#include <vector>
#include "make_test_data_available.hpp"
namespace
{
// alternating objects and arrays nested `depth` levels deep, with members that
// depend on `variant` at some levels, so diffing two variants yields
// operations on many levels: replacing the innermost value, adding, removing,
// and (for ordered_json) reordering members, and changing array lengths
template<typename BasicJsonType>
BasicJsonType nested(const std::size_t depth, const int variant)
{
BasicJsonType value = variant;
for (std::size_t i = 0; i < depth; ++i)
{
if (i % 2 == 0)
{
BasicJsonType object = BasicJsonType::object();
if ((i + static_cast<std::size_t>(variant)) % 7 == 0)
{
object["x"] = i;
}
if (variant == 2 && i % 11 == 0)
{
object["z"] = "z";
}
object["a"] = std::move(value);
if (variant == 1 && i % 5 == 0)
{
object["y"] = 1;
}
value = std::move(object);
}
else
{
BasicJsonType array = BasicJsonType::array({std::move(value)});
if ((i + static_cast<std::size_t>(variant)) % 3 == 0)
{
array.push_back(i);
}
value = std::move(array);
}
}
return value;
}
// a path of `depth` reference tokens, as nested() nests its values
std::string nested_path(const std::size_t depth)
{
std::string path;
for (std::size_t i = depth; i > 0; --i)
{
path += (i - 1) % 2 == 0 ? "/a" : "/0";
}
return path;
}
} // namespace
TEST_CASE("JSON patch")
{
SECTION("examples from RFC 6902")
@@ -1752,6 +1809,102 @@ TEST_CASE("JSON patch - diff emits array removals in descending index order")
}
}
TEST_CASE("JSON patch: diff of deeply nested values")
{
SECTION("the diff reproduces the target at every depth")
{
// depths on either side of the nesting depth up to which diff()
// recurses (detail::recursion_depth_limit(), 128); not every depth up
// to 300, as the test would then time out under Valgrind
std::vector<std::size_t> depths;
for (std::size_t depth = 0; depth <= 16; ++depth)
{
depths.push_back(depth);
}
for (std::size_t depth = 120; depth <= 136; ++depth)
{
depths.push_back(depth);
}
depths.push_back(300);
for (const auto depth : depths)
{
CAPTURE(depth);
for (int from = 0; from < 3; ++from)
{
for (int to = 0; to < 3; ++to)
{
CAPTURE(from);
CAPTURE(to);
const auto source = nested<json>(depth, from);
const auto target = nested<json>(depth, to);
const auto patch = json::diff(source, target);
CHECK(source.patch(patch) == target);
CHECK(patch.empty() == (from == to));
const auto ordered_source = nested<nlohmann::ordered_json>(depth, from);
const auto ordered_target = nested<nlohmann::ordered_json>(depth, to);
CHECK(ordered_source.patch(nlohmann::ordered_json::diff(ordered_source, ordered_target)) == ordered_target);
}
}
}
}
SECTION("a difference only in the innermost value is one replace operation")
{
for (std::size_t depth = 0; depth <= 300; ++depth)
{
CAPTURE(depth);
json source = 1;
json target = 2;
for (std::size_t i = 0; i < depth; ++i)
{
source = i % 2 == 0 ? json::object({{"a", std::move(source)}}) : json::array({std::move(source)});
target = i % 2 == 0 ? json::object({{"a", std::move(target)}}) : json::array({std::move(target)});
}
CHECK(json::diff(source, target, "/root") == json::array({{{"op", "replace"}, {"path", "/root" + nested_path(depth)}, {"value", 2}}}));
}
}
SECTION("values nested too deeply for the call stack (#5393)")
{
// diff() used to recurse once per nesting level, and compared the
// values with operator== on every level. The values are only
// parsed and diffed, never copied or compared, since those recurse
// too.
const std::size_t depth = 100000;
for (const bool objects :
{
false, true
})
{
CAPTURE(objects);
std::string source_text;
std::string target_text;
std::string equal_text;
std::string path;
for (std::size_t i = 0; i < depth; ++i)
{
source_text += objects ? "{\"a\":" : "[";
path += objects ? "/a" : "/0";
}
target_text = source_text + "2";
equal_text = source_text + "1";
source_text += "1";
const std::string closing(depth, objects ? '}' : ']');
const auto source = json::parse(source_text + closing);
const auto patch = json::diff(source, json::parse(target_text + closing));
REQUIRE(patch.size() == 1);
CHECK(patch[0]["op"] == "replace");
CHECK(patch[0]["path"] == path);
CHECK(patch[0]["value"] == 2);
CHECK(json::diff(source, json::parse(equal_text + closing)).empty());
}
}
}
TEST_CASE("JSON patch - diff() takes the fast path for non-reorderable object types (regression #5639)")
{
// #5465 added an order check to diff()'s object handling so a