perf(stable-api): inline num2long/num2ulong Bignum fast path for MRI#736
Merged
Conversation
2177cd3 to
3f5459d
Compare
88535a9 to
59d5175
Compare
For Bignums that fit in c_long/c_ulong, read BDIGIT digits directly from RBignum instead of calling rb_num2long/rb_num2ulong. Handles both embedded (BIGNUM_EMBED_FLAG set) and heap layouts. On 64-bit: BDIGIT=u32, BIGNUM_EMBED_LEN_MAX=2, c_long=i64. - 0 digits → 0 - 1 digit → always fits i64 (max u32::MAX < i64::MAX) - 2 digits → range-checked before returning - 3+ digits → fall back to dylib (which raises RangeError) Only active on 64-bit targets (#[cfg(target_pointer_width = "64")]). 32-bit always falls through to rb_num2long/rb_num2ulong for correctness. Float coercion and to_int dispatch also fall through to dylib unchanged. Parity tests added for 1-BDIGIT and 2-BDIGIT cases (positive, negative, boundary values) confirming equality with the compiled C shim.
On Windows x64, c_long is i32 (LLP64 ABI), not i64 like on Linux/macOS (LP64). The RBignum fast path was gated only on target_pointer_width="64", which includes Windows, but bignum values like 2**31 exceed i32::MAX and rb_num2long correctly raises RangeError for them on Windows. Our fast path returned a truncated value instead of falling back, causing the parity-test compiled path to raise a Ruby exception that longjmp'd through Rust code — STATUS_ACCESS_VIOLATION on all Windows Ruby versions. Fix: gate the fast path and all its supporting structs/helpers on not(target_os = "windows"), and apply the same cfg to the parity tests so the compiled-path call never raises on Windows.
26df2c0 to
30d124e
Compare
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.
Summary
For Bignums that fit in
c_long/c_ulong, read BDIGIT digits directly fromRBignuminstead of callingrb_num2long/rb_num2ulong. Falls back to the dylib for overflow (which raisesRangeError), Float coercion, andto_intdispatch.Only active on 64-bit targets (
#[cfg(target_pointer_width = "64")]). 32-bit always falls through to the dylib.Layout
BIGNUM_EMBED_FLAG = RUBY_FL_USER2— digits are inas_.ary(embedded)BIGNUM_EMBED_LEN_MASK = USER3|USER4|USER5, shift =RUBY_FL_USHIFT + 3 = 15BIGNUM_POSITIVE_P=flags & RUBY_FL_USER1BIGNUM_EMBED_LEN_MAX = 2on 64-bit (=sizeof(u64)/sizeof(u32))Fits-in-i64 check (64-bit, BDIGIT=u32)
Asm: fully inlined; fixnum path unchanged, Bignum fast path adds ~10 instructions before the dylib fallback.
Parity tests added (10 new)
2**32(1 BDIGIT, positive)2**62(2 BDIGITs, fits)-(2**32)(negative, 1 BDIGIT)-(2**62)(negative, 2 BDIGITs)i64::MAX,i64::MIN189/189 tests pass.
Stack
num2dblT_FLOAT fast pathdbl2numflonum encode fast pathrhash_sizeAR/ST inline ⬅ basenum2long/num2ulongBignum fast path🤖 Generated with Claude Code