Correctness fixes, ~35% faster parsing, and a portable path - #1
Open
asp24 wants to merge 43 commits into
Open
Conversation
`go test ./...` failed: TestNdjsonCountWhere2 died with a nil pointer dereference inside loadFile. Two of the corpora fetched on demand now answer 404, and the error path called err.Error() on the nil error http.Get returns when the request succeeded and only the status was bad. loadFile now takes a testing.TB and skips the caller when a corpus cannot be obtained - there is nothing wrong with the code under test - while a corrupt archive or an unwritable cache stays fatal. It also closes the response body, which it never did. .gitignore was empty; it now covers the downloaded corpora, editor directories and test output. Test files and .gitignore only.
The dependencies set the floor: compress v1.19.1 and cpuid v2.4.0 ask for 1.24, x/sys v0.47.0 for 1.25.0. Both modules now declare go 1.25.0 and the CI matrix follows, since an older toolchain cannot build the module without fetching a newer one. A consumer whose own go directive is below 1.25 stops building against this package, so the floor is set by what the dependencies need and no higher. 1.21 onwards also makes the hand-written min and max in appendfloat_f.go redundant. The builtins are generic over ordered types and for two ints behave exactly as the pair they replace, so no call site changes. That file comes from the standard library, which is why LICENSE-go arrives with it. Its header says the licence "can be found in the LICENSE file", and the LICENSE here is Apache-2.0 - so the BSD-3 terms it actually stands under, and the disclaimer that licence requires to be reproduced, were nowhere in the tree. The header now points at LICENSE-go and records that this fork changed the file; the three other files taken from the standard library are pointed at it as they come up.
The only in-repo parse benchmarks were the three payload documents, so any statement about the corpus needed a throwaway harness. BenchmarkParseCorpus walks testCases - the same fourteen documents README.md quotes - in both copy modes, which is what a change to the parser has to be judged against: the float-heavy and the string-heavy documents move in opposite directions, and a single file hides that. Test code only.
Nothing measured the consuming side. The one benchmark that looked like it did - Iter.MarshalJSONBuffer - is re-serialization: its time goes into escapeBytes and the float formatter, neither of which a reader touches. BenchmarkIterRead walks the corpus three ways, in rising order of what a caller asks of the tape: descending it and converting nothing, pulling every leaf out as its Go type, and going through Object so key names are read too. TestReadWalksEverything counts the leaves straight off the tape and checks each walk against it, so a walk that quietly stops early cannot pass for a fast one. Test code only.
Five .s files were headed "AUTO-GENERATED BY C2GOASM -- DO NOT EDIT". The C they came from is not in this repository and every one has been hand-edited since - the AVX-512 variants were written directly - so a reader who believed the header would go looking for a generator that does not exist. Each file now records where it came from and what has happened to it since, and the four hand-written ones say so too. The constraints mattered more. `//+build !noasm !appengine gc` is an OR, so it held on essentially every build and the assembly was compiled even under -tags=noasm; the tag took effect only because the Go declarations alongside used the AND form. Both now say `//go:build !noasm && !appengine && gc`. NOTICE and a line at the top of the README say the same thing about the repository as a whole: what it is a fork of, and how to read the notice on a file. MinIO's notice alone means the file is as they published it; MinIO's with a modification line means this fork changed it; a DATA.BET notice alone means it was written here. The assembly carries no copyright notice of its own, so the files this fork changes record the modification in the comment above the code - which Apache-2.0 section 4(b) asks for anyway, of anyone who distributes a changed copy.
It is a separate module, so `go test ./...` from the repository root never reaches it and it can break unnoticed - its go.sum drifting from the root module's, for instance.
The repository has been unmaintained for years, so the row measured a library nobody should be choosing now. benchmarkJsoniter and its fourteen named benchmarks go, and go mod tidy takes json-iterator/go, modern-go/concurrent and modern-go/reflect2 out of the module with them. The README's second benchcmp table compared against it and nothing here can reproduce those numbers any more, so it goes too. encoding/json stays as the row that says what the standard library costs.
sonic is the fast alternative people actually weigh simdjson against, so it gets rows in the historical group. Its native implementation exists only on the platforms and Go versions its own build constraints name, and elsewhere it forwards to encoding/json - so sonic_native_test.go carries that constraint verbatim and the rows skip rather than report encoding/json's numbers under sonic's name. The historical group is not a parser comparison: simdjson.Parse builds a tape and stops, while the others build a Go value out of every element. samework_test.go adds two groups where every library is asked for the same thing - BenchmarkMaterialise turns a document into Go values, BenchmarkExtract reads three fields out of one - and its header comment lists every asymmetry between the libraries and which were equalised. TestExtractAgree checks all five implementations return the same checksum, so the fastest row cannot be the one that quietly found nothing.
Three things about this suite are not visible from the code and each of them has produced a wrong answer here: the historical group compares calls that do different amounts of work, a short -benchtime makes the framework pick its iteration count off noise (sonic on github_events read 715 us/op at 200ms and 110 us/op at a fixed 2000x), and an unpinned BenchmarkExtract spread +-73% between rounds because the scheduler moved rows between cores that do not hold the same clock. benchstat is now a tool dependency, so `go tool benchstat` runs the version go.mod names and the commands in the README work as written.
parseStringSimdValidateOnly handed the assembly `&maxStringSize`, but maxStringSize is already a *uint64, so the callee's single dereference produced the address of the pointer rather than the size. The scan bound was an arbitrarily large number and the loop was in practice terminated only by finding a closing quote - past the end of the window the caller allowed, and on unterminated input past whatever the caller had reserved. TestParseStringValidateOnlyBeyondBuffer covered exactly this and was disabled with t.Skip(). It is reinstated, rewritten to put the closing quote beyond the permitted window inside a generously sized buffer, and fails without the one-character fix. Throughput is unchanged: geomean -0.15% over the six string-heavy documents in both copy modes, every file inside the noise band that a same-source control run puts under 1%.
parseString passed peekSize's answer straight to the scanner. peekSize returns 0 when the next structural index has not arrived yet, and the assembly reads a zero bound as "no room at all" and fails immediately; a bound larger than the bytes remaining would let the scan run off the end of the message instead. Both are now clamped to what is left in the buffer, in one unsigned comparison. Stage 1 holds a trailing quote back for the next index buffer, so neither case is reachable today for a string. The tape and re-serialized JSON are unchanged over the whole corpus in both copy modes.
parseStringSimd took a reflect.SliceHeader of the destination and then called append on a copy of the slice. Had that append ever reallocated, the assembly would have written into the new array while the length was bumped on the old one, publishing uninitialized bytes to the caller. The header manipulation also hid the assumption the whole function rests on: the caller has already reserved the room. It now writes in place through the caller-guaranteed spare capacity and derives the new length from the slice, so under-reservation panics instead of corrupting memory, and reflect is no longer imported. The tape and re-serialized JSON are unchanged over the corpus in both copy modes, with -race clean. This costs about 1% where it applies, and the pattern says it is causal rather than noise: copy mode alone moves (apache_builds +1.8%, update-center +1.5%, twitterescaped +1.2%, twitter +1.0%, geomean +0.55%) while nocopy, which never reaches this code, stays flat. The extra work is the reslice to capacity and its bounds check, and it buys a panic instead of memory corruption on under-reservation.
maxdepth was only ever a capacity hint, never a limit, so containingScopeOffset grew without bound: 2 MB of nothing but '[' allocated an 8 MB scope stack, and the input that does it is two lines long. Nesting is now rejected past maxdepth, which is raised from 128 to 1024 to match the reference simdjson's default - documents between the two depths still parse. The push goes through pushScope, which fills the stack initialize already reserved, so the depth check doubles as the bounds check and append's growslice and write barrier leave the hot path. Throughput is unchanged: geomean +0.06% over six documents of different shapes in both copy modes, everything inside the noise band. Filling the pre-reserved stack in place rather than appending is what keeps it there.
ParseND returned &pj.ParsedJson without setting internal, so passing the result back as `reuse` reused nothing: every call re-allocated internalParsedJson, including its ~96 KB of index buffers. Parse already did this correctly. TestParseNDReuse checks the returned document carries its parser and that a second call picks the same one up; it fails without the two added lines. BenchmarkParseNDReuse makes the cost measurable from the repository. BenchmarkParseNDReuse over parking-citations, reusing the returned document: before 573 us/op 2 425 388 B/op 15 allocs/op after 428 us/op 64 B/op 3 allocs/op That is -25.2%, and it is allocation rather than parsing: the same 2.4 MB of index buffers was being allocated and thrown away on every call.
AsString and AsStringCvt estimated their result length from the raw remaining tape length, but a string, like a float or an integer, occupies two tape entries. Both over-allocated by 2x. AsFloat, AsInteger and AsUint64 already divide.
Serialize tested `cap(s.tagsBuf) <= tagBufSize`, which is true again immediately after `make([]byte, tagBufSize)`, so the 64 KB tag buffer was allocated fresh on every call. The neighbouring values buffer already used `<`. BenchmarkSerialize with the default compression: twitter -1.7%, canada -1.2%, citm_catalog -0.4%, geomean -1.1%, and canada's allocation per call drops from 70 006 B/op to 4 492 B/op.
unsafeBytesToString assembled a reflect.StringHeader through an unsafe.Pointer. That header's Data field is a uintptr, which the collector does not recognise as a reference, so between writing the header and using the string nothing kept the bytes alive - it worked only because the caller's slice happened to be live. unsafe.String takes a real pointer and has no such window. This was the last reflect header outside the tests, so parse_number.go no longer imports reflect. The number tests and the strconv diff test are unchanged, as is the tape over the corpus.
`{"a":"\u00 !"}` parsed and produced a string holding a NUL, as did `\u00,-`, `\u+000`,
`\u0 00` and `\u00/.` - any of the four digit positions.
The digittoval table the assembly carries maps a byte to its value as a hex digit or to
-1, and its 0x00 to 0x2f entries were never written: the DATA block jumps from 0x038
straight to 0x070, so those forty-eight bytes were zero and read as the digit 0 instead
of as an error. The sixteen bytes from 0x20 to 0x2f are the space and most of the
punctuation, all legal inside a JSON string, so this was reachable from Parse and not
only from a unit test calling the routine directly.
The entries are spelled out now. A poisoned digit sign-extends into the high bits of the
code point, so the existing range check rejects it without a new branch. RFC 8259 wants
four hexadecimal digits, so nothing valid changes: the digest over the corpus is
unchanged. Each of the seven cases added to parse_string_test.go was confirmed to fail
against the old table.
The default copy-strings path ran two full AVX2 passes over every string: _parse_string_validate_only to measure it, then _parse_string to decode it into the string buffer. The first pass cannot go. _parse_string takes no bound - only src, dst and the output cursor - and writes a whole YMM word per iteration, so it is the validating scan that establishes the closing quote lies within maxStringSize, and with it both the destination capacity and the termination of the decode loop. The second pass is what is redundant, whenever there is nothing to decode. The scan already reports the source length alongside the decoded length, and since every escape sequence decodes to strictly fewer bytes than it occupies, the two being equal is an exact test for "no escapes"; then the decoded form is the source bytes verbatim and a plain append replaces a word-at-a-time validating copy. Strings that do carry escapes, and WithCopyStrings(false) where the tape points into the message, are untouched. The padded copy - needed because the scan re-checks its bound only between words and may read up to 31 bytes past maxStringSize - moves off the stack onto the parser. It was a 512 byte array zeroed on every call that took the branch, in a frame large enough to cost a stack check on every string; it is now a reused buffer of maxStringSize+64 with only its tail cleared. The tail must stay zeroed, or a stray quote there would be taken for the closing one. parseStringSimdValidateOnly is marked go:noinline: letting it inline into parseString leaves more live across the assembly call and measured as a regression on strings that need no copy, which have the least work after the scan to hide the spills behind. With WithCopyStrings, which is the default: github_events -10.1%, twitter -9.7%, gsoc-2018 -6.7%, twitterescaped -5.2%, update-center -4.8%, apache_builds -1.5%. Without it -0.2% to -3.4%, from the padded copy moving off the stack. Geomean -4.0% over the six string-heavy documents in both modes.
parseNumber classified a token byte by byte and then passed it to strconv, which walked the same digits again. On canada that was 47.7% of parse time in internal/strconv.readFloat on top of 18.4% in parseNumber's own loop; on the integer-only files, 11.1% in strconv.ParseUint. fastInt and fastFloat fold the digits themselves for the shapes where the result is exact by construction: up to 19 digits always fit a uint64, and up to 15 keep a float64 mantissa inside the 52 bit significand, so a single multiply or divide by an exactly representable power of ten rounds once and lands on the nearest float64. Everything else still goes to strconv unchanged - exponents, a second decimal point, stray signs. Exponents are under 0.3% of the numbers in every corpus file, so they are not worth special-casing. The digit count is derived after the classification loop rather than inside it: every byte of a token is a digit except a leading minus and a decimal point, so pos minus those two flags bounds it. Counting in the loop is about three instructions per byte and measured as a regression on its own. Over-estimating is safe, since fastInt and fastFloat re-check the shape. isDotFlag is new, to tell a decimal point from an exponent marker, which previously shared isFloatOnlyFlag. parse_number_diff_test.go pins this: it keeps the previous all-strconv implementation as an oracle and requires bit-identical tags, flags and values over hand-picked edge cases, generated tokens and every number in the corpus - 4.5M comparisons. numbers -44.5%, mesh -44.4%, marine_ik -37.0%, mesh.pretty -19.9%, citm_catalog -12.8%, geomean -28.1% over the six number-heavy documents in both copy modes. canada is +1.4%: 91.2% of its coordinates carry 17 significant digits, so they fall through to strconv and pay for the attempt. That is what the next-but-one commit fixes.
atof64exact followed Go's strconv in requiring the mantissa to fit 52 bits and in routing every positive exponent past a 1e15 guard. The real condition is the one the reference simdjson uses: -22 <= power <= 22 && i <= 9007199254740991. 2^53-1 is the largest integer a float64 holds exactly, and with both operands exact a single multiply or divide is correctly rounded. Exponents up to 22 now go straight through, and the 1e15 guard is confined to the case that splits a factor of 10^22 off a larger exponent, where an inexact intermediate is the actual hazard. The digit gate moves from 15 to 16. This is coverage rather than speed: the fast path takes 2.3% -> 8.8% of canada's floats, 99.8% -> 100% on mesh, 99.9% -> 100% on numbers, and is unchanged on marine_ik and mesh.pretty. The files that could gain were already near full coverage, and canada's remaining 91.2% needs 17 significant digits and so a different method entirely. Kept because it is the documented bound and it simplifies the common path. The strconv oracle in parse_number_diff_test.go still matches on every comparison. canada -5.4% copy and -4.8% nocopy, mesh -1.4%, mesh.pretty -1.0%, numbers -0.9%, geomean -1.4%. Small, and the reason is that the files which could gain were already near full fast-path coverage.
The string buffer grew by doubling from an initial guess of a tenth of the message, so a document that is mostly strings took five reallocations and most of the megabytes a cold parse churned through. The reference simdjson sidesteps this by reserving 5/3 of the document up front and never checking capacity again. Here the message length is a tighter ceiling and a provable one - strings are disjoint pieces of the message and decoding never lengthens one - but jumping straight to it over-reserves badly for a document that is mostly numbers: it cost citm_catalog 50% more allocated bytes. reserveStrings extrapolates instead. At the point of growth it knows both how much string content it holds and how far into the message that came from, so it projects the density over the remainder, adds a quarter as margin, and clamps to the ceiling. Cold parse, allocated bytes per document: gsoc-2018 14.5 MB -> 7.8 MB update-center 2.50 MB -> 1.41 MB twitter 1.88 MB -> 1.43 MB random 2.33 MB -> 2.03 MB citm_catalog, canada, marine_ik, mesh unchanged Raising the tape estimate was tried and reverted. The real figure runs from 2.3% of a tape entry per message byte on string-heavy files to 21.6% on arrays of numbers, and no single number suits both: covering the dense files bought nothing beyond noise and cost citm_catalog 12.7% on a cold parse. The comment in initialize records that, since the tape's growth policy is still what a cold parse on marine_ik and mesh pays for, and fixing it means touching append in the hottest write path. This is a trade, not a win, and the numbers are worth stating both ways. Cold parse, allocated bytes per document: gsoc-2018 14.5 MB -> 7.8 MB, update-center 2.50 MB -> 1.41 MB, twitter 1.88 MB -> 1.43 MB, random 2.33 MB -> 2.03 MB, with citm_catalog, canada, marine_ik and mesh unchanged. Warm parse, where the buffer is already large enough and only its footprint matters: copy mode is 1.2% to 3.3% slower (geomean +1.28%) and nocopy is flat. Dropping the 25% margin was measured and changes nothing (+0.05%), so the cost is the larger reservation itself, not the margin. Kept because a service that parses a document once with a fresh parser pays the allocation and not the footprint - but it is the one change in this series that a caller might reasonably want the other way round.
__flatten_bits_incremental consumed the structural bitmask by shifting it right past every bit it found: TZCNTQ to locate the bit, then SHRQ by that count. That put TZCNT's three cycle latency on the loop-carried path, since the next iteration's mask depended on this iteration's trailing-zero count. BLSR clears the lowest set bit from the mask alone, so the carried dependency is one cycle and TZCNT overlaps with the body. Increments become the difference between two bit positions, with the bits carried over from empty blocks folded in by seeding the previous position with -(carried+1) - one NOT, and the arithmetic falls out the same. The emitted format is untouched: still increments, still uint32, same carry and position semantics. TestFlattenBitsIncremental passes unchanged, and TestFlattenBitsDiff is new - it diffs the assembly against a Go reference over every single bit position, empty masks and several thousand random masks at three densities, restarting at intervals so a wide range of carried values is reached. BenchmarkFlattenBits was measuring nothing: find_structural_bits returns the mask rather than writing through a pointer, and the benchmark discarded the result, so it flattened ten thousand zero masks and only ever took the empty-block branch. It now collects real masks and asserts it found some. BenchmarkFlattenBits -16.1%, BenchmarkStage1 -10.3%. End to end, with GOMAXPROCS=1 so the two stages' times add rather than race: geomean -3.1% over six documents in both copy modes, every one of the fourteen rows negative - update-center -6.3%, twitter -4.9%, random -3.7%, citm_catalog -3.0%, github_events -2.9%, apache_builds -1.8%. On all cores the same pair scatters 25 points between files against 1.3 for a same-source control, and its geomean lands at +0.7%. That is the concurrent path, not this change: above the cutoff wall clock is max(stage 1, stage 2), so a stage 1 gain only shows where stage 1 is the longer half, and which half wins varies run to run. A stage 1 change has to be judged on the stage 1 benchmarks or on one core.
updateChar read pj.indexesChan.{index,length,indexes} back out of the heap-allocated
parser on every structural character: four loads and a store per element, plus a nil check
on the array pointer and a bounds check against the constant buffer size, and the
store-to-load forwarding on the position serialized one element against the next. It
measured at 6.5-8.7% of parse time.
The cursor now lives in a local of unifiedMachine, so the position and the remaining
indexes stay in registers. Holding them as a slice rather than a pointer to the whole
array also retires the bounds check, since the branch that refills from the channel
establishes that the position is inside it.
peekSize becomes indexCursor.peek and updateCharDebug becomes nextDebug, which wraps next
instead of duplicating it. pj.indexesChan is gone, along with its reset in initialize:
nothing outside stage 2 ever read it, and a local starts fresh by construction.
The documents that gain are the ones with the most structural characters per byte, which
is what a per-element cost should look like.
geomean -1.5% over six document shapes in both copy modes: twitterescaped -3.5%,
twitter -2.2% copy and -4.3% nocopy, marine_ik -2.1%, apache_builds -2.1%, citm_catalog
-2.0% copy. canada is +1.6% copy, which is a document with 21% structural characters and
almost no strings - the one shape where the extra branch in the refill is not paid back.
parseMessage already ran the stages one after the other for messages up to 8 KB, but stage 1 still handed every buffer of indexes to stage 2 down a channel it then had to receive from, plus a terminator. On payload-small that machinery was about a quarter of the parse: runtime.lock2 and unlock2 together 10.6%, chanrecv 10.4%, chansend 7.6%. On that path stage 1 now collects the buffers in pj.indexList and the cursor walks it. Nothing has to be drained on the way out either, so the two error paths lose their drain loops, and the channel is only created when it is used - a workload of small documents never allocates one. Without the channel there is no back pressure, so every buffer stage 1 produces stays live at once. That is what bounds the synchronous path, and the bound holds with room to spare: buffers take indexSizeWithSafetyBuffer indexes each and a message cannot have more indexes than bytes, so 8 KB needs at most six of the sixteen slots. TestSyncPathMultipleIndexBuffers builds the densest document that fits - `[1,1,...]`, two indexes per two bytes - and asserts both that more than one buffer is used and that the count stays inside indexSlots; it reports six. indexCursor.next grew a branch for the two sources. Moving the refill out of line made things worse: a call costs 57 against the inliner's budget of 80, so next stopped being inlined into unifiedMachine and the cursor would have gone back to memory. The refill stays in the body and next inlines at a cost of 78, which is close enough to the limit that anything added here needs the inliner output checked. The refill being a loop rather than a single step also fixes a latent bug: stage 1 can emit an empty buffer, having stripped a trailing quote back for the next round to re-emit, and the old code would have read an index out of it anyway and picked up whatever the slot held before. Three tests that drive findStructuralIndices on its own now say pj.async = true, which is what they were always testing. The three payload documents, which are what sits under the cutoff: payload-small -16.5% copy, -18.4% nocopy, -20.4% parallel; payload-medium -3.8% to -4.1%; payload-large -0.6% to -2.1%, since it is over the cutoff and still runs concurrently. Geomean -8.3%.
findStructuralIndices asked cpuid whether the CPU has AVX-512F on every call. The answer cannot change while the process runs, so it is now a package variable read at initialisation. This is not about the cost of the call - it was already once per document, not once per 64 byte block. It is so that the choice of pipeline is a single place a test can set. `cpuid.CPU.Disable(cpuid.AVX512F)` no longer switches paths, and a test that used it to compare the two would quietly compare one against itself.
Stage 1 carries an AVX2 and an AVX-512 implementation of every step and picks between them at runtime. The subroutine tests do cover both halves - break either and four of them fail - but each checks one step against hand-written input. What none of them reached is the two assembled pipelines running the same megabyte of JSON: the state carried from one 64 byte block to the next, a quote left dangling at the end of an index buffer, a document whose length is not a multiple of anything. TestStage1PathsAgree runs both over the corpus and compares every structural offset. It skips where AVX-512 is missing, so it is a check for a developer who has it; on this machine AVX-512 is the default path, which means the AVX2 pipeline had nothing running the whole corpus through it at all. Test code only.
canada was the one number-heavy document the short-number path did nothing for: 91.2% of
its coordinates carry 17 significant digits, which overflow the float64 significand, so
the exact path had to decline and every one of them went to strconv.
eiselLemire64 covers them. It implements the algorithm from Eisel and Lemire's "Number
Parsing at a Gigabyte per Second": normalise the mantissa, multiply by a 128 bit
approximation of the power of ten, read the answer off the top of the product, and decline
whenever the product sits too near a rounding boundary for that to be decidable, or would
come out subnormal or overflow. Those still go to strconv. Over the exponent range
parseNumber can reach it commits to an answer 99.87% of the time.
The powers of ten are detailedPowersOfTen, which has been in the tree since the Ryu float
formatter was ported: same -348..347 range, same {low, high} layout, and nothing had
verified it. TestDetailedPowersOfTen recomputes all 696 entries with math/big, which is
worth having now that the table has a second consumer.
TestEiselLemireAgainstStrconv drives the function directly, so the whole table is covered
rather than only the part parseNumber reaches - every exponent with mantissas chosen to sit
on rounding boundaries, plus 500,000 random pairs - and wherever it commits, the answer
matches strconv bit for bit.
Eisel-Lemire alone measured as nothing, and the profile said why: strconv's cost on canada
was never the algorithm, since it already uses Eisel-Lemire, but internal/strconv.readFloat
at 47.7%, folding the digits. Both spend 17 iterations of mant*10 + digit, each waiting
three cycles on the multiply before the next can start. fastFloatLong breaks that chain by
folding eight digits at a time - pairing them and combining the pairs, one multiply per
eight digits instead of eight - which is the SWAR trick upstream simdjson keeps behind
SIMDJSON_SWAR_NUMBER_PARSING. Measured on its own it is a loss, because without
Eisel-Lemire the fast path declines on canada anyway and the folded digits are thrown
away; the two only pay together.
canada -47.2%, mesh.pretty -35.9%, numbers -2.7%, geomean -17.0% over the number-heavy
documents in both copy modes. The rest are flat: their floats already took the exact path,
and this commit is about the ones that could not.
A string that has to be copied was walked twice: the scan to find the closing quote and measure it, then either an append of the body or the decode. The second walk existed only because the caller has to reserve room in the string buffer before the assembly writes into it, and it needed the length to know how much. The bound stage 2 already carries answers that. peek is the distance to the next structural character, which exists only once stage 1 has passed the closing quote, so the quote is certainly within it - and a decoded string is never longer than its source. Reserving that much lets the copy run first, and it copies a word and looks for the quote and the backslash in the same step, which is what the reference simdjson does in copy_and_find; not walking a string twice is the whole reason it needs no string lengths out of stage 1. The fused routine is now the only thing validating an escape sequence on the copying path, so parse_string_diff_amd64_test.go pins the two paths against each other over 2737 documents - escapes, surrogates, raw control characters, unterminated strings and every closing-quote offset within a 32 byte word - plus the corpus. Both must accept the same inputs and produce the same JSON. Checked with negative controls: dropping the failure check on the fused path is caught on `["\x"]`, and reporting one byte less is caught on the corpus. With WithCopyStrings, which is the default: twitterescaped -27.5%, gsoc-2018 -24.6%, update-center -15.4%, apache_builds -14.9%, github_events -8.6%, twitter -4.9%. Without it nothing moves (+2.4% to -2.1%, scatter of the concurrent path), which is expected: that path never walked a string twice. Geomean -8.2% across both modes.
With the second walk gone from the copying path, what was left around it was mostly Go. A fresh profile put parseString itself at 16% of the parse, ahead of the 10% in the SIMD routine it calls, with the wrapper at 5.5% and the reservation at 2%. Three things account for most of that. Every string wrote the tape twice, once for the tag and once for the length, so it paid two capacity checks and two stores of the slice header. The number path has always written its two words in one append; strings do now too, on all three branches. parseStringSimd took the string buffer as a *[]byte and wrote the new length back through it. Reading the assembly's output pointer out of the stack slot it had just written, and then storing a slice header through a pointer, was half the cost of the function. It returns the length now and the caller extends its own slice. reserveStrings misses the inlining budget by one unit, so the check it performs is written out at the call site, where it runs once per string and almost never finds anything to do. The rest moved to growStrings. With WithCopyStrings: github_events -15.1%, apache_builds -11.1%, gsoc-2018 -10.5%, twitterescaped -8.1%, twitter -7.5%. Without it the shared append still helps: github_events -5.1%, gsoc-2018 -2.8%, twitterescaped -1.7%, apache_builds -1.4%. Geomean -4.9%. update-center is +4.0% in both modes, which is the concurrent path's scatter rather than this change - it sits just over the cutoff at this point in the series.
The 8 KB cutoff for running stage 1 and stage 2 one after the other predates any measurement. Forcing each mode over the corpus puts the real crossing between 215 KB and 500 KB: below it the goroutine, parking stage 2 until the first buffer arrives and waking it on another core cost more than the overlap returns. This is measured after the string path stopped walking a copied string twice, which took a fifth off stage 2 and could have moved the crossing - it did not. 256 KB takes the sequential side of the band. Where the two are close the sequential path is the better bargain anyway: same wall clock on one core rather than two, which is what a server parsing several documents at once actually pays for. The synchronous path holds every buffer stage 1 produces live at once, so a 256 KB document needs more than the sixteen ring slots. syncBuffer extends the ring past them with buffers indexed by round, kept between parses, so a parser settles at the largest document it has seen and stops allocating. The cutoff bounds that: a message of 256 KB cannot hold more indexes than bytes, so the most a parser ends up holding is about 900 KB, and only if it was handed the densest possible document at the top of the range. TestSyncPathMultipleIndexBuffers covers the extension. The documents that change sides: apache_builds -2.1% copy and -3.0% nocopy, instruments -1.6% and -0.7%, numbers -1.4%, github_events +2.5% and +0.2%. Geomean -0.75%. So on wall clock this is close to neutral, and smaller than the reference figures taken before the string path was fused - a faster stage 2 moves the crossing down. It is kept for the other half of the argument: below the cutoff the same wall clock is reached on one core rather than two, which is what a server parsing several documents at once pays for.
Every structural character cost two running totals: POSITION advanced by the increment just written, and CARRIED was worked out from the last position after the loop. Neither needs the loop. The increments cancel, so the run as a whole only moves POSITION from the previous structural character to the last one in this block, and the last one is wherever the highest set bit is - both come out of the mask with one LZCNT before any of them is written. Stepping the previous position on by the increment rather than reloading it from the trailing-zero count drops another move. Eight instructions in the loop where there were ten. The obvious other half of this did not work. The reference simdjson writes eight indexes whether or not there are eight and moves its pointer on by the population count, trading a mispredicted loop exit for straight-line stores; done here it was 10.9% slower in the routine and 8% slower in stage 1. Their indexes are absolute and independent, so a wasted slot costs one store; ours are increments off the previous position, so a wasted slot costs a full five-instruction step in a serial chain - and the corpus averages one to ten structural characters per 64 byte window, well under eight. BenchmarkFlattenBits -3.3%, BenchmarkStage1 -2.2%, and invisible end to end - flattening is about 6% of a parse. Committed as a simplification that measures faster where it can be measured, not as a win.
The AVX2 pipeline rebuilt its constants on every 64 byte block: each step loaded its shuffle tables and character masks from memory into scratch Y registers before doing any work, and handed its results back through the caller's stack frame for the next step to load again. The AVX-512 pipeline already avoided both, keeping its constants in fixed Z registers loaded once per document. The AVX2 path now does the same. Three __init_*_avx2 routines fill fixed Y registers at the top of find_structural_bits_in_slice, the steps read them from there, and the masks move between steps in registers instead of through memory. The register map is documented at the top of find_structural_bits_amd64.s, with the shared names in common.h. BenchmarkStage1AVX2 is new and necessary: this machine has AVX-512, so the default path never touches the AVX2 assembly and a change to it cannot otherwise be timed on the hardware most likely to be developing it - while AVX2 is what most CPUs in the field run. It forces haveAVX512 off and measures stage 1 alone, so a few percent is not swamped by stage 2. Correctness of the AVX2 path over real documents is TestStage1PathsAgree, which compares it against AVX-512 across the corpus. BenchmarkStage1AVX2, which is the only thing on this machine that runs the AVX2 pipeline: all fourteen documents between -7.1% and -12.2%, geomean -10.0%. gsoc-2018 -12.2%, numbers -11.4%, twitter -11.1%, instruments -11.6%, marine_ik -7.1%.
BenchmarkIterRead put 41% of a typed walk in walking the tape and 13% in getting at string bytes - and a quarter of that second figure was the call to stringByteAt, which two fmt.Errorf paths kept far outside the inlining budget. The range check splits into an inlinable stringBytes with the error left to a noinline helper, and Iter.StringBytes and Object.NextElementBytes call it directly. The other half of this did not work. calcNext picks how far to skip from the tag, so a lookup table should beat the switch, and two thirds of its time was branch dispatch. But addNext feeds the next element's offset, so replacing compares with a load puts that load on the walk's own dependency chain: the typed walk gained and a plain walk lost 20-33%. The switch stays, with a comment saying why. BenchmarkIterRead, geomean -4.0%. The object walk, which reads a key name per element: apache_builds -10.2%, github_events -9.0%, instruments -9.3%, citm_catalog -5.9%, twitter -5.7%, update-center -4.5%. The typed walk -3.9% to -8.7%. The plain walk, which converts nothing, is flat, and so is canada in every mode - it has almost no strings, which is the control this wants.
Four files handled floats and their names did not say which direction any of them went: eisel_lemire.go parsed, ftoaryu.go and appendfloat_f.go formatted. They are now atof_eisel_lemire.go, ftoa_ryu.go and ftoa_append.go. The substantive part is detailedPowersOfTen. It lived in ftoaryu.go, 700 of that file's 1089 lines, with its two bounds constants a further 60 lines up among the Ryu helpers - so two thirds of a file nominally about formatting was data, and the parser on the stage 2 hot path had to depend on the formatter's file to reach it. The table and its bounds are now detailed_powers_of_ten.go, which neither direction owns and both read. It keeps the BSD header, since that is where it came from. TestDetailedPowersOfTen moves alongside the data it checks, and takes abs with it, its only caller. No executable code changed: every non-comment, non-blank line of the old ftoaryu.go was compared against the union of the two files it became.
Nine unused declarations, none of them reachable. All but one were already unused on master, and the exception was never used either. write_tape_s64 and write_tape_double have no callers, and writeTapeTagVal was alive only because those two called it; stage 2 writes through writeTapeTagValFlags. A grep for writeTapeTagVal appears to find a caller, but it matches the Flags name as a substring. mult64bitPow10 is the float32 half of the Ryu port - it takes a 25-bit mantissa and reads only the high word of each table row - and nothing here formats a float32, so it has been dead since the file was vendored. expbits in computeBounds and the neg field of decimalSlice go the same way, since fmtF takes the sign as a parameter. serializeNDStream and Serializer.splitBlocks are an unfinished streaming API with no exported entry points, and splitBlocks was the only reader of Serializer.maxBlockSize, which NewSerializer still set. No behaviour and no reachable code changes; the tape and re-serialized JSON over the corpus are unchanged.
Stage 1 filled buffers of 1536 indexes and passed them to stage 2 sixteen slots at a time, both numbers arrived at by feel. Widening the buffer to 6144 and cutting the slots to six leaves about as much in flight as before while the stages synchronise four times less often, which is what a parse actually pays for. Stage 2 was never found waiting on a buffer anywhere in the corpus, so this is the cost of the handover itself, not a stall. citm_catalog -19.7% copy and -21.8% nocopy, update-center -15.5% and -16.9%, random -11.7% and -11.8%, twitter -2.5% and -1.8%; geomean -8.0%. Against that gsoc-2018 copy is +9.4% and twitterescaped +1.9%, so this is not free everywhere. It is last in this series on purpose. Measured at the front, before stage 2 got faster, the same change was +1.0% geomean with twitter and twitterescaped 3% slower: what it removes is handover cost, and that is only worth removing once the work either side of it is cheap. The two constants were also swept apart to tell them apart - more memory in flight buys nothing on its own and costs the number-heavy documents.
The parser checks everything else about a string - escapes, surrogate pairs, unescaped control characters - but passes malformed UTF-8 through untouched, so a caller that treats the result as text can end up with invalid sequences it never put there. WithValidateUTF8(true) rejects those. It is off by default, because it is a separate pass over the message and callers have relied on the existing behaviour for years. The check is done up front and over the whole message rather than per string: a byte above 0x7f outside a string is neither structural nor whitespace and starts no valid value, so the stages reject it anyway, and doing it once avoids touching the hot path at all. Cost when enabled, over the corpus: about 0.5% on number-heavy input, up to 19% where the document is both string-dense and largely non-ASCII - utf8.Valid runs far faster over plain ASCII than over multibyte sequences. Making it cheap would need an AVX2 validator, which is not what this commit is. utf8_test.go covers the classes a checker has to get right - bare continuation bytes, truncated sequences, overlong encodings, the surrogate range, code points above U+10FFFF - and asserts that without the option every one of them still parses.
Everything except amd64 had no parser at all: SupportedCPU() was false and Parse returned an error. stage1_find_marks_generic.go and parse_string_generic.go are the same algorithm in Go - the step the vector units do, turning 64 bytes into the masks the rest of stage 1 works on, becomes a table lookup per byte, and the string routines become bytes.IndexByte plus a byte-at-a-time escape decoder. They are compiled on every platform, including amd64, so that they can be checked against the assembly rather than only against expectations. stage1_generic_diff_amd64_test.go and parse_string_generic_diff_amd64_test.go run both implementations over the corpus and over generated input - escapes, surrogates, raw control characters, unterminated strings, every closing-quote offset within a word - and require the same acceptance and the same bytes out. Two things only showed up that way: the VPSHUFB bit-7 rule the portable classifier has to reproduce, where a shuffle index with the high bit set yields zero rather than indexing, and a `room < 12` bound that turned out to be genuinely unobservable rather than untested. The hex-digit and escape tables match the assembly's LCDATA1 block entry for entry, which is what keeps the two accepting the same input - including the 0x00-0x2f entries the assembly was missing until they were spelled out. Nothing routes to this path yet; that is the next commit.
SupportedCPU() was false everywhere except amd64 with AVX2, and Parse returned an error.
The portable implementation added in the previous commit now runs there instead: the
per-platform surface is two files, dispatch_amd64.go and dispatch_nosimd.go, holding the
same three declarations - SupportedCPU, findStructuralBitsInSlice, and the two string
routines - and everything else loses its `_amd64` suffix and its build tags.
simdjson_other.go, which existed only to return "unsupported CPU", is gone.
The equivalence digest over the corpus is byte for byte identical between the assembly and
the `-tags=noasm` build, tape word for tape word and re-serialized JSON for re-serialized
JSON, in both copy modes.
amd64 without AVX2 is the one case left with no fallback, deliberately: the feature test
would have to go in the two string routines, which run per string in the hottest part of
stage 2, and that measured at over 2% for every other user. Such a CPU gets an error naming
-tags=noasm.
Compiling stage 2 and stage 1 for a 32-bit platform for the first time turned up two real
defects, both fixed here rather than left for the first 386 user to find:
- atomic.AddUint64 on a plain uint64 struct field panics with "unaligned 64-bit atomic
operation" on 386, since 8 byte alignment is not guaranteed there. The field is
atomic.Uint64 now, which carries the guarantee. It is reachable from Parse for any
document over syncSizeLimit, so citm_catalog was enough to hit it.
- uint64(STRINGBUFBIT+start) is an int expression before the conversion and does not fit
32 bits.
A test helper that took internalParsedJson by value now takes a pointer, since the struct
holds an atomic.
The portable stage 1 spends nearly all of its time in one step: turning 64 bytes into the whitespace and structural masks, which the assembly does with VPSHUFB and the Go version does with a table lookup per byte. stage1_classify_arm64.s does that step with NEON - TBL over the same nibble tables, then a compare and a mask narrowing - and arm64 shares the rest of stage 1, and all of stage 2, with the portable path. Nothing else on arm64 gets assembly. The bit arithmetic after classification compiles about as well from Go as it would by hand, and the string parser's byte scanning already goes through bytes.IndexByte, which is NEON in the runtime; what is left there is branchy escape decoding. A fused copy-and-find like the AVX2 one would still be worth something, but there is no arm64 hardware here and emulation cannot time anything, and assembly nobody has measured is a liability. stage1_classify_arm64_test.go checks the NEON classifier against the portable one over the corpus and over generated blocks, which is the arm64 half of what stage1_generic_diff_amd64_test.go does on amd64. Verified under qemu-aarch64: the full test suite passes, and the equivalence digest from the arm64 build is byte for byte identical to the amd64 one across all 18 corpus documents in both copy modes. No speed claim: emulating NEON costs more host work than emulating the scalar code it replaces, so any timing from this machine would be an artefact. The static figure is that the classifier is 124 straight-line instructions against a 36 instruction body run 64 times.
BenchmarkSimdJsonTwitterEscaped against BenchmarkEncodingJsonTwitterescaped and BenchmarkSonicTwitterescaped: one capital letter, and it is enough that benchstat cannot pair the rows and a script comparing the groups silently drops that document. It is the one file in the corpus where escape handling dominates, so it is not a good one to lose.
Every figure in the README was taken in 2021 with benchcmp, a tool that has not existed for years, on a 32-core machine, against a version of this library that is now several times slower in places. The serializer table used parking-citations-1M, which the asset host no longer serves, so it could not be reproduced even in principle. All three tables are re-measured here, and each says what produced it: the machine, the Go version, that the runs were pinned, and the command to repeat them. The parse comparison now carries copy and nocopy side by side, and an allocation table next to the throughput one - the allocation gap against encoding/json is three to five orders of magnitude and was not shown before. Two claims in the introduction are corrected. "About 10x faster than encoding/json" turns out to be right on this corpus - geomean 10.8x, from 4.7x to 27.7x - but it was stated without saying that Parse and Unmarshal do different amounts of work, which is the first thing a reader should know; the fair-comparison group in benchmarks/ says 2.7x, and that number is now here too. "40% to 60% of the speed of simdjson" is dropped: nothing in this repository measures the C++ implementation, so the claim cannot be checked here. The inplace-strings section needed rewriting rather than renumbering. Copying used to cost 20-30% on string-heavy documents; now that the copy and the scan are one pass it is a couple of percent, and on the two documents where every string carries escapes the copying path is actually faster - which the old text would have made look like a mistake.
asp24
changed the title
asp24
changed the title
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.
43 commits over
origin/master: correctness and memory-safety fixes, a parser about 35% faster on the whole test corpus, a portable path for platforms without the assembly, and the test and benchmark infrastructure that backs those numbers.The sequence is ordered so it can be read: tests and benchmarks first, then correctness, then performance, then portability, then documentation. Every commit builds and passes
gofmt,go vet,go test ./...andgo test -tags=noasm ./..., and compiles for amd64,-tags=noasmand arm64. Every performance commit states what it measured, against its own parent, on the machine described in the doc comment onBenchmarkParseCorpus.Correctness and memory safety
Nine commits, each with its own test and a negative control:
reflect.SliceHeaderafter anappendon a copy of the slice - a reallocation there would have published uninitialized bytes;maxdepthwas a capacity hint, not a limit, so 2 MB of[allocated an 8 MB scope stack;ParseNDreturned a document without its parser, soreusereused nothing: -25% and 2.4 MB per call;\ufollowed by non-hex digits parsed and produced a NUL, because 48 entries of the assembly's hex table were never written;Array.AsStringover-allocating 2x, andunsafe.Stringreplacing the last hand-built string header.Performance
origin/masterto the tip, medians over three padded builds per side and four interleaved rounds:WithCopyStrings(false)GOMAXPROCS=1ParseNDwith reuseEvery one of the 28 corpus rows is negative. The larger items: short numbers converted without
strconv, long mantissas through Eisel-Lemire with a SWAR digit fold, the string copy fused into the scan, one tape append per string, the channel skipped when the stages run in sequence, the AVX2 stage 1 constants hoisted into registers, and the index buffers handed over four times less often.Portability
A portable Go stage 1 and string parser, diffed against the assembly over the corpus, and NEON classification on arm64. The equivalence digest - tape words plus re-serialized JSON, every document, both copy modes - is byte for byte identical to
origin/masterfor the amd64,-tags=noasmand arm64 builds. Compiling stage 2 for 32-bit for the first time turned up two real defects, both fixed here: an unaligned 64-bit atomic that panics on 386, and anintoverflow in a tape offset.Tests and benchmarks
go test ./...did not pass on master. It does now. Added: a corpus benchmark for the parser, a benchmark and a leaf-count test for reading a parsed document, the AVX2/AVX-512 pipeline agreement test, the portable/assembly differential tests, aBenchmarkStage1AVX2that is the only way to time the AVX2 path on a machine with AVX-512, and asoniccomparison group where both libraries are asked for the same thing.Licensing and attribution
Worth reading if you are reviewing the fork rather than the code. Upstream ships
MinIO Cloud Storage, (C) 2020 MinIO, Inc.on every file, including files it never wrote. Here each file says what it is, and the notice lands in the commit that creates or first changes that file, so no commit carries one that is wrong:Copyright (c) 2026 DATA.BETaloneModifications copyright (c) 2026 DATA.BETcommon.h,options.go- upstream gives them no notice at allCopyright 2009/2020 The Go Authors+ BSD-3Two substantive corrections, not just relabelling:
LICENSE-gois added with the Go BSD-3 text. Four files come from the standard library and their headers say the licence "can be found in the LICENSE file" - andLICENSEhere is Apache-2.0, so the terms those files actually stand under, and the disclaimer BSD-3 requires to be reproduced, were nowhere in the tree.atof_eisel_lemire.gonow carries the Go Authors notice instead of an Apache one. It is a re-implementation ofeiselLemire64fromstrconv: the constants, the branch conditions and the order of the bail-outs are the standard library's.NOTICEstates what this repository is and how to read a file's notice. The modification notices on changed files are also what Apache-2.0 section 4(b) asks of anyone distributing a changed copy.What to look at first
benchmarks_test.go- the measurement protocol every figure in these messages was taken with, and why a single before/after pair here cannot be trusted.NOTICEand the table above - who owns what, and under which licence.go.modisgo 1.25.0, which is what the dependencies require, not what the toolchain happens to be.