Repository navigation
perf: several small improvements - #488
Merged
Merged
Conversation
Add copy_small (inline path, <=24B overlapping-copy ladder) and copy_medium (heap path, [16,64] overlapping-SIMD bands, memcpy fallback otherwise). A runtime-length ptr::copy_nonoverlapping lowers to an out-of-line memcpy call; for these short copies the call boundary costs more than the copy itself.
Drop #[inline] from HeapBuffer::{new,with_capacity,with_additional}. These are
the cold, allocating construction paths; keeping them out-of-line makes them a
single shared function rather than being stamped into every CompactString
construction callsite.
Add HeapBuffer::new_panic (panics on allocation failure) and route Repr::new_panic through it. Since HeapBuffer construction is already out-of-line, the fallible-result check and the cold panic path now live in that shared function instead of being inlined into every callsite. A shared #[inline] build core keeps new/new_panic DRY without either calling the other.
An empty string flows through InlineBuffer::new to a byte-identical EMPTY repr (copy_small(_, _, 0) is a no-op), so the special case only added a branch to the hot construction path for no benefit.
Exercises the medium/large copy path (copy_medium and the memcpy fallback), not just the inline range.
ParkMyCar
enabled auto-merge (squash)
July 12, 2026 20:59
ParkMyCar
added a commit
that referenced
this pull request
Jul 12, 2026
…egression #488 moved heap construction out-of-line via a helper that returned a full HeapBuffer. Returning that 24-byte aggregate across the outlined call boundary forced its result through the stack, which made LLVM route all of Repr::new_panic -- including the hot inline arm -- through a stack temporary and scatter-copy it into the return slot on x86: misaligned 1+8+4+2+1 loads overlapping the 16-byte build stores, a store-to-load-forwarding stall on every construction. This regressed inline creation on x86 (it was only ever wall-clock benched on aarch64, where the reload stays clean). Replace HeapBuffer::new_panic with alloc_copy_panic, which returns the two-word (ptr, capacity) pair -- passed back in registers -- and let Repr::new_panic assemble the HeapBuffer/Repr at the callsite. The inline arm's buffer is now moved with the clean 3x movq (x86) / ldp+ldr (aarch64) again, and the alloc/copy path stays out-of-line (a single call). Verified: lib tests pass; Miri clean across inline/full-24/multibyte/heap lengths plus clone/drop under strict provenance.
ParkMyCar
added a commit
that referenced
this pull request
Jul 12, 2026
…line regression #488 moved heap construction out-of-line via a helper that returned a full HeapBuffer. Returning that 24-byte aggregate across the outlined call boundary forced its result through the stack, which made LLVM route all of Repr::new_panic -- including the hot inline arm -- through a stack temporary and scatter-copy it into the return slot on x86: misaligned 1+8+4+2+1 loads overlapping the 16-byte build stores, a store-to-load-forwarding stall on every construction. This regressed inline creation on x86 (only ever wall-clock benched on aarch64, where the reload stays clean). Split the allocating path into: * HeapBuffer::alloc_copy -- #[cold] #[inline(never)], returns the two-word (ptr, capacity) pair (passed back in registers, not a stack aggregate); * HeapBuffer::alloc_copy_panic -- #[inline(always)], assembles the HeapBuffer from that pair at the callsite and unwraps. Only (ptr, capacity) crosses the cold boundary, so the inline arm keeps its clean 3x movq / ldp+ldr copy, the heap arm is laid out cold (out of the hot path), and the callsite stays the tidy Repr::from_heap(HeapBuffer::alloc_copy_panic(text)). Verified: lib tests pass; Miri clean across inline/full-24/multibyte/heap lengths plus clone/drop under strict provenance.
ParkMyCar
added a commit
that referenced
this pull request
Jul 13, 2026
…86 inline regression) #488 built the heap variant via a helper that returned a full HeapBuffer. Returning that 24-byte aggregate across the outlined call boundary forced its result through the stack, which made LLVM route the whole constructor -- including the hot inline arm -- through a stack temporary and scatter-copy it into the return slot on x86: misaligned 1+8+4+2+1 loads overlapping the 16-byte build stores, a store-to-load forwarding stall on every construction. This regressed inline creation on x86 (only ever wall-clock benched on aarch64, where the reload stays clean). Replace HeapBuffer::build with HeapBuffer::alloc_copy, the single #[cold] #[inline(never)] allocate+copy core that returns the two-word (ptr, capacity) pair (passed back in registers, not a stack aggregate). new and alloc_copy_panic become #[inline(always)] wrappers that assemble the HeapBuffer from that pair at the callsite. So every construction path -- new_panic and the fallible Repr::new behind ToCompactString/From/number formatting -- keeps its clean 3x movq / ldp+ldr copy, lays the heap arm out cold, and only (ptr, capacity) ever crosses the boundary. Verified: lib tests pass; Miri clean across inline/full-24/multibyte/heap lengths plus clone/drop under strict provenance.
ParkMyCar
added a commit
that referenced
this pull request
Jul 13, 2026
* perf: route heap construction through a cold alloc_copy core (fixes x86 inline regression) #488 built the heap variant via a helper that returned a full HeapBuffer. Returning that 24-byte aggregate across the outlined call boundary forced its result through the stack, which made LLVM route the whole constructor -- including the hot inline arm -- through a stack temporary and scatter-copy it into the return slot on x86: misaligned 1+8+4+2+1 loads overlapping the 16-byte build stores, a store-to-load forwarding stall on every construction. This regressed inline creation on x86 (only ever wall-clock benched on aarch64, where the reload stays clean). Replace HeapBuffer::build with HeapBuffer::alloc_copy, the single #[cold] #[inline(never)] allocate+copy core that returns the two-word (ptr, capacity) pair (passed back in registers, not a stack aggregate). new and alloc_copy_panic become #[inline(always)] wrappers that assemble the HeapBuffer from that pair at the callsite. So every construction path -- new_panic and the fallible Repr::new behind ToCompactString/From/number formatting -- keeps its clean 3x movq / ldp+ldr copy, lays the heap arm out cold, and only (ptr, capacity) ever crosses the boundary. Verified: lib tests pass; Miri clean across inline/full-24/multibyte/heap lengths plus clone/drop under strict provenance. * perf: construct the inline buffer in registers on 64-bit little-endian The overlapping-copy ladder's stores straddle 8-byte word boundaries, so whole-word reloads of a freshly built repr fail store-to-load forwarding on x86 (a ~12-cycle stall per blocked load; inline creation measured a flat ~10ns for every length except 24). Build the buffer's three words in registers instead: overlapping loads from the source merged with shifts, so the value can stay in registers entirely. Inline creation drops to ~1.35ns on Sapphire Rapids; aarch64 lowers to ldp/lsr/csel/stp with no stack traffic. copy_small now only backs the fallback path, so gate it. * perf: clone heap strings through new_panic clone_heap was #[inline(never)] and returned the 24-byte Repr by value, forcing an sret return that routed even the plain-copy inline arm through a scatter-copied stack temporary on x86 -- the same shape fixed for construction by alloc_copy. new_panic's heap arm already is that cold out-of-line core, so call it directly. clone small -53%, clone large -32% on Sapphire Rapids. * perf: drop from_string's by-value cold helpers The #[cold] empty()/capacity_on_heap() helpers returned Result<Repr> through an sret slot, inviting the hot buffer-steal arm into a shared stack temporary. Inline the empty case (it is one compare) and route the over-16MB-on-32-bit case through the cold alloc_copy core. * perf: route with_capacity/with_additional through cold register-pair cores Same protocol as alloc_copy: the out-of-line allocating core returns (ptr, capacity) in registers and an #[inline(always)] wrapper packs the HeapBuffer at the callsite, instead of returning the 24-byte buffer by value across the cold call boundary. * perf: build Default::default from const_new const_new("") folds the empty string to a constant instead of running the runtime constructor. * cargo fmt
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.
This PR includes several small perf improvements, aimed at both runtime and instruction count.
fn copy_mediumwhich is a memcpy optimization for small heap strings in the range of[16, 64]. Like the existingcopy_small, the new functions get compiled down to a few optimized instructions which avoids the call to into libc's_memcpy. micro-benchmarks now showCompactStringis faster thanStringfor lengths <= 64 characters. above that the two are equalHeapBufferconstruction to prevent bloating the instruction cache. still an overall walltime improvement with [1]HeapBuffer's error check outline, an extension of commit [2] reallylen == 0special case inRepr::newto reduce instruction bloata. there's a bit of nuance here, in some cases
CompactString::new(<empty string>)is now a tad bit slower, but specificallyCompactString::new("")(with a static""as input) is actually unchanged because LLVM optimizes that to the same empty buffer construction we had before.