From 21fdad139216ea603ee303e22a208670b55c1720 Mon Sep 17 00:00:00 2001 From: "David R. MacIver" Date: Sat, 30 May 2026 11:34:19 +0100 Subject: [PATCH 1/5] Optimise Repr::clone for the inline case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The callgrind profile of hegel-rust's shrinker (per the hegel_shrinker bench file's comments) reports Repr::clone at 13.24 % of total Ir, with values almost exclusively in the i64 / i128 range — i.e. the inline variant of Repr (capacity in {-2,-1,1,2}). The old implementation went through sign_capacity to extract the absolute capacity + Sign, then constructed a positive Repr, then re-applied the sign via with_sign. That's a redundant round-trip when all we need to do for the inline case is copy 3 machine words verbatim. This patch: * marks Clone::clone #[inline] so the trivial inline path lands at every call site without a function call; * inline path is now a direct struct copy: the union's field plus the original signed ; * heap path is split into a #[cold] + #[inline(never)] helper so the inline fast path stays tiny everywhere it's instantiated. (Without the explicit #[inline(never)] LLVM happily inlines it back and the heap case actually gets slightly slower in benches — apparently the inlined-in-clone shape compiles worse than going through a normal function call. Tracked the experiment in the commit log.) Results against the new integer/benches/hegel_shrinker baseline: shrinker_consider/64 -29.1 % shrinker_consider/16 -25.5 % shrinker_consider/4 -12.7 % ibig_clamp/two_word -42.6 % (clamp uses clone heavily) ibig_clamp/one_word -40.2 % ibig_drop/two_word -35.3 % (incidental: tighter assignment ibig_drop/one_word -34.6 % pattern in the surrounding bench ibig_drop/zero -34.2 % loop exposes a folding win) ibig_clone/two_word -21.4 % ibig_clone/one_word -20.3 % ibig_clone/zero -20.0 % ibig_clone/just_over_inline +7.7 % (heap path: extra function call) ibig_clone/mid +12.7 % ibig_clone/large ~0 % (memcpy-bound, call overhead lost in the noise) The heap regression is the deliberate trade-off: hegel's shrinker uses ≤128-bit values almost exclusively, so the inline win dwarfs it. Any downstream consumer that clones large IBig values heavily would hit the +13 % on mid-sized values; if that ever shows up as a hot spot we can revisit the inline/outline split. --- integer/src/repr.rs | 75 +++++++++++++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/integer/src/repr.rs b/integer/src/repr.rs index c92a8a69..a5991b18 100644 --- a/integer/src/repr.rs +++ b/integer/src/repr.rs @@ -443,6 +443,34 @@ impl Repr { self } + /// Out-of-line slow path for `Clone::clone`. Heap-resident values only + /// (capacity > 2 in absolute value). Kept as a separate `#[cold]` + + /// `#[inline(never)]` function so the inline fast path stays tiny at + /// every `Clone::clone` call site — without `#[inline(never)]` LLVM + /// chooses to inline this back into `Clone::clone`, which both bloats + /// the inline path and (empirically) emits noticeably slower code for + /// the heap case itself than going through a regular function call. + #[cold] + #[inline(never)] + fn clone_heap(&self) -> Self { + debug_assert!(self.capacity.get().unsigned_abs() > 2); + // SAFETY: abs(capacity) > 2 ⇒ the `heap` variant of the union is + // the live one. + unsafe { + let (ptr, len) = self.data.heap; + debug_assert!(len >= 3); + let mut new_buffer = Buffer::allocate(len); + new_buffer.push_slice(slice::from_raw_parts(ptr, len)); + // `Buffer::allocate(len)` gives a positive capacity; preserve + // self's sign by negating if necessary. + let mut new: Repr = mem::transmute(new_buffer); + if self.capacity.get() < 0 { + new.capacity = NonZeroIsize::new_unchecked(-new.capacity.get()); + } + new + } + } + /// Returns a number representing sign of self. /// /// * [Self::zero] if the number is zero @@ -461,33 +489,28 @@ impl Repr { // Cloning for Repr is written in a verbose way because it's performance critical. impl Clone for Repr { + #[inline] fn clone(&self) -> Self { - let (capacity, sign) = self.sign_capacity(); - - // SAFETY: see the comments inside the block - let new = unsafe { - // inline the data if the length is less than 3 - // SAFETY: we check the capacity before accessing the variants - if capacity <= 2 { - Repr { - data: ReprData { - inline: self.data.inline, - }, - // SAFETY: the capacity is from self, which guarantees it to be zero - capacity: NonZeroIsize::new_unchecked(capacity as isize), - } - } else { - let (ptr, len) = self.data.heap; - // SAFETY: len is at least 2 when it's heap allocated (invariant of Repr) - let mut new_buffer = Buffer::allocate(len); - new_buffer.push_slice(slice::from_raw_parts(ptr, len)); - - // SAFETY: abs(self.capacity) >= 3 => self.data.len >= 3 - // so the capacity and len of new_buffer will be both >= 3 - mem::transmute::(new_buffer) - } - }; - new.with_sign(sign) + // Inline case (abs(capacity) <= 2): just copy the union and the + // signed capacity verbatim. Avoids the `sign_capacity` extract + + // `with_sign` re-apply round-trip the old implementation did. + // + // Profiled hegel-rust shrinker workloads spend ~13 % of total Ir + // in this function, almost entirely on the inline case (values + // are i64- or i128-sized). Inlining the trivial path here lets + // it disappear into the caller; the heavier heap path is split + // out into a `#[cold]` helper. + if self.capacity.get().unsigned_abs() <= 2 { + return Repr { + data: ReprData { + // SAFETY: capacity in {-2,-1,1,2} ⇒ the `inline` + // variant of the union is the live one. + inline: unsafe { self.data.inline }, + }, + capacity: self.capacity, + }; + } + self.clone_heap() } fn clone_from(&mut self, src: &Self) { From 6951b5b2022feb3579cf4d289cba75e835679b9d Mon Sep 17 00:00:00 2001 From: "David R. MacIver" Date: Sat, 30 May 2026 11:37:33 +0100 Subject: [PATCH 2/5] Inline Repr::clone_from and tighten its fast path Same shape as the clone() optimisation: marks clone_from #[inline] and rewrites the inline-source fast path to avoid the sign_capacity unpack (it only needs to know abs(capacity) <= 2 to decide which path to take, and can copy the signed capacity verbatim). The sign-aware extraction is still done in the heap path for the reallocation-and-copy logic. Not shown in the hegel_shrinker benches because Vec::clone() goes through T::clone() per element, not T::clone_from(); the bench file does not have an explicit clone_from-driven scenario. The change is for symmetry / downstream consumers that hit *dest = src.clone() patterns the compiler rewrites to clone_from. Tests pass (lib + hegel-tests stateful). --- integer/src/repr.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/integer/src/repr.rs b/integer/src/repr.rs index a5991b18..e7b6e1bf 100644 --- a/integer/src/repr.rs +++ b/integer/src/repr.rs @@ -513,14 +513,19 @@ impl Clone for Repr { self.clone_heap() } + #[inline] fn clone_from(&mut self, src: &Self) { - let (src_cap, src_sign) = src.sign_capacity(); - let (cap, _) = self.sign_capacity(); + let src_cap_raw = src.capacity.get(); + let cap = self.capacity.get().unsigned_abs(); // SAFETY: see the comments inside the block unsafe { - // shortcut for inlined data - if src_cap <= 2 { + // Fast path: src is inline. Same shape as `clone`'s fast path + // — copy union + signed capacity, with a rare dealloc when + // self was previously heap-resident. The shrinker's + // ChoiceNode = (IBig, IBig, IBig, IBig) clone pattern is + // dominated by inline-source clones. + if src_cap_raw.unsigned_abs() <= 2 { if cap > 2 { // release the old buffer if necessary // SAFETY: self.data.heap.0 must be valid pointer if cap > 2 @@ -530,6 +535,7 @@ impl Clone for Repr { self.capacity = src.capacity; return; } + let src_sign = if src_cap_raw > 0 { Sign::Positive } else { Sign::Negative }; // SAFETY: we checked that abs(src.capacity) > 2 let (src_ptr, src_len) = src.data.heap; From 314b426388c9db23edc84fa9d1eba9f1d5dd6481 Mon Sep 17 00:00:00 2001 From: "David R. MacIver" Date: Sat, 30 May 2026 11:51:24 +0100 Subject: [PATCH 3/5] Optimise heap clone_heap path; drop the cold annotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to Repr::clone_heap, both motivated by hegel_shrinker bench data showing the just_over_inline / mid heap clones regressed +8 to +13% vs the prior baseline (commit 583ee7e). 1. Skip the Buffer scaffolding and allocate directly. The old body went through Buffer::allocate (which calls allocate_exact which calls allocate_raw, with two near-duplicate size-bound asserts along the way), then push_slice (one more capacity-vs-len assert), then mem::transmute, then a conditional sign flip on the resulting Repr. The new body computes default_capacity inline, calls alloc::alloc::alloc once via from_size_align_unchecked, ptr::copy_n's, and constructs the Repr with the signed capacity set from the start. That eliminates the redundant checks plus a __rust_no_alloc_shim_is_unstable_v2 bl that the Buffer wrapper was pulling in. 2. Drop the #[cold] annotation. The shrinker_consider/64 workload bench (the most realistic top-level scenario from hegel_shrinker.rs) does ~3 pp better without #[cold]. The isolated ibig_clone microbench does ~3 pp worse — i.e. the trade-off is in code-layout, not in clone work itself. The realistic compound workload is what matters. Bench impact vs the 'current' baseline (saved before any Repr::clone changes — so this is the cumulative result of both this commit and the prior fast-path inline): ibig_clone/zero -16.6 % (vs -20.0 % before) ibig_clone/one_word -15.2 % ibig_clone/two_word -17.9 % ibig_clone/just_over_inline +2.0 % (was +7.7 %) ibig_clone/mid +0.7 % (was +12.7 %) ibig_clone/large -5.1 % (was ~0 %) shrinker_consider/64 -32.3 % (was -29.1 %) shrinker_consider/16 -27.4 % shrinker_consider/4 -13.5 % The heap regression is essentially gone: just_over_inline and mid are in the noise, large is actively faster than the original code by 5 %. The inline microbench gave up ~3 pp vs the prior commit, but that's a code-layout side effect of dropping #[cold] and is more than compensated for in the realistic shrinker workload. --- integer/src/repr.rs | 82 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 19 deletions(-) diff --git a/integer/src/repr.rs b/integer/src/repr.rs index e7b6e1bf..8353ffa4 100644 --- a/integer/src/repr.rs +++ b/integer/src/repr.rs @@ -443,31 +443,75 @@ impl Repr { self } - /// Out-of-line slow path for `Clone::clone`. Heap-resident values only - /// (capacity > 2 in absolute value). Kept as a separate `#[cold]` + - /// `#[inline(never)]` function so the inline fast path stays tiny at - /// every `Clone::clone` call site — without `#[inline(never)]` LLVM - /// chooses to inline this back into `Clone::clone`, which both bloats - /// the inline path and (empirically) emits noticeably slower code for - /// the heap case itself than going through a regular function call. - #[cold] + /// Slow path for `Clone::clone`. Heap-resident values only + /// (capacity > 2 in absolute value). + /// + /// Allocates directly via `alloc::alloc::alloc` rather than going + /// through `Buffer::allocate` + `push_slice` + `mem::transmute`. + /// That bypass saves: + /// + /// * the redundant `0 < capacity <= MAX_CAPACITY` assertion inside + /// `Buffer::allocate_raw` (we just computed `new_cap` from a + /// bounded `len`); + /// * `push_slice`'s `len <= capacity - 0` check (we own the buffer + /// we just allocated, the only `len` ever written is the input + /// `len`); + /// * `Buffer::allocate_exact`'s second check on the same bound; + /// * the `mem::transmute(Buffer) -> Repr` shape and the conditional + /// sign-flip dance — the signed capacity is set directly. + /// + /// `#[inline(never)]` (without `#[cold]`) keeps the inline fast path + /// tiny at every `Clone::clone` instantiation and gives LLVM a stable + /// call boundary so the inline path's code-layout doesn't depend on + /// how aggressively the heap path gets considered for inlining. + /// + /// Removing the attribute lets LLVM occasionally inline this back — + /// which leaves the inline microbench unchanged but regresses + /// `ibig_clone/large` by ~5 pp (probably i-cache: the inlined + /// 30-instruction heap body lands inside the bench's hot loop). + /// `#[cold]` was also dropped (`shrinker_consider` gains ~3 pp without + /// it; see commit notes). #[inline(never)] fn clone_heap(&self) -> Self { debug_assert!(self.capacity.get().unsigned_abs() > 2); - // SAFETY: abs(capacity) > 2 ⇒ the `heap` variant of the union is - // the live one. + // SAFETY: abs(capacity) > 2 ⇒ the `heap` variant of the union + // is the live one, and `len >= 3`. unsafe { - let (ptr, len) = self.data.heap; + let (src_ptr, len) = self.data.heap; debug_assert!(len >= 3); - let mut new_buffer = Buffer::allocate(len); - new_buffer.push_slice(slice::from_raw_parts(ptr, len)); - // `Buffer::allocate(len)` gives a positive capacity; preserve - // self's sign by negating if necessary. - let mut new: Repr = mem::transmute(new_buffer); - if self.capacity.get() < 0 { - new.capacity = NonZeroIsize::new_unchecked(-new.capacity.get()); + + // Mirrors `Buffer::default_capacity(len)`. `len <= + // Buffer::MAX_CAPACITY` is invariant for any heap Repr, and + // `default_capacity` is monotone, so `new_cap` is bounded + // and the bound check inside `Buffer::allocate_raw` would + // never fire. + let new_cap = len + len / 8 + 2; + debug_assert!(new_cap >= 3 && new_cap <= Buffer::MAX_CAPACITY); + + let layout = core::alloc::Layout::from_size_align_unchecked( + new_cap * mem::size_of::(), + mem::align_of::(), + ); + let new_ptr = alloc::alloc::alloc(layout) as *mut Word; + if new_ptr.is_null() { + crate::error::panic_out_of_memory(); + } + + ptr::copy_nonoverlapping(src_ptr, new_ptr, len); + + // Set the result's signed capacity in one shot so we never + // build a positive Repr only to negate it back. + let signed_cap = if self.capacity.get() < 0 { + -(new_cap as isize) + } else { + new_cap as isize + }; + Repr { + data: ReprData { + heap: (new_ptr, len), + }, + capacity: NonZeroIsize::new_unchecked(signed_cap), } - new } } From 402c867b6f6c64c163356ce62cafa657cb2075e6 Mon Sep 17 00:00:00 2001 From: "David R. MacIver" Date: Tue, 2 Jun 2026 07:48:33 +0100 Subject: [PATCH 4/5] Add clone regression test; satisfy upstream rustfmt and clippy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds tests/clone.rs covering clone and clone_from across every inline/heap (dst, src) combination — including the heap-dst + inline-src case that frees the old buffer, and inline-dst + heap-src that allocates. Also fixes two lints the cherry-picked clone code tripped under upstream CI: the manual range check (clippy::manual_range_contains) and the if-else formatting in clone_from. Co-Authored-By: Claude Opus 4.8 (1M context) --- integer/src/repr.rs | 8 +++++-- integer/tests/clone.rs | 53 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 integer/tests/clone.rs diff --git a/integer/src/repr.rs b/integer/src/repr.rs index 8353ffa4..a1db7495 100644 --- a/integer/src/repr.rs +++ b/integer/src/repr.rs @@ -486,7 +486,7 @@ impl Repr { // and the bound check inside `Buffer::allocate_raw` would // never fire. let new_cap = len + len / 8 + 2; - debug_assert!(new_cap >= 3 && new_cap <= Buffer::MAX_CAPACITY); + debug_assert!((3..=Buffer::MAX_CAPACITY).contains(&new_cap)); let layout = core::alloc::Layout::from_size_align_unchecked( new_cap * mem::size_of::(), @@ -579,7 +579,11 @@ impl Clone for Repr { self.capacity = src.capacity; return; } - let src_sign = if src_cap_raw > 0 { Sign::Positive } else { Sign::Negative }; + let src_sign = if src_cap_raw > 0 { + Sign::Positive + } else { + Sign::Negative + }; // SAFETY: we checked that abs(src.capacity) > 2 let (src_ptr, src_len) = src.data.heap; diff --git a/integer/tests/clone.rs b/integer/tests/clone.rs new file mode 100644 index 00000000..e1d5cb5d --- /dev/null +++ b/integer/tests/clone.rs @@ -0,0 +1,53 @@ +mod helper_macros; + +#[test] +fn test_clone_inline_and_heap() { + // value (and sign) preserved across clone, for inline and heap reprs + let ubigs = [ + ubig!(0), + ubig!(5), + ubig!(0x10000000000000000), // 2^64, two-word inline + ubig!(1) << 200, // heap + (ubig!(1) << 256) - ubig!(1), // heap, all ones + ]; + for v in &ubigs { + assert_eq!(&v.clone(), v); + } + let ibigs = [ + ibig!(0), + ibig!(5), + ibig!(-5), + ibig!(1) << 200, + -(ibig!(1) << 200), + ]; + for v in &ibigs { + assert_eq!(&v.clone(), v); + } +} + +#[test] +fn test_clone_from_transitions() { + // clone_from across every inline/heap combination of (dst, src) — including + // dst heap-allocated with an inline src (dst's buffer must be freed) and + // inline dst with a heap src (must allocate). + let vals = [ + ubig!(0), + ubig!(7), + ubig!(0x10000000000000000), // inline cap 2 + ubig!(1) << 130, // heap + ubig!(1) << 200, // heap, larger + ]; + for src in &vals { + for dst0 in &vals { + let mut dst = dst0.clone(); + dst.clone_from(src); + assert_eq!(&dst, src, "clone_from {dst0} <- {src}"); + } + } + // signed: heap negative -> inline negative -> heap positive + let mut x = -(ibig!(1) << 200); + x.clone_from(&ibig!(-3)); + assert_eq!(x, ibig!(-3)); + x.clone_from(&(ibig!(1) << 200)); + assert_eq!(x, ibig!(1) << 200); +} From 59f63441b9eec83bea6ef73f35720f7cfd1f7373 Mon Sep 17 00:00:00 2001 From: "David R. MacIver" Date: Tue, 2 Jun 2026 10:10:01 +0100 Subject: [PATCH 5/5] Trim verbose and profiling-context comments in clone path --- integer/src/repr.rs | 51 +++++++++------------------------------------ 1 file changed, 10 insertions(+), 41 deletions(-) diff --git a/integer/src/repr.rs b/integer/src/repr.rs index a1db7495..2698e3b5 100644 --- a/integer/src/repr.rs +++ b/integer/src/repr.rs @@ -443,34 +443,12 @@ impl Repr { self } - /// Slow path for `Clone::clone`. Heap-resident values only - /// (capacity > 2 in absolute value). + /// Slow path for `Clone::clone`: heap-resident values only (|capacity| > 2). /// - /// Allocates directly via `alloc::alloc::alloc` rather than going - /// through `Buffer::allocate` + `push_slice` + `mem::transmute`. - /// That bypass saves: - /// - /// * the redundant `0 < capacity <= MAX_CAPACITY` assertion inside - /// `Buffer::allocate_raw` (we just computed `new_cap` from a - /// bounded `len`); - /// * `push_slice`'s `len <= capacity - 0` check (we own the buffer - /// we just allocated, the only `len` ever written is the input - /// `len`); - /// * `Buffer::allocate_exact`'s second check on the same bound; - /// * the `mem::transmute(Buffer) -> Repr` shape and the conditional - /// sign-flip dance — the signed capacity is set directly. - /// - /// `#[inline(never)]` (without `#[cold]`) keeps the inline fast path - /// tiny at every `Clone::clone` instantiation and gives LLVM a stable - /// call boundary so the inline path's code-layout doesn't depend on - /// how aggressively the heap path gets considered for inlining. - /// - /// Removing the attribute lets LLVM occasionally inline this back — - /// which leaves the inline microbench unchanged but regresses - /// `ibig_clone/large` by ~5 pp (probably i-cache: the inlined - /// 30-instruction heap body lands inside the bench's hot loop). - /// `#[cold]` was also dropped (`shrinker_consider` gains ~3 pp without - /// it; see commit notes). + /// Allocates directly instead of going through `Buffer::allocate` + + /// `push_slice` + `mem::transmute`, skipping the redundant capacity/length + /// bound checks and the sign-flip dance. Kept `#[inline(never)]` so the + /// inline fast path in `clone` stays small. #[inline(never)] fn clone_heap(&self) -> Self { debug_assert!(self.capacity.get().unsigned_abs() > 2); @@ -535,15 +513,9 @@ impl Repr { impl Clone for Repr { #[inline] fn clone(&self) -> Self { - // Inline case (abs(capacity) <= 2): just copy the union and the - // signed capacity verbatim. Avoids the `sign_capacity` extract + - // `with_sign` re-apply round-trip the old implementation did. - // - // Profiled hegel-rust shrinker workloads spend ~13 % of total Ir - // in this function, almost entirely on the inline case (values - // are i64- or i128-sized). Inlining the trivial path here lets - // it disappear into the caller; the heavier heap path is split - // out into a `#[cold]` helper. + // Inline case (|capacity| <= 2): copy the union and signed capacity + // verbatim, avoiding the `sign_capacity` + `with_sign` round-trip the + // old implementation did. The heap path is split into `clone_heap`. if self.capacity.get().unsigned_abs() <= 2 { return Repr { data: ReprData { @@ -564,11 +536,8 @@ impl Clone for Repr { // SAFETY: see the comments inside the block unsafe { - // Fast path: src is inline. Same shape as `clone`'s fast path - // — copy union + signed capacity, with a rare dealloc when - // self was previously heap-resident. The shrinker's - // ChoiceNode = (IBig, IBig, IBig, IBig) clone pattern is - // dominated by inline-source clones. + // Fast path: src is inline. Copy union + signed capacity, with a + // rare dealloc when self was previously heap-resident. if src_cap_raw.unsigned_abs() <= 2 { if cap > 2 { // release the old buffer if necessary