Speed up dump(), and keep it from overflowing the stack - #5285
Conversation
5cbc06c to
95662c8
Compare
4183304 to
c4a3b4d
Compare
45c47ef to
bb8c130
Compare
Quick review notes
Otherwise, the SWAR scanner and buffering logic look solid. 👍 |
|
Thanks for your feedback!
|
There was a problem hiding this comment.
not guaranteed to be big enough (handled by #5186).
There was a problem hiding this comment.
Right, and it turned out to be worth chasing: indent_string is gone as of a1e9147.
put_indent() now memsets the indentation straight into the write buffer, filling and flushing it as needed, so there is no auxiliary string to size, grow, or keep in sync with the deepest nesting level reached. That removes both halves of #5186 structurally rather than patching the growth rule — the doubling that isn't enough when indent_step more than doubles the string, and the ' ' literal in the resize instead of indent_char.
Both were reproducible against develop:
json{{"a", 1}}.dump(2000)—indent_stringstarts at 512, doubles once to 1024, then 2000 bytes are read from it. Right length, garbage content: a heap over-read.json{{"a", 1}}.dump(600, '\t')— the part past 512 comes back as spaces, not tabs.
Both now pass, and there are tests for them (plus one for nesting whose accumulated indentation spans several buffer-fulls). next_indent() keeps #5186's assertion that the unsigned indentation accumulation hasn't wrapped on deep nesting, since that one is independent of how the characters get written.
@nlohmann — this does mean #5186 will conflict here and its reserve_indent() has nothing left to reserve. Worth deciding which lands first, especially with an advisory pointing at it.
(Written by Claude Code.)
| are not copied an extra time. | ||
| */ | ||
| JSON_HEDLEY_NON_NULL(2) | ||
| void put_chars(const char* s, std::size_t length) |
There was a problem hiding this comment.
There are multiple separate things being handled by this function.
- Write the characters of a string literal.
- Write a number of indent characters.
- Write the first
lencharacters from astd::array<char, N>. - Write an arbitrary number of characters from an arbitrary position in a
string_t.
I'm not sure that each of these should be the same function.
Number 1 is asking for the number to get out of sync with the length of the string literal. It could be something like this:
template<int N> void put_literal(const char (&str)[N])
{
const int length = N - 1;
if (JSON_HEDLEY_UNLIKELY(write_buffer_pos + length >= write_buffer.size()))
{
flush();
}
std::memcpy(write_buffer.data() + write_buffer_pos, str, length);
write_buffer_pos += length;
}
maybe with some enable_if to keep N under 1024 so you don't need to worry about it overflowing.
Number 2 could be:
void put_indent(unsigned int indent)
{
if (JSON_HEDLEY_UNLIKELY(write_buffer_pos + indent > write_buffer.size()))
{
flush();
}
if (JSON_HEDLEY_UNLIKELY(indent > write_buffer.size()))
{
// the buffer is clear, fill it with the indent character
std::memset(write_buffer.data(), indent_char, write_buffer.size());
// keep writing it out until all the indent is processed.
while (JSON_HEDLEY_UNLIKELY(indent > write_buffer.size()))
{
flush();
indent -= write_buffer.size();
}
// set the write position to cover the rest of the indent
write_buffer_pos = indent;
return;
}
std::memset(write_buffer.data() + write_buffer_pos, indent_char, indent);
write_buffer_pos += indent;
}
which also eliminates the need for indent_string and resizing it.
Number 3 and 4 could be safer than just a char * and a count, not sure if they could easily share a "safer" implementation though.
There was a problem hiding this comment.
Agreed — split into separate entry points in a1e9147, close to what you sketched.
1. String literals → put_literal(), taking the literal by reference and deducing the length from the array bound, as you had it. All 27 call sites converted. For what it's worth I checked every hand-written count before replacing it and none of them was actually wrong today, but the class of bug is gone.
One deviation: instead of an enable_if to bound N, the length is checked against the buffer with a static_assert, which lets the oversized-run branch disappear entirely rather than becoming unreachable — this repo wants 100% line coverage, so an uncoverable branch is its own small problem.
template<std::size_t N>
void put_literal(const char (&s)[N])
{
static_assert(N >= 2, "put_literal expects a non-empty string literal");
static_assert(N - 1 < write_buffer_size, "string literal must fit into the write buffer");
...
}2. Indentation → put_indent(), and this removes indent_string and the resizing exactly as you predicted. Details in the other thread, since it also settles #5186.
Your sketch needs one adjustment though: in the wider-than-the-buffer loop, write_buffer_pos is already 0 from the first flush() and never set back to write_buffer.size(), so each flush() inside the while writes zero characters — the buffer gets memset but nothing comes out, and the indentation ends up truncated. I used a fill-and-flush loop instead, which also collapses the two cases into one:
void put_indent(unsigned int indent)
{
while (indent > 0)
{
if (JSON_HEDLEY_UNLIKELY(write_buffer_pos == write_buffer.size()))
{
flush();
}
const std::size_t chunk = (std::min)(static_cast<std::size_t>(indent),
write_buffer.size() - write_buffer_pos);
std::memset(write_buffer.data() + write_buffer_pos, indent_char, chunk);
write_buffer_pos += chunk;
indent -= static_cast<unsigned int>(chunk);
}
}3. std::array + length → put_buffer(const std::array<char, N>&, std::size_t), so the length is checked against the buffer's own bound instead of travelling next to a bare pointer. Covers the string_buffer and number_buffer writes.
4. Arbitrary range of a string_t → left on put_chars(). Together with to_chars()'s output those are the only two callers now, and both are genuinely a pointer into the middle of something plus a length; I didn't find a safer shape that didn't just move the arithmetic. Open to ideas.
(Written by Claude Code.)
There was a problem hiding this comment.
write_buffer_pos is already 0 from the first flush() and never set back to write_buffer.size(), so each flush() inside the while writes zero characters
Yep, needed a write_buffer_pos = write_buffer.size(); before the flush().
I used a fill-and-flush loop instead, which also collapses the two cases into one:
I was trying to avoid the memset() on every iteration. If you have an indent of 4x the buffer size, you'll fill the whole buffer 4 times with the indent character, which is essentially a no-op on 3 of them because you know the state of the buffer.
There was a problem hiding this comment.
Both fair — fixed in b4fdf76.
You're right that re-filling the buffer is wasted work: flushing doesn't disturb what the buffer holds, so it only needs writing once. It now fills the room that's left, and if anything remains, flushes, fills the buffer once, and re-flushes that same content:
void put_indent(unsigned int indent)
{
// closing braces at the outermost level ask for no indentation at all
if (indent == 0)
{
return;
}
const std::size_t capacity = write_buffer.size();
const std::size_t head = (std::min)(static_cast<std::size_t>(indent), capacity - write_buffer_pos);
std::memset(write_buffer.data() + write_buffer_pos, indent_char, head);
write_buffer_pos += head;
indent -= static_cast<unsigned int>(head);
if (JSON_HEDLEY_LIKELY(indent == 0))
{
return;
}
flush();
std::memset(write_buffer.data(), indent_char, capacity);
while (indent >= capacity)
{
write_buffer_pos = capacity;
flush();
indent -= static_cast<unsigned int>(capacity);
}
// the buffer still holds indentation characters throughout, so the tail
// only has to be claimed, not written again
write_buffer_pos = indent;
}Counting memset calls and bytes inside put_indent over a dump():
| indent | before | after |
|---|---|---|
| 4 | 1 call / 4 B | 1 call / 4 B |
| 2000 | 2 calls / 2000 B | 2 calls / 2046 B |
| 100000 | 98 calls / 100000 B | 2 calls / 2046 B |
Constant instead of proportional to the width, and ordinary indents are untouched. The indent == 0 early return is worth having on its own — every outermost closing brace asks for a zero-width indent, and without it the common path paid for a zero-length memset.
Tests extended to cover several whole buffer-fulls and an exact multiple of the buffer size.
(Written by Claude Code.)
There was a problem hiding this comment.
- Arbitrary range of a string_t → left on put_chars(). Together with to_chars()'s output those are the only two callers now, and both are genuinely a pointer into the middle of something plus a length; I didn't find a safer shape that didn't just move the arithmetic. Open to ideas.
This at least puts all the arithmetic inside the function.
template<typename StringOrArray>
void put_string(StringOrArray const &str, std::size_t start, std::size_t end)
{
JSON_ASSERT(start < str.size());
JSON_ASSERT(end< str.size());
JSON_ASSERT(start < end);
const std::size_t length = end - start;
const char *s = str.data();
...
}
There was a problem hiding this comment.
That does, and it is the better shape — added as put_string() in 5567a0e.
Two adjustments to the assertions. end is exclusive, so end <= str.size() rather than <: a run reaching the end of the string is the normal case in dump_escaped, and < would fire on it. And start < end rules out the empty range, which is harmless to write, so I made that <= too and dropped the separate start check as implied:
template<typename StringType>
void put_string(const StringType& str, std::size_t start, std::size_t end)
{
JSON_ASSERT(start <= end);
JSON_ASSERT(end <= str.size());
put_chars(str.data() + start, end - start);
}With this and the dump_float fix from the other thread, put_chars() is down to two callers, both of them put_string()/put_buffer() — so nothing outside the buffer helpers passes a bare pointer and a count any more, which was your original point.
(Written by Claude Code.)
c4a3b4d to
509494e
Compare
bb8c130 to
a1e9147
Compare
| write_buffer[write_buffer_pos++] = c; | ||
| } | ||
|
|
||
| /*! |
There was a problem hiding this comment.
This comment block got detached from put_chars.
There was a problem hiding this comment.
Good catch — entirely my doing. I inserted put_indent/put_literal/put_buffer in front of put_chars and left its comment sitting on top of the first new function, so put_indent ended up with two doc blocks and put_chars with none. Reattached in 5567a0e.
(Written by Claude Code.)
| auto* end = ::nlohmann::detail::to_chars(begin, begin + number_buffer.size(), x); | ||
|
|
||
| o->write_characters(begin, static_cast<size_t>(end - begin)); | ||
| put_chars(begin, static_cast<size_t>(end - begin)); |
There was a problem hiding this comment.
Yes — missed that one. begin is number_buffer.data(), so it is the same fixed-size-buffer case as the others; it is put_buffer(number_buffer, ...) as of 5567a0e.
That was the last one: put_chars() now has no callers outside put_string()/put_buffer().
(Written by Claude Code.)
| are not copied an extra time. | ||
| */ | ||
| JSON_HEDLEY_NON_NULL(2) | ||
| void put_chars(const char* s, std::size_t length) |
There was a problem hiding this comment.
- Arbitrary range of a string_t → left on put_chars(). Together with to_chars()'s output those are the only two callers now, and both are genuinely a pointer into the middle of something plus a length; I didn't find a safer shape that didn't just move the arithmetic. Open to ideas.
This at least puts all the arithmetic inside the function.
template<typename StringOrArray>
void put_string(StringOrArray const &str, std::size_t start, std::size_t end)
{
JSON_ASSERT(start < str.size());
JSON_ASSERT(end< str.size());
JSON_ASSERT(start < end);
const std::size_t length = end - start;
const char *s = str.data();
...
}
| */ | ||
| template<std::size_t N> | ||
| void put_literal(const char (&s)[N]) | ||
| { |
There was a problem hiding this comment.
I still like only computing the length once so you don't forget the - 1:
const int length = N - 1;
There was a problem hiding this comment.
Fair, and it reads better. Done in 5567a0e:
static_assert(N >= 2, "put_literal expects a non-empty string literal");
// the array bound counts the terminating NUL, which is not written
constexpr std::size_t length = N - 1;
static_assert(length < write_buffer_size, "string literal must fit into the write buffer");constexpr rather than const so the static_assert can still use it, and std::size_t to match what it is compared against.
(Written by Claude Code.)
| put_indent(new_indent); | ||
|
|
||
| o->write_characters("\"subtype\": ", 11); | ||
| put_literal("\"subtype\": "); |
There was a problem hiding this comment.
Not a change here, but the "subtype" about 30 lines later doesn't have a space after the colon.
There was a problem hiding this comment.
Checked this one rather than trusting the read — I think the two you are comparing are in different branches. Line 294 is the pretty-print path ("subtype": with the space); the one about 30 lines later, at 322, is the compact path (],"subtype":), where nothing has spaces.
Actual output, and it is byte-identical to develop:
--- compact ---
{"b":{"bytes":[1,2,3],"subtype":42},"n":{"bytes":[1,2,3],"subtype":null}}
--- pretty ---
{
"b": {
"bytes": [1, 2, 3],
"subtype": 42
},
...
}
So both modes are internally consistent and neither changed here. No fix — but say the word if you were pointing at something else.
(Written by Claude Code.)
There was a problem hiding this comment.
I couldn't tell from the restricted diff that one was pretty and one wasn't (one having newlines and one not should have given me a clue but didn't), so this difference makes sense.
| A very large @a indent_step can wrap the unsigned accumulation on deep | ||
| nesting, which would silently truncate the indentation. | ||
| */ | ||
| static unsigned int next_indent(const unsigned int current_indent, const unsigned int indent_step) |
There was a problem hiding this comment.
Should the indent be size_t then?
There was a problem hiding this comment.
Yes — changed in 5567a0e. It is a size, it is compared and combined with buffer sizes throughout, and put_indent() was casting to std::size_t and back on every call. dump(), dump_internal(), next_indent() and put_indent() all carry std::size_t now, and the casts are gone.
I kept next_indent()'s assertion: on a 64-bit size_t you would need an implausible nesting depth to wrap it, but it is still reachable where size_t is 32 bits, which is the same place the original unsigned int overflow was reachable.
(Written by Claude Code.)
| rather than at the call site, which is all @ref put_chars could offer. | ||
| */ | ||
| template<typename StringType> | ||
| void put_string(const StringType& str, std::size_t start, std::size_t end) |
There was a problem hiding this comment.
Looks like I didn't write out part of what I was thinking. I was thinking that this would replace put_chars. That way there would not be a function that took a const char * and a length. Then put_buffer is just put_string(buffer, 0, length); That's why I had
const std::size_t length = end - start;
const char *s = str.data();
...
as the rest of the function would be the current body of put_chars().
5567a0e to
b03ee56
Compare
| { | ||
| if (JSON_HEDLEY_UNLIKELY(depth >= dump_depth_limit())) | ||
| { | ||
| dump_iteratively(val, pretty_print, ensure_ascii, indent_step, current_indent); |
There was a problem hiding this comment.
Rather than bailing out here to what is basically a copy of this function that can get out of sync, can you have the stack here and either recurse or push onto the stack based on the current depth? I haven't analyzed in depth enough to know if that would be possible.
|
|
||
| *out++ = static_cast<char>('0' + (byte % 10)); | ||
|
|
||
| write_buffer_pos = static_cast<std::size_t>(out - write_buffer.data()); |
There was a problem hiding this comment.
Would it be better to do write_buffer_pos++ along with each out++ and avoid the pointer math here?
|
|
||
| @complexity Linear in the length of string @a s. | ||
| */ | ||
| /*! |
There was a problem hiding this comment.
Inserted the new function between the comment block and the old function again.
a108a8a to
7a5bd71
Compare
| } | ||
|
|
||
| const auto byte = static_cast<unsigned>(value); | ||
| std::size_t pos = write_buffer_pos; |
There was a problem hiding this comment.
Is this variable even needed? Just use write_buffer_pos directly?
7a5bd71 to
07c8393
Compare
When ensure_ascii is false, dump_escaped previously ran every byte of every string and object key through the UTF-8 DFA decoder, even for the common case of ordinary text with nothing to escape. This mirrors the per-byte cost the parser had before the contiguous fast paths. At a character boundary, bulk-copy the longest run of bytes that need no escaping using string_bulk_run() - the same SWAR scanner and UTF-8 bulk validator the lexer's contiguous path uses - and only fall back to the byte-at-a-time DFA loop for the first byte that needs individual handling (a quote, backslash, control character, or ill-formed/truncated UTF-8). Because every "hard" or invalid byte is still processed by the unchanged byte path, escaping output and error handling (including strict-mode error 316 position and message) are byte-identical to before. The ensure_ascii=true path is unchanged: it must escape non-ASCII and 0x7F, which string_bulk_run does not stop on, so a separate predicate would be needed for it. Verified byte-for-byte identical dump output against the pre-change implementation across ~20k randomized byte strings plus curated edge cases (all escapes, control chars, valid multibyte, surrogates, overlong, truncated sequences) for both ensure_ascii settings and all three error handlers, in C++11/17/20 at -O2/-O3. Throughput (g++ -O3, ensure_ascii=false, vs pre-change): long ASCII strings 4.2x twitter-like objects 2.3x dense CJK 1.4x (further headroom with JSON_USE_SIMDUTF) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Two further serialization speedups on top of the ensure_ascii=false bulk
copy, both reusing the SWAR primitives in detail/input/string_scan.hpp.
1. Internal write buffer (devirtualization). Every structural character
('{', '"', ',', ...) previously went straight to the output adapter
through a virtual call. Route all writes through put_char/put_chars
into a 1 KiB buffer that flushes in bulk; the public dump() flushes
once the top-level value is done (the recursive worker is split out as
dump_internal). Runs larger than the buffer are written straight
through, so large payloads are not copied twice. This is the dominant
cost for object/array-heavy values.
2. ensure_ascii fast path. dump_escaped previously ran the UTF-8 DFA over
every byte when escaping non-ASCII. Add find_ascii_copyable_run() (a
SWAR scan stopping at '"', '\\', < 0x20, 0x7F, and >= 0x80) so runs of
printable ASCII are bulk-copied, with the byte path handling each
escape/non-ASCII byte exactly as before.
Behavior is unchanged: dump output is byte-for-byte identical to the
previous implementation across ~20k randomized byte strings plus curated
edge cases (all escapes, control chars, 0x7F, valid multibyte,
surrogates, overlong, truncated), for object/array/pretty output, both
ensure_ascii settings, and all three error handlers, in C++11/17/20 at
-O2/-O3. New unit tests cover the buffer flush boundaries, the escape and
0x7F handling, multibyte under both settings, and invalid-UTF-8 handling.
Throughput (g++ -O3, vs the ensure_ascii=false-only baseline):
long ASCII, ensure_ascii=0 4.2x
long ASCII, ensure_ascii=1 4.1x
twitter-like objects 2.7x
dense CJK 1.8x
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
test-convenience failed (macOS finished first; the failure is platform-independent) because check_escaped() calls the internal serializer::dump_escaped() directly and then reads the output stream. Since dump_escaped() now writes into the serializer's internal write buffer, the bytes were still buffered and the stream was empty. Expose flush() under JSON_PRIVATE_UNLESS_TESTED (same visibility as dump_escaped) and flush in check_escaped() before inspecting the output. Per-string flushing inside dump_escaped() was rejected on purpose: it would defeat the buffering that makes object/array-heavy dumps faster. Library behavior is unchanged (flush()'s body is identical; only its access label moved). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me>
The "many small structural writes exceed the write buffer" subcase built a 1100-deep nested array and dumped it to force >1024 consecutive single-character writes through put_char (exercising the write buffer's flush-when-full branch). dump() recurses per nesting level, so on MSVC debug builds (smaller default stack, larger frames) this overflowed the stack and crashed test-serialization; Linux/macOS have enough headroom to hide it. Replace the nesting with a flat array of 500 empty strings. Each element emits '"', '"', ',' via put_char, so the dump is a long run of single-character writes (1501 bytes > the 1024-byte buffer) at nesting depth two, hitting the same flush branch without deep recursion. Library code is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Follow-up to @gregmarr's review: put_chars() was doing four unrelated jobs, so give the two that can be made safe their own entry points. - put_literal(): takes the literal by reference and deduces the length from the array bound, so the 27 hand-counted lengths at the call sites can no longer drift from the literals they describe. A literal is checked at compile time to fit the buffer, so this path needs no write-through branch. - put_buffer(): takes the fixed-size buffer itself rather than a bare pointer, so the length can be checked against the buffer's own bound. - put_indent(): memsets the indentation into the write buffer, filling and flushing it as needed. This removes indent_string entirely, and with it both bugs of #5186: the indentation string was grown by doubling, which is not enough when indent_step more than doubles it (a heap over-read - dump(2000) read 2000 bytes out of a 1024-byte string), and the grown part was filled with a space instead of the configured indent_char. next_indent() keeps that PR's assertion against the unsigned indentation accumulation wrapping on deep nesting. put_chars() keeps the two cases that are genuinely a pointer and a count: the run-length copies out of the string being escaped, and to_chars() output. Tests cover an indent_step wider than the write buffer, a non-space indentation character past the old growth point, and nesting whose accumulated indentation spans several buffer-fulls. All three fail against develop. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@gregmarr's point on the fill-and-flush loop: flushing does not disturb what the write buffer holds, so an indentation spanning several buffer-fulls only has to be written into the buffer once and can then be handed to the adapter as many times as needed. The loop re-filled it every time, doing work it already knew was there. put_indent() now fills the room left in the buffer, and if anything remains, flushes, fills the buffer once, and re-flushes that same content. It also returns early for a zero-width indentation, which is what the closing brace of every outermost value asks for. Measured over a dump(), counting memset calls and bytes inside put_indent: indent before after 4 1 call / 4 B 1 call / 4 B 2000 2 calls / 2000 B 2 calls / 2046 B 100000 98 calls / 100000 B 2 calls / 2046 B The wide case is now constant work rather than proportional to the indentation width; ordinary widths are unchanged. Tests extended to cover several whole buffer-fulls and an exact multiple of the buffer size. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
More of @gregmarr's review on the put_* split: - Reattach the put_chars() doc comment, which the new helpers had been inserted in front of, leaving it describing put_indent(). - Compute the literal length once in put_literal() instead of spelling N - 1 at each use. - Add put_string(str, start, end), which keeps the pointer arithmetic and the bounds assertions inside the function instead of at the call site. With dump_float()'s to_chars() output moved onto put_buffer() as well, put_chars() now has no callers outside put_string()/put_buffer(): nothing passes a bare pointer and a count any more. - Carry the indentation as std::size_t rather than unsigned int. It is a size, it is compared and combined with buffer sizes throughout, and the casts in put_indent() disappear. next_indent() keeps its assertion, which is far harder to trip on a 64-bit size_t but still reachable where that is 32 bits. No output change: pretty and compact dumps, binary values included, are byte-identical to develop. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
clang-tidy flags the reference-to-array parameter under cppcoreguidelines/hicpp/modernize-avoid-c-arrays, and the CI treats warnings as errors. Binding to the array is the whole point here - it is what lets the length be deduced from the literal instead of hand-written at the call site - so suppress it the same way from_json(), to_json() and get_to() already suppress it for their own T (&arr)[N] parameters. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Serializing a container serializes its elements, so dump() descended into one call 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 descent goes rather than take the call stack away from it. The first 128 levels are written by exactly the code that always wrote them, and only below that does dump_iteratively write out what is left, keeping the containers it has entered on an explicit stack. Serializing can therefore no longer exhaust the stack, however deeply a value is nested, while a value nested less deeply than the bound pays only for one comparison per container. Writing every value that way instead measured between 2% and 20% slower - 20% on object-heavy documents - which is why the descent is kept for all but the values that cannot afford it. The bound costs nothing measurable: between -1.4% and +1.2% across compact and pretty output of number, integer, string, object-heavy, wide-object and deeply nested documents. The output is unchanged for every value. Both ways of writing a container emit the separator in front of every element but the first, rather than after every element but the last, which puts exactly one between each pair and none at the end. This fixes #5387 for dump(). The copy constructor is fixed in #5389. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Two hot spots that the write buffer and the bulk scanner left behind. dump_escaped took ensure_ascii as a runtime flag and tested it inside the loop, once per character run, although it cannot change while a string is written. It is now a template parameter, dispatched once per string, which folds the choice of scanner and lets each of the two be inlined into a loop of its own. This is the hottest loop in the serializer: it runs over every string and every object key. A binary value's bytes went through dump_integer, which counts digits and does 64-bit arithmetic for a number that is always in [0, 255]. dump_byte writes the three digits it takes at most straight into the write buffer instead. Any byte type that is not a plain unsigned byte is still left to dump_integer, whose representation of it may differ. Measured against the previous commit (medians of 9 interleaved runs, clang -O3): binary values -33.8%, dense CJK with ensure_ascii -20.6%, key-heavy objects -17.8%, deeply nested pretty output -17.9%, dense CJK without ensure_ascii -11.8%, object-heavy documents -9.3% compact and -9.5% pretty, a small value dumped in a loop -21.4%, wide objects -2.3%. Arrays of plain ASCII strings measured 3.5% to 4.2% slower, the one shape that loses; number and integer arrays are unchanged. Also tried and dropped: leaving the write and string buffers uninitialized rather than zeroing 1.5 KB per dump() call. It is worth -30% on small values, but two nearly identical string workloads moved 18% apart in opposite directions, so the measurements did not support it. The output is unchanged for every value: the differential now also covers every one of the 256 byte values, alone and together, in both binary layouts. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
clang-tidy's misc-const-correctness reads the pointer dump_byte advanced over the write buffer as one whose pointee could be const. Index the buffer instead, which says the same thing without a raw pointer at all. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
clang-tidy's readability-math-missing-parentheses wants the multiplication spelled out in reserve(6 * depth + 1), and CI treats its warnings as errors. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
07c8393 to
a2e9855
Compare
What & why
dump()had two hot spots that mirror exactly what #5283 fixed on the parse side, so this PR reuses that PR's SWAR/UTF-8 primitives (detail/input/string_scan.hpp) to fix the serializer. Output is byte-for-byte unchanged on every input; only the path taken to produce it is faster.The changes (each independent and reversible)
SWAR bulk string escaping (
ensure_ascii == false) —dump_escaped()ran the UTF-8 DFA over every byte of every string and key, even for ordinary text with nothing to escape (the serializer twin of the parser's per-characterget()). At a character boundary it now bulk-copies the longest run needing no escaping viastring_bulk_run()— the same scanner + UTF-8 bulk validator the lexer's contiguous path uses — and drops to the byte-at-a-time DFA only for the first byte that needs individual handling (a quote, backslash, control char, or ill-formed/truncated UTF-8). Forensure_ascii == false,string_bulk_run()'s stop set is an exact match for the bytesdump_escapedcopies verbatim, so escaping and error handling (including strict-mode error 316 position and message) are identical. Picks upJSON_USE_SIMDUTFfor free.Internal write buffer (devirtualization) — every structural character (
{,",,, …) previously went straight to the output adapter through a virtual call. All writes now route throughput_char/put_charsinto a 1 KiB buffer flushed in bulk; the publicdump()flushes once the top-level value is serialized (the recursive worker is split out asdump_internal). Runs larger than the buffer are written straight through, so large payloads are not copied twice. This is the dominant cost for object/array-heavy values.ensure_ascii == truefast path — addedfind_ascii_copyable_run()(a SWAR scan stopping at",\,< 0x20,0x7F, and>= 0x80) so runs of printable ASCII are bulk-copied even when non-ASCII must be\u-escaped.0x7F/DEL is a deliberate stop (it is escaped underensure_ascii), which is whyfind_string_special()could not be reused directly here.Bounded descent, so
dump()cannot overflow the stack (Stack overflow in copy constructor and dump() on deeply nested json (destructor was fixed in #1436) #5387) — serializing a container serialized its elements, sodump()descended into one call per nesting level and a value nested deeply enough terminated the process withSIGSEGV, no exception to catch. Parsing such a value works (the parser is iterative) and so does destroying one (Prevent stackoverflow caused by recursive deconstruction #1436); serializing never got the same treatment. The descent is now bounded: the first 128 levels are written by exactly the code that always wrote them, and only below that does a newdump_iterativelywrite out what is left, keeping the containers it has entered on an explicit stack.Writing every value that way instead — the obvious fix — measured 2% to 20% slower, 20% on object-heavy documents, which would have undone much of what this PR is for. Keeping the descent for everything that can afford it costs one comparison per container: between -1.4% and +1.2% across compact and pretty output of number, integer, string, object-heavy, wide-object and deeply nested documents.
Both writers emit the separator in front of every element but the first, rather than after every element but the last; that puts exactly one between each pair and none at the end.
ensure_asciifolded into the escaper —dump_escaped()took it as a runtime flag and tested it inside the loop, once per character run, although it cannot change while a string is written. It is now a template parameter dispatched once per string, which folds the choice of scanner and lets each of the two be inlined into a loop of its own. This is the hottest loop in the serializer — it runs over every string and every object key.Binary bytes no longer go through
dump_integer— a byte is always in[0, 255], so it needs neither digit counting nor 64-bit arithmetic;dump_bytewrites the at most three digits straight into the write buffer. Any byte type that is not a plain unsigned byte is still left todump_integer, whose representation of it may differ.Gains from 5 and 6 (medians of 9 interleaved runs, clang -O3, against the commit before them):
ensure_ascii=trueensure_ascii=falseThe last row is the one shape that loses, consistently across two independent benchmarks. It is a small cost against the rest, but it is a real one.
Tried and dropped: leaving the write and string buffers uninitialized instead of zeroing 1.5 KB per
dump()call. Worth -30% on small values, but two nearly identical string workloads moved 18% apart in opposite directions, so the measurement did not support the change — and it would have traded away safety margin that cannot be checked here (MemorySanitizer is unavailable on arm64 macOS).Measured gains
json::dump(), C++17, g++ 13-O3, vs the pre-change serializer (representative synthetic data):ensure_ascii=falseensure_ascii=trueJSON_USE_SIMDUTF)Verification
ensure_asciisettings and all three error handlers; plus every nesting depth from 1 to 300 in array, object and mixed shapes at three indent settings, so the depth at which the two writers hand over is covered from every side. New tests cover a 100,000-deep value (which crashes on the base), the depths around the bound, pretty-printing across it, and empty containers reached below it.0x7F, valid multibyte, surrogates, overlong, truncated sequences), for compact/pretty and object/array output, bothensure_asciisettings, and all three error handlers, in C++11/17/20 at-O2/-O3.0x7Fhandling, multibyte under both settings, and invalid-UTF-8 handling.-Weverythingand the gcc pedantic flag set; clang-tidy clean on the changed headers;make check-amalgamationclean.How other libraries handle this
Worth knowing where this sits, because the trade-off taken here is deliberately not the common one.
Most libraries reject deeply nested input rather than support it — a hard depth limit at parse time, which then protects everything downstream:
parse_options::max_depth= 32RecursionLimitExceededmaxNestingDepth1000 (500 in 3.0)System.Text.JsonJackson's limit exists because of CVE-2025-52999, and CVE-2026-29062 covers a parser that bypassed it; PostgreSQL patched its OAuth JSON parser the same way. Only a few libraries are iterative throughout instead: yyjson advertises "unlimited JSON nesting levels" as a headline feature, and miniserde gives its value type a non-recursive
Drop. A third approach is to grow the stack on demand, as serde_stacker does.This library sits in the worst spot of the three. Its parser is iterative, so it accepts input nested arbitrarily deeply — and then hands it to operations that cannot cope. lovasoa/bad_json_parsers, which measures exactly this, buckets it under "unlimited → segfault". A library with a parse limit never builds such a value in the first place.
The closest C++ peer has the same split and has fixed none of it. RapidJSON's
kParseIterativeFlagmakes only parsing iterative, and is opt-in because the recursive parser is faster; its destructor is still recursive (#2217, open, asking for exactly what #1436 did here in 2019),CopyFromstill overflows on deep values, andValue::Accept()— its serializer — still recurses.Two things that supports about the approach taken here: RapidJSON keeps recursion as the default because it is faster, which matches the measurement that writing every value iteratively costs up to 20%; and a bound of 128 sits in the same range as everyone else's hard limits — except that nothing is rejected at it, so this library stays strictly more permissive than all of them while no longer crashing.
make amalgamate.🤖 Generated with Claude Code
https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG
Generated by Claude Code