perf(bigint): batch digits in to_string_radix for non-power-of-two radixes - #3830
Draft
bobzhang wants to merge 1 commit into
Draft
perf(bigint): batch digits in to_string_radix for non-power-of-two radixes#3830bobzhang wants to merge 1 commit into
bobzhang wants to merge 1 commit into
Conversation
…dixes Closes #3827. The generic radix path extracted one digit per full BigInt division (grade_school_div per output digit, O(n^2) overall). Generalize the radix-10 algorithm already used by to_string: convert limb-by-limb into base chunk = radix^chunk_len slots using only Int64 arithmetic, with chunk chosen as the largest power keeping (slot << RADIX_BIT_LEN) | limb inside Int64, then emit chunk_len digits per slot into a pre-sized buffer. Also drops the -self magnitude copy the old path made for negative inputs. Native release benchmarks on a ~4000-bit value (committed): - radix=7: 1.28 ms -> 83 us (~15x) - radix=36: 762 us -> 95 us (~8x) - both now match the optimized radix-10 path (~105 us) as expected New tests: known values for radixes 3/6/36, from_string round-trips for ten non-power-of-two radixes on a 700+ bit value (positive and negative), and radix-3 digit-length checks across chunk boundaries (3^e for e around 19 and 38). Reviewed by Codex CLI (codex-cli 0.144.1): "Approved; no findings" — with an explicit Int64 overflow bound derivation, slot-count and pos-underflow checks, sign-magnitude equivalence for negatives, and an independent 7,650-case differential check across all 30 applicable radixes. Signed-off-by: Codex CLI <codex@openai.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Coverage Report for CI Build 5187Coverage increased (+0.02%) to 91.288%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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.
Closes #3827.
The generic radix path (radixes 3, 5, 6, 7, ... — anything not 10 or a power of two) extracted one digit per full BigInt division (
grade_school_divper output digit — O(n²) overall). This PR generalizes the limb-by-limb algorithm the radix-10 path already uses: convert into basechunk = radix^chunk_lenslots using onlyInt64arithmetic (withchunkthe largest power keeping(slot << RADIX_BIT_LEN) | limbinsideInt64), then emitchunk_lendigits per slot into a pre-sized buffer. The-selfmagnitude copy for negative inputs is gone too — limbs are sign-magnitude and are read directly, as radix-10 already does.Benchmarks (native release, ~4000-bit value; committed)
Tests
Known values for radixes 3/6/36 (incl. negatives),
from_stringround-trips for ten non-power-of-two radixes on a 700+ bit value (positive and negative), and radix-3 digit-length checks across the chunk boundaries (3^e for e ∈ {1, 18, 19, 20, 37, 38, 39} — 19 digits per chunk).Review
Reviewed by Codex CLI (codex-cli 0.144.1): "Approved; no findings" — including an explicit overflow-bound derivation (
y ≤ chunk·2³² − 2³² + 2³² − 1 < Int64.max), slot-count andpos-underflow verification, sign-magnitude equivalence for negatives, and an independent 7,650-case differential check across all 30 applicable radixes.Signed-off-by: Codex CLI codex@openai.com
Validation
moon checkclean,moon fmtapplied, no.mbtichangesmoon test: 6716 passed, 0 failed (bigint 157/157)🤖 Generated with Claude Code