Performance - #240
Merged
Merged
Conversation
…1.8x, allocations down 80-94%. No API or behavior changes.
Owner
|
Beautiful, thank you! |
ericmj
added a commit
that referenced
this pull request
Aug 20, 2026
…241) * Performance: cut context, digit-count and float-conversion overhead Follow-up to #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. * Keep bench.exs to_float operands inside the double range * Compare benchmark runs in Elixir instead of a Python script 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 * Tighten comments and drop a redundant exponent check 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. * Pin that equal exponents cannot amplify a coefficient 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. * Make bench.exs measure another checkout and load a saved run 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. * Drop the unreachable base10?/1 branch in div_calc/4 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. * normalize/1: return the input struct when the coefficient is already 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. * Keep the digit accumulator running past leading zeros 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. * Remove duplication and churn from the perf pass 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. * Make the bench scripts measure what they claim 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. * Inline adjust_exp/1 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. --------- Co-authored-by: Eric Meadows-Jönsson <eric.meadows.jonsson@gmail.com>
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.
Rewrites the hot paths of the library: division, context rounding,
round/3, comparison, and power-of-ten computation to use native bignum arithmetic instead of digit at a time loops and charlist manipulation. No public API changes, no behavior changes; results, struct representations, and signal flags are byte-identical to the current implementation, verified by the existing suite plus new unit and property tests added in this PR.Headline numbers (M1, Elixir 1.19.5 / OTP 29, benchmarked with the extended
bench.exsin this PR):div(mixed sizes)div(34-digit operands)add(34-digit)mult(34-digit)add/submultroundcomparesqrtto_stringMemory allocation drops are larger than the speedups:
compare−94% (1135 MB → 68 MB per benchmark iteration),div−82%,add−83%,mult−77%,sqrt−89%.Change-by-change
1.
bench.exs: cover the full operation surfaceThe benchmark previously measured only
compare/2. It now has 14 jobs:add,sub,mult,divover a deterministic sample of the operand matrix; the same four over 34-digit (decimal128-precision) operands where division and rounding costs dominate;round,normalize,sqrt,to_string(:scientificand:normal); and a same-scalecompareworkload (equal exponents, equal coefficient lengths — the shape of money comparisons) that always reaches coefficient alignment instead of short-circuiting on the adjusted exponent. The originalcomparejob keeps its exact workload so previously saved results remain comparable.2.
compare/2: no padding when exponents are equalpad_num/2multiplied both coefficients by at least 10 on every coefficient-level comparison (pow10(max(n, 0) + 1)), even when the exponents were already equal. The comparison branch now reusesadd_align/4: equal exponents compare coefficients directly with zero multiplications; unequal exponents multiply only the larger-exponent side bypow10(diff). Since this branch is only reached when adjusted exponents are equal, the exponent gap is bounded by the coefficient-length gap, so the padding stays small.pad_num/2had no other callers and is deleted.3.
pow10/1: binary powering;decimal_power10/1deletedAbove its 104-entry compile-time table,
pow10/1recursed aspow10(104) * pow10(n - 104)— a linear chain multiplying an ever-growing bignum, quadratic overall. It now uses square-and-multiply (recurse onn >>> 1, square, multiply by 10 when odd): O(log n) multiplications.This also let us delete
decimal_power10/1, which computed 10^n by building and parsing a string (binary_to_integer("1" <> zeroes(n))). That function sat ininteger_decimal_digit_count/2's verification step — meaning every digit count of a >18-digit coefficient paid one or two string-build-and-parse cycles. This predates the PR and was already the dominant allocation cost incompare(viaadjust_exp); an intermediate benchmark run during development showed it regressing add/mult by 36–80% once the new rounding fast path leaned on digit counting harder, which is how it was found. All three call sites now usepow10/1(a table lookup for anything ≤104 digits). This single change accounts for most of the memory reduction across the library.4. Division: one native division instead of a digit loop
div_calc/5computed each quotient digit by repeated subtraction — up to 9 interpreted bignum subtractions plus two multiplications per digit, ~300 bignum operations per division at precision 34. The loop is equivalent to a single scaled division, so it is now one:div_adjust/2aligns the operands tocoef2 <= coef1 < 10 * coef2by computing the shift fromcoef_length/1digit counts (O(1)) instead of one power of ten per iteration.div_calc/4computesdiv(coef1 * pow10(precision), coef2): given the alignment invariant, the quotient has exactlyprecision + 1digits (the last being the guard digit), and the remainder is the sticky bit carried into rounding, exactly as before.div(100, 1)is still100, not1E+2).div_int_calc/5is deleted.integer_division/5uses the same closed form, and because the quotient's digit count is known up front (exp1 - exp2 - adjust + 1), quotients over the precision limit are rejected before being materialized. The old loop's iteration count was proportional to the exponent gap, unbounded for hostile input; the new check is O(1).Two loop quirks are deliberately preserved (and now pinned by tests):
div(1e40-as-coefficient, 1)) signals[:inexact, :rounded]exactly as the loop did — arguably a latent bug, kept for parity and documented in a test so any future fix is a conscious decision.base10?signal check runs on the closed-form remainder, which in some alignments is 10x the loop's remainder — equivalent, since being a power of ten is invariant under that scaling, and the remainder is otherwise only used as a boolean sticky bit.5. Context rounding (
precision/4): O(1) fast path, integer arithmeticThis function runs on virtually every arithmetic result. It previously started with
:erlang.integer_to_list(coef)+length/1— an O(digits) conversion and traversal — before discovering, in the common case, that nothing needed rounding. It now:coef_length/1(a guard-clause chain up to 18 digits, bit-length estimate beyond), and returns the number untouched with zero allocation when it fits the precision and no sticky bit is set;div/remvia the newsplit_digits/3helper instead of:lists.split;signif + 1instead of walking a reversed charlist carrying nines;signif == pow10(precision).increment?/5now decides on three integers — guard digit, rest-nonzero (sticky folded in), and significand parity (rem(signif, 2)— the parity of an integer equals the parity of its last decimal digit) — instead of pattern-matching charlists. This is the classic guard/round/sticky formulation: a rounding decision never needs the digit sequence, only those three facts. The charlist helpers (digits_increment,digits_to_integer,any_nonzero) are deleted.6.
round/3: integerdo_round/5Same representation change as #5, sharing
split_digits/3and the newincrement?/5. Appending zeros becomescoef * pow10(k). The old code needed two separateexp < target_expbranches because:lists.split/2requires an in-range index — when more digits were dropped than existed it first prepended literal zero characters. Integerdiv/remhas no such domain restriction (div(coef, pow10(drop))is simply 0 whendropexceeds the digit count), so that branch disappears.The
normalize/1call at the top ofround/3is deliberately kept: removing it would change behavior for coefficients exceeding context precision (normalize applies context rounding before the placewise round), and with the new fast path its cost is negligible for in-precision values.7.
sqrt/1: digit count without list conversioninteger_to_list+lengthreplaced bycoef_length/1. A bit-length-based Newton seed was prototyped and abandoned: benchmarking showed the existingshiftconstruction already scales the operand so the root lies within one decade of the fixedpow10(precision + 1)seed, so the "better" seed saved no iterations and cost anencode_unsignedper call. The seed logic is unchanged; its comment now explains why the fixed power of ten is a valid (and required) over-estimate.sqrt's speedup comes from #3 and #5.8. Context threading
add,div, andsqrtread the process-dictionary context two or three times per operation (e.g.addinadd_bounded?,add_bounded,add_sign, andcontext/3). Each now reads it once at entry and passes the struct down via a newcontext/4. Not observable — the reads are within one synchronous call, so nothing can mutate the process dictionary between them — but it removes redundant lookups and struct copies.Decimal.Context's public API is untouched.Compatibility
Testing
Context.with(... :floor)blocks in theadd/2andsub/2tests had bare==with noassert) — they were the only coverage of negative-zero under:floorrounding and have never tested anything.div_int/remquotient size limit including the exact10^precisionboundary; the full rounding-mode table from theDecimal.Contextmoduledoc (10 values x 7 modes — previously verified nowhere);round/3when every digit is dropped, for the modes that lacked it; carry-at-context-precision (99999 + 0.5at precision 5);:uprounding at context precision (exact vs. inexact vs. sticky);multprecision rounding with flag assertions;coef_lengthboundaries (18/19 digits and 34/35 digits);sqrtacross magnitudes 1e-100..1e101;:xsdrendering past thepow10table.div_int/remagainstKernel.div/Kernel.rem;div_remreconstruction (q*b + r == a, domains sized to keep it exact); floor/ceiling bracketing of all rounding modes; andsqrtof exact squares.Notes for reviewers
The highest-risk areas are
div_calc/4(the closed-form equivalence to the old loop, including exact-quotient zero stripping) andincrement?/5(the charlist-to-integer translation of each rounding mode). The rounding table test and the bracketing property were added specifically to make errors there loud.🤖 Generated with Claude Code
Reviewed and prompted by me.