Parse a long input once instead of twice - #411
Open
Algunenano wants to merge 1 commit into
Open
Algunenano wants to merge 1 commit into
Algunenano wants to merge 1 commit into
Conversation
A mantissa of more than 19 significant digits needs more than 19 characters,
so only a longer input can come back too_many_digits. Today that is discovered
after a full parse without spans, and the input is then parsed a second time
with them. Testing the length up front sends it straight to the parse that
materializes the spans.
benchmarks/benchmark.cpp, Intel Xeon 6975P-C, clang 22.1.8, cycles per float,
best of 5:
canada mesh long fractions 20-38 digit ints >38 digit ints
before 47.30 26.20 233.94 259.73 447.73
after 46.90 25.68 195.68 181.65 273.11
Results are bit-identical over 400k adversarial inputs.
Caveat: the test looks at the range, not at the number. A caller that passes
the whole remaining buffer for a short value takes the slow path on every
value, measured at 20% for 7-character numbers in a large buffer. Deciding on
the token instead costs more than it saves: a 20-character probe loop is
slower than the second parse it avoids.
Member
|
@Algunenano Why is ClickHouse parsing these numbers with > 19 digits ? What is the application? What system is producing these outputs? |
This branch has not been deployed
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
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.
A mantissa of more than 19 significant digits needs more than 19 characters, so only a longer input can come back
too_many_digits. Today that is discovered after a full parse without spans, and the input is then parsed a second time with them, inparse_number_slow_path. Testing the length up front sends it straight to the parse that materializes the spans, so it is parsed once.This comes out of a round of profiling
from_charsin ClickHouse, which parses floats from text for every row of a text format. We have a local helper that does this, parsing once with the spans materialized for inputs known to be long, though our own dispatch reaches it rarely. Of the float-parsing changes we ended up carrying, this is the one that also helps the library's own benchmark datasets, which is why it is the one being proposed.The change is eight lines and does not touch the common path: the length test is under
fastfloat_unlikely, and the parse below still gets the literalstore_spans = falseit had, which is what lets the force-inlined parser drop the span stores entirely.Measurements
benchmarks/benchmark.cppon an Intel Xeon 6975P-C (Granite Rapids) with clang 22.1.8, cycles per float, best of 5.canadaandmeshare the repository's own datasets, the rest are 50k-line sets chosen to hit one path each.Results are bit-identical to
main(value,errcand consumed length, for bothdoubleandfloat) over 400k adversarial inputs: 1 to 84 digit integers, long fractions, leading zeros, explicit exponents, and%.17gof random bit patterns.ctestpasses with clang 22 and with gcc in C++20 mode.The trade-off
The test looks at the range, not at the number. A caller that hands
from_charsthe whole remaining buffer for a short value will take the slow path on every value: measured at 20% for 7-character numbers in a large buffer (9.31 to 11.08 ns per value), against 6% faster when the range is tight (8.09 to 7.59 ns).I tried deciding on the token instead, with a probe loop that counts mantissa characters one at a time, up to twenty, before committing. It costs more than the second parse it avoids: long fractions went to 258 cycles per float, worse than doing nothing. That is a result about that particular probe, though, not about token classification in general. ClickHouse classifies the token as well, but with a SWAR scan that consumes eight digits per iteration and bails out as soon as it sees
.,eorE, and there it pays for itself. I have not tried a SWAR probe here, so this trade-off may be removable. Happy to do that, or to drop or gate the change, whichever you prefer.Two fast paths ClickHouse keeps but that do not belong here
ClickHouse has two more float-parsing fast paths that I am deliberately not proposing. They work there because its dispatch already knows the token length and routes by shape, so each one only runs on the inputs it suits. Inside the library the choice would have to be made blind, and then each one wins on one shape and loses on the rest:
unsigned __int128path for plain integers of at most 38 digits. 27% better on exactly that shape, 7 to 16% worse on every other long input, because of the extra scan needed to recognise it. Placed beforedigit_compinstead of in front of the parse it does nothing at all, since themantissaversusmantissa + 1disambiguation already resolves those inputs.meshand on small integers.This touches the same function as #410, so whichever lands second needs a trivial rebase.