Performance: cut context, digit-count and float-conversion overhead - #241
Merged
Conversation
Follow-up to ericmj#240, on the paths that pass turned out to dominate what is left. Behaviour is unchanged: results, struct representations, error messages and the context flags (including their order) are identical, checked by dumping the whole public surface over 146k cases against the previous implementation and diffing. Numbers below are per operation on an Apple M4 Max, Elixir 1.19.5 / OTP 28, measured on production-compiled code, as wall time and as reductions - the deterministic work count, which is immune to machine noise. Allocation is heap words per operation. operation wall reductions allocation div -47% -53% -28% add (34-digit) -57% -36% -21% div (34-digit) -42% -44% -21% mult (34-digit) -40% -42% -20% to_float -37% -69% -9% to_float (1e-300) -92% -96% 0% sub -27% -52% -19% compare -27% -59% -27% add -26% -53% -23% round -24% -44% -19% normalize -22% -43% -19% mult -22% -45% -17% sqrt (34-digit) -21% -35% -11% from_float -15% -11% -35% new (from string) -13% -36% -31% What changed: * Operations that signal nothing no longer touch the context. The flag bookkeeping ran unconditionally: wrapping the signal list, folding it into the flags, copying the context and writing it back to the process dictionary, then scanning for a trap - about a quarter of the cost of `add/2` and half of `round/3` to conclude nothing happened. An empty signal list now returns the result directly, and a signal that is already recorded skips the write, so a loop of inexact operations writes the context once instead of every time. * The result's digit count is computed once per operation. `precision/4` counted the coefficient's digits, then `exponent_limits/2` counted them again, and `add/2` counted both operands' digits purely to decide whether the exponent gap needed the bounded path. Rounding now returns the count it already knows, and division passes the count its own invariant pins to `precision + 1`. * `coef_length/1` compares against machine-word literals only. Comparing a bignum costs an order of magnitude more than comparing a small integer, so walking eighteen rungs before reaching the bit-length estimate cost more than the estimate: 34-digit coefficients counted their digits 3x faster now, and no coefficient counts them slower. * Equal exponents short-circuit `compare/2` and `add/2`. With equal exponents the adjusted exponents differ exactly as the coefficient lengths do, so comparison needs no digit counting at all, and alignment is free, so the bounded path has nothing to protect against and would compute the same sum from the same `base_exp`. * `to_float/1` scales with a computed shift. Both scaling loops moved one bit per iteration, allocating a bignum each time: ~50 iterations for 1.5 and over a thousand near the ends of the double range. The shift is the difference of the operands' bit lengths, exact to within one bit, so one comparison settles it. Values comfortably inside the range also skip the DBL_MIN/DBL_MAX comparisons, which built three decimals and ran up to three full comparisons to establish that an exponent was nowhere near the limits. * Parsing reads the coefficient out of the scan. Digits were accumulated into a character list, reversed and converted; they are now accumulated directly into an integer while they fit a machine word, with longer runs converted from the input slices in one step. The exponent is bounded from its digit count instead of by building a character list for the bound and comparing digit by digit, and the default limits are a compile-time constant instead of a map rebuilt per call. * Smaller ones: `from_float/1` matches the redundant `.0` going forward instead of accumulating and reversing, dropping a cons cell per character; dividing off a single guard digit uses `div`/`rem` instead of two powers of ten; a remainder that does not end in zero is rejected as a power of ten by one division rather than a table of 105 literals; `to_string/2` reuses its digit count; `inspect/1` builds one binary instead of concatenating three. Tests: the rounding, comparison, parsing, float conversion and flag paths above have unit tests pinning them, all of which also pass against the previous implementation, plus properties checking `compare/2` against integer comparison of scaled coefficients, `add/2` against exact integer addition, `to_float/1` against the runtime's own decimal-to-float conversion, and `parse/1` against building the coefficient from its digits. Tools used to produce the numbers, since wall time alone is not a verdict: `bench_resources.exs` reports wall, CPU across all schedulers, reductions, allocation, collector copying, collection counts and peak footprint, and runs every workload twice - once discarding results, once holding a live set and retaining them, because allocation looks free when the garbage dies before the collector runs. `verify_diff.exs` dumps the public surface for a build so two builds can be diffed. `bench.exs` grew jobs for the parsing, conversion and same-scale arithmetic it did not cover, and both it and the new harness refuse to run against a non-production build.
The comparison lived in a Python script that re-parsed the printed table.
It is now a mode of the harness itself: `SAVE=<file>` writes the
measurements as a term, `COMPARE=<file>` reads them back and prints each
metric next to its change, so nothing parses formatted output and the repo
keeps one toolchain.
DECIMAL_PATH=../decimal-main SAVE=/tmp/main.bench MIX_ENV=prod \
elixir bench_resources.exs
COMPARE=/tmp/main.bench MIX_ENV=prod elixir bench_resources.exs
Review pass over the previous two commits. * Comments trimmed to the reasoning that is not in the code, and two that had gone stale: the scaling comments in `to_float/1` described the one-bit-at-a-time loop they replaced as if it were still there, and said nothing about why one branch of `scale_down/3` shifts 53 rather than 52. * `exp_value/8` checked the parsed exponent against `max_exponent + float_size` and then against the limit itself. The first check cannot reject anything the second accepts - for a positive exponent `value - float_size <= max` gives `value <= max + float_size`, and for a negative one the limit is stricter still - so it only obscured which check does the work. The digit-count check above it stays: that is what keeps a hostile exponent from being turned into an integer. * `signed_exp/2` folded into `exponent/3`, which now applies the sign and the fraction digits in one place instead of at each call site. * `default_parse_limits/0` removed in favour of the constant it returned, now declared with the other defaults at the top of the module. * The flag ordering assertion in the accumulation test says why it pins the order rather than the membership. Verified unchanged: 146,539 differential cases identical to the previous implementation, suite green on Elixir 1.12.3/OTP 24 and 1.19.5/OTP 28.
The bounded addition path exists to stop coefficient amplification from an exponent gap (CVE-2026-32686), and the equal-exponent short-circuit added here skips it, so say why that is safe where the code does it and cover the shape with a test: with nothing to align there is nothing to amplify, and the bounded path would pick `base_exp == exp1`, scale both coefficients by 10^0 and compute the same sum. Also note in the parser that the digit limit is still checked before the coefficient is built, which is the property the mitigation relies on.
Adds a DECIMAL_PATH env (default ".") so the same script can benchmark a different checkout, and a LOAD env so a previously saved run is loaded into the next one and Benchee prints both per job. The tag is now derived from the checkout under test with --abbrev-ref, so a worktree at a bare commit tags itself HEAD-<hash> instead of failing.
A nonzero division remainder is the sticky bit, and precision/5 signals :inexact whenever it is set, so whether the remainder was a power of ten never changed the flags - the branch that suppressed :inexact for a power-of-ten remainder could not produce a different result than the one that did not. Confirmed by forcing base10?/1 to false: 146,539 differential cases unchanged. Removed entirely, along with its 105-clause literal table and the pow10_max binding that only it used.
…minimal A coefficient that does not end in zero has no trailing zeros to strip, so the input struct is already the normalized value. The function rebuilt a fresh struct in that case - twice, through do_normalize_one and the sign-update - to produce an equal value. Return the input directly instead. Allocation drops 42% on normalize and 15% on round, which normalizes first. Behaviour is unchanged: the struct is equal by value, only its identity differs.
The accumulator guard counted leading zeros, so an input like "00...01.5" left the machine-word path and converted through binary_to_integer despite having two significant digits. Guard on count - leading_zeros, key parse_coef on the significant digit count, and let exponent digits reuse the same rule. Also drop the fraction binary threaded through parse_unsign and parse_coef: a fraction exists only when a dot was consumed, so its digits sit at int_size + 1 in the input.
check_dbl_min_max/1 inlined the adjusted-exponent formula that adjust_exp/1 already implements. The Inspect rewrite compiles to the same single bs_create_bin instruction as plain binary concatenation, so revert it. Drop two comments describing the implementations this branch removed.
bench_resources.exs took its end timestamps after the worker hashed the 20k-decimal live set, charging ~860us of harness overhead to every run, and the spawn closure's copy of the live set was billed to the workload's allocation metric. Hash only after the parent has read every measurement, and baseline reductions and heap on the pre-run process. copy/op added heap_size + old_heap_size at every collection end, but a minor collection never touches the already promoted old heap. Pair each end event with its start and count the surviving young heap plus what was promoted; a major collection still counts the whole live set. Zero-pad the money cents so every value has exponent -2 and the same-exponent paths are what run; 8% of the zipped pairs had unequal exponents. The install preamble and the money workload now live in bench_helper.exs instead of two diverging copies. bench.exs fails on a LOAD path that matches no saved file instead of silently dropping the comparison, and the formatter covers every root script.
Calling the function instead of the arithmetic check_dbl_min_max/1 spelled out by hand cost two reductions per to_float/1 call, +7% reds/op on small values. The directive keeps the single definition at the inlined cost; compare/2 and the add/sub estimates call it on hot paths too.
Owner
|
This is great! Thank you! 💜 |
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.
Follow-up to #240, on the paths profiling showed still dominated. No public function changes: results, struct representations, error messages and the context flags (including order) are identical — checked by dumping the whole public surface for both builds and diffing, 146,539 cases, byte-identical.
Benchmark
bench.exs, baseline saved and loaded into the candidate run so both come from the same harness on the same machine (Apple M4 Max, Elixir 1.19.5 / OTP 28). Each row is one Benchee job over its whole workload.Allocation per operation (heap words; 1 word = 8 bytes), from
bench_resources.exs— the deterministic per-operation figure, independent of how many iterations a job runs, unlike the workload-time table above.to_float tinyandto_stringare unchanged in allocation: the first is dominated by building the10^300denominator (inherent to the conversion), and the second's output binary is the irreducible cost. The biggest spender,from_float, is the charlist round-trip through:io_lib_format.fwrite_g/1; a binary path exists but trades ~10% wall time for the memory, so it was left alone.Reductions — the deterministic work count — drop 20-55%.
bench_resources.exsreports wall, CPU, reductions, allocation, collector copying and peak footprint per operation, and is what caught two candidates that lowered allocation but cost wall time (rejected, below).What changed
add/2, ~50% ofround/3— to conclude nothing happened. An empty signal list returns directly; an already-recorded signal skips the write.precision/5returns it;exponent_limits/3takes it;add/2no longer counts both operands' digits just to decide on the bounded path;div/2passes the count its own invariant pins toprecision + 1.coef_length/1compares against machine-word literals only. Bignum comparisons cost an order of magnitude more, so walking 18 rungs before the bit-length estimate cost more than the estimate. 34-digit coefficients count ~3x faster; nothing counts slower.compare/2andadd/2. With equal exponents the coefficients decide alone; foradd/2there is nothing to align, so the bounded path (the CVE-2026-32686 mitigation) has nothing to guard against — every exponent-gap case still does. A test pins that a 5,000-digit coefficient at an equal exponent stays bounded.to_float/1scales with a computed shift. Both scaling loops moved one bit per iteration, allocating a bignum each time (~50 for1.5, over a thousand near the range ends). The shift is the difference of the bit lengths, exact to within one bit.from_float/1matches the redundant.0forward; dropping a single guard digit usesdiv/rem;to_string/2reuses its digit count;Inspectbuilds one binary;normalize/1returns its input struct when the coefficient has no trailing zero instead of rebuilding it (−42% allocation onnormalize, and onroundwhich calls it). The unreachablebase10?/1branch indiv_calc/4was removed (a nonzero remainder always signals:inexactthrough the sticky bit, so it could never change the flags).Compatibility
No public function changed signature, spec, docs, or behaviour; everything touched was private. Elixir 1.12.3/OTP 24 and 1.19.5/OTP 28 verified,
--warnings-as-errorsclean; nothing newer than Elixir 1.12 is used.Testing
The 146,539-case differential covers pairwise arithmetic over a coefficient/exponent matrix, all 7 rounding modes × 7 place counts, non-default precision/
emax/emin/traps, 16,800 fuzzed parse inputs, 29,622 float conversions across the full exponent range, and multi-operation flag sequences. New unit tests and properties pin the changed paths against independent oracles (comparevs scaled-integer comparison,to_floatvs the runtime's own conversion,parsevs building the coefficient from its digits); all pass against the previous implementation too.Prior work
Builds directly on work in the tree; several changes are re-tunings of code someone else introduced:
div_calc/4,split_digits/3,increment?/5, square-and-multiplypow10/1, the bench script. Its note that the fixedsqrt/1seed already lands within a decade of the root checked out — a bit-length seed measured no faster, sosqrt/1is untouched.base10?/1branch above unreachable.coef_length/1, the bit-length digit-count estimate, the bounded addition path, chunked normalize. Three load-bearing properties are preserved: bignum digit counting stays estimate-based, parsing rejects on counts before converting, and every addition with an exponent gap still goes through the bounded path.to_float/1's algorithm (26e1443, 2016), including the Rick Regan reference — only the scaling loops changed.bench_resources.exs.Notes for reviewers
Highest-risk areas are
scale_up/3/scale_down/3(the closed-form shift and its one-bit correction) and the digit count threaded throughprecision/5→exponent_limits/3; theto_floatoracle test and the differential dump make errors there loud.bench_resources.exsandverify_diff.exsare included as the measurement and verification tools; happy to drop either if they don't belong in the repo.Rejected candidates
binary_to_integer/1on input slices for the parser — −32% reductions but +37% wall (BIF work is cheap in reductions, not in cycles). Accumulating during the scan won.:binary.match/2to splice the redundant.0infrom_float/1— −39% reductions, 3.1x slower (pattern compiled per call).sqrt/1— no change, confirming Performance #240's note.to_string/2— 1.5x slower (each nested construction copies what it has built).🤖 Generated with OpenCode using Claude. Prompted and reviewed by me.