Repository navigation
reduce init time cost by 76% without lazy initialization - #159
Merged
Merged
Conversation
makeConstWithPrecision parsed and rounded the ~2000-digit ln(10) and 1/ln(10) constants at init time via Decimal.SetString and Context.Round. Both need powers of ten far beyond pow10LookupTable to count digits and to round, which made them the bulk of the package's init cost. Since the constants are positive decimal strings, round their digits directly as RoundHalfUp would and build each Decimal from its digits. The test checks every value against Decimal.SetString and Context.Round. The coefficients stay in heap-allocated big.Int values, as Context.Round left them; storing the low-precision ones inline made BenchmarkLn about 4% slower at P10. As measured by https://github.com/mvdan/benchinit: │ old │ mid │ │ sec/op │ sec/op vs base │ GithubComCockroachdbApdV3 694.9µ ± 1% 237.3µ ± 0% -65.85% (p=0.000 n=10) │ old │ mid │ │ B/op │ B/op vs base │ GithubComCockroachdbApdV3 554.5Ki ± 0% 138.6Ki ± 0% -75.01% (p=0.000 n=10) │ old │ mid │ │ allocs/op │ allocs/op vs base │ GithubComCockroachdbApdV3 2.426k ± 0% 1.457k ± 0% -39.94% (p=0.000 n=10) Ln and Log10 are the only users of these constants, and their speed is unaffected. BenchmarkLog10 mirrors BenchmarkLn; both were run with GODEBUG=randautoseed=0 so that both sides use the same random inputs: │ old │ new │ │ sec/op │ sec/op vs base │ Ln/P10/S10/D10-24 12.35µ ± 2% 12.44µ ± 1% ~ (p=0.169 n=8) Log10/P10/S10/D10-24 11.15µ ± 1% 11.20µ ± 9% ~ (p=0.442 n=8) Ln/P100/S10/D10-24 472.2µ ± 2% 473.6µ ± 1% ~ (p=0.328 n=8) Log10/P100/S10/D10-24 462.9µ ± 3% 463.5µ ± 2% ~ (p=0.798 n=8) │ old │ new │ │ allocs/op │ allocs/op vs base │ Ln/P10/S10/D10-24 9.000 ± 0% 9.000 ± 0% ~ (p=1.000 n=8) Log10/P10/S10/D10-24 9.000 ± 0% 9.000 ± 0% ~ (p=1.000 n=8) Ln/P100/S10/D10-24 5.191k ± 0% 5.191k ± 0% ~ (p=1.000 n=8) Log10/P100/S10/D10-24 5.036k ± 0% 5.036k ± 0% ~ (p=1.000 n=8)
pow10LookupTable computed each power of ten with a separate Exp call, and digitsLookupTable computed each entry via String and Exp too. Instead, build each power of ten by multiplying the previous one by ten, and derive each digits entry from pow10LookupTable. As measured by https://github.com/mvdan/benchinit: │ mid │ new │ │ sec/op │ sec/op vs base │ GithubComCockroachdbApdV3 237.3µ ± 0% 165.4µ ± 2% -30.27% (p=0.000 n=10) │ mid │ new │ │ B/op │ B/op vs base │ GithubComCockroachdbApdV3 138.6Ki ± 0% 107.9Ki ± 0% -22.10% (p=0.000 n=10) │ mid │ new │ │ allocs/op │ allocs/op vs base │ GithubComCockroachdbApdV3 1457.0 ± 0% 511.0 ± 0% -64.93% (p=0.000 n=10) The tables hold the same values, so NumDigits is unaffected. It is in fact slightly faster for small numbers, as Exp always left its results in heap-allocated big.Int values, whereas the borders which fit inline are now stored that way: │ old │ new │ │ sec/op │ sec/op vs base │ NumDigitsLookup-24 8.612µ ± 17% 7.542µ ± 8% -12.42% (p=0.014 n=12) NumDigitsFull-24 2.709m ± 3% 2.765m ± 4% ~ (p=0.478 n=12)
Contributor
Author
|
This is a second version of #137, with most of the benefit but no slowdown :) cc @yuzefovich @michae2 |
Document the init ordering between the two lookup tables, the input format makeConstWithPrecision now accepts, and the preconditions of roundDigitsHalfUp. Fix the test message that named PowerOfTenDec instead of tableExp10. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
rafiss
approved these changes
Oct 20, 2026
rafiss
left a comment
rafiss
left a comment
Contributor
There was a problem hiding this comment.
thanks for your contribution! i've adjusted a few comments for clarity and will merge this.
Contributor
Author
|
Thanks very much! |
cueckoo
pushed a commit
to cue-lang/cue
that referenced
this pull request
Oct 21, 2026
Its master builds the rounding constants from their digits directly and the digit lookup tables from each other at init, rather than parsing each constant with Decimal.SetString and rounding it with Context.Round at every precision, which computes powers of ten far beyond the lookup table; see cockroachdb/apd#159. cmd/cue links the package, so every invocation pays its init cost; the total init cost of cmd/cue is reduced as measured by benchinit plus benchstat: │ old │ new │ │ sec/op │ sec/op vs base │ CuelangOrgGoCmdCue 1.683m ± 2% 1.143m ± 1% -32.13% (p=0.000 n=12) │ old │ new │ │ B/op │ B/op vs base │ CuelangOrgGoCmdCue 977.5Ki ± 0% 531.9Ki ± 0% -45.59% (p=0.000 n=12) │ old │ new │ │ allocs/op │ allocs/op vs base │ CuelangOrgGoCmdCue 5.585k ± 0% 3.670k ± 0% -34.29% (p=0.000 n=12) Signed-off-by: Daniel Martí <mvdan@mvdan.cc> Change-Id: Iba39e74df4e09a517d5aa12715edc9e6a73c0d04 GitOrigin-RevId: ceb2e988c72a0b690c751966dc8b5eb935cc3d82
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.
(see commit messages - please do not squash)