Repository navigation
Allocate the deep copy's key scratch space with the provided allocator - #5573
Merged
Merged
Conversation
The iterative deep copy builds each object's keys in a temporary vector of key/value pairs before handing them to the object's range constructor. That vector holds basic_json values, so like the values themselves it now uses AllocatorType instead of std::allocator. Also document that AllocatorType covers the JSON values, while most temporary storage still uses std::allocator. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
added
the
review needed
From C++23 on, libc++'s containers allocate through allocate_at_least when the allocator has one. The test allocator inherited it from std::allocator, so the scratch allocations were not counted and the test failed on Xcode. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
gregmarr
approved these changes
Sep 25, 2026
nlohmann
removed
the
review needed
Closed
1 of 2 tasks
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
2 participants
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
Follow-up to @gregmarr's question in #4843: should the temporary vectors added by the recent non-recursive rewrites also use the provided allocator?
Most of them hold only pointers, iterators and counters: the copy worklist and the stacks for merging (#5547), hashing (#5546),
dump()(#5285) and the binary readers (#5505). That matches the parser'sref_stack, the lexer'stoken_stringand the other internal bookkeeping, which have always usedstd::allocator.The exception is
copy_scratch_t(from #5389). The iterative deep copy uses it to build an object's keys asstd::pair<key_type, basic_json>values before passing them to the object's range constructor. It holds JSON values, so this PR allocates it withAllocatorType, the same way #4843 does for the destructor's stack.Changes
copy_scratch_tnow usesAllocatorType<std::pair<typename object_t::key_type, basic_json>>.unit-allocator.cpp: "deep copy uses the provided allocator". It deep-copies an object nested 300 levels deep with an allocator that counts allocations ofstd::pair<Key, T>with a non-const key. The object types only storestd::pair<const Key, T>, so the only such allocations come from the scratch space. The test fails without the change and passes with it under C++11 and C++20, and withJSON_NO_THREAD_LOCAL.template_parameters.md: adds the newAllocatorTypeinstantiation to the list, and states thatAllocatorTypeallocates the JSON values, while most temporary storage usesstd::allocator.One more candidate is left:
json_sax_dom_callback_parser::duplicate_key_stashholds values as well but still usesstd::allocator. It's only used by the callback parser when there are duplicate keys, and it lives in a class templated onBasicJsonType, so switching it would needrebind_alloc. I left it out.Breaking changes
No breaking changes to the public API.
AllocatorTypeis now also instantiated withstd::pair<StringType, basic_json>. This only affects deep copies of values nested deeper than 128 levels, or any depth whenJSON_NO_THREAD_LOCALis defined. Any allocator template usable asAllocatorTypetoday already has to accept arbitrary single type arguments.🤖 Generated with Claude Code