From ad499109ee27aebd0fe2cda681f32e9b4b55c98a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 20 Aug 2026 17:20:44 +0200 Subject: [PATCH 1/2] perf(string): bypass empty accumulator copies --- .../src/codegen/declared_string_add_tests.rs | 12 ++--- .../perry-codegen/src/lower_string_concat.rs | 18 ++++--- .../src/runtime_decls/strings.rs | 1 + crates/perry-runtime/src/string/append.rs | 50 +++++++++++++++++-- crates/perry-runtime/src/string/mod.rs | 2 +- crates/perry-runtime/src/string/tests.rs | 38 ++++++++++++++ 6 files changed, 103 insertions(+), 18 deletions(-) diff --git a/crates/perry-codegen/src/codegen/declared_string_add_tests.rs b/crates/perry-codegen/src/codegen/declared_string_add_tests.rs index 65ba022b37..25eaf4ddff 100644 --- a/crates/perry-codegen/src/codegen/declared_string_add_tests.rs +++ b/crates/perry-codegen/src/codegen/declared_string_add_tests.rs @@ -359,7 +359,7 @@ fn a_self_append_chain_retains_the_accumulator_and_fuses_only_the_suffix() { let ir = function_ir(module); assert!( - ir.contains("call i64 @js_string_append("), + ir.contains("call i64 @js_string_append_known_heap("), "the growing prefix must reach the amortized append path:\n{ir}" ); assert_eq!( @@ -423,7 +423,7 @@ fn a_self_append_chain_keeps_an_opaque_numeric_head_pair_intact() { let ir = function_ir(module); assert!( - !ir.contains("call i64 @js_string_append("), + !ir.contains("call i64 @js_string_append_known_heap("), "an opaque numeric-capable head pair must remain in source-tree order:\n{ir}" ); } @@ -455,7 +455,7 @@ fn a_module_global_self_append_uses_the_amortized_path_and_demotes_extractions() let ir = function_ir(module); assert!( - ir.contains("call i64 @js_string_append("), + ir.contains("call i64 @js_string_append_known_heap("), "a module root is binding storage and can retain the unique string owner:\n{ir}" ); assert!( @@ -512,7 +512,7 @@ fn a_boxed_local_self_append_uses_the_amortized_path() { let ir = function_ir(module); assert!( - ir.contains("call i64 @js_string_append("), + ir.contains("call i64 @js_string_append_known_heap("), "a variable box must retain the accumulator owner across iterations:\n{ir}" ); assert!( @@ -574,7 +574,7 @@ fn a_boxed_capture_self_append_uses_the_amortized_path() { let ir = module_ir(module); assert!( - ir.contains("call i64 @js_string_append("), + ir.contains("call i64 @js_string_append_known_heap("), "a captured variable box must reach the append helper:\n{ir}" ); assert!( @@ -608,7 +608,7 @@ fn a_module_global_numeric_capable_head_pair_does_not_select_append() { let ir = function_ir(module); assert!( - !ir.contains("call i64 @js_string_append("), + !ir.contains("call i64 @js_string_append_known_heap("), "the newly eligible storage must not weaken the numeric-head guard:\n{ir}" ); } diff --git a/crates/perry-codegen/src/lower_string_concat.rs b/crates/perry-codegen/src/lower_string_concat.rs index ba2ab30d3e..bcadabcfa9 100644 --- a/crates/perry-codegen/src/lower_string_concat.rs +++ b/crates/perry-codegen/src/lower_string_concat.rs @@ -309,9 +309,11 @@ fn lower_tag_dispatched_str_self_append( }; let bits_d_after = ctx.block().bitcast_double_to_i64(&lhs_after_coercion); let h_d = ctx.block().and(I64, &bits_d_after, POINTER_MASK_I64); - let h_new = ctx - .block() - .call(I64, "js_string_append", &[(I64, &h_d), (I64, &r_handle)]); + let h_new = ctx.block().call( + I64, + "js_string_append_known_heap", + &[(I64, &h_d), (I64, &r_handle)], + ); let box_append = nanbox_string_inline(ctx.block(), &h_new); let append_pred = ctx.block().label.clone(); ctx.block().br(&merge_label); @@ -386,9 +388,11 @@ fn lower_tag_dispatched_str_self_append( ctx.current_block = heap_idx; let h_d = ctx.block().and(I64, &bits_d, POINTER_MASK_I64); let h_r = ctx.block().and(I64, &bits_r, POINTER_MASK_I64); - let h_new = ctx - .block() - .call(I64, "js_string_append", &[(I64, &h_d), (I64, &h_r)]); + let h_new = ctx.block().call( + I64, + "js_string_append_known_heap", + &[(I64, &h_d), (I64, &h_r)], + ); let box_heap = nanbox_string_inline(ctx.block(), &h_new); let heap_pred = ctx.block().label.clone(); ctx.block().br(&merge_label); @@ -408,7 +412,7 @@ fn lower_tag_dispatched_str_self_append( let h_d_after = ctx.block().and(I64, &bits_d_after, POINTER_MASK_I64); let h_sso = ctx.block().call( I64, - "js_string_append", + "js_string_append_known_heap", &[(I64, &h_d_after), (I64, &r_handle)], ); let box_sso = nanbox_string_inline(ctx.block(), &h_sso); diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 8798e21e08..241af90223 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -54,6 +54,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { // Either way the caller must use the returned pointer. // (`crates/perry-runtime/src/string.rs:88`) module.declare_function("js_string_append", I64, &[I64, I64]); + module.declare_function("js_string_append_known_heap", I64, &[I64, I64]); // String methods (Phase B.12). // All take/return raw i64 string handles. Length args are i32. diff --git a/crates/perry-runtime/src/string/append.rs b/crates/perry-runtime/src/string/append.rs index c0cde49f23..75d3e36787 100644 --- a/crates/perry-runtime/src/string/append.rs +++ b/crates/perry-runtime/src/string/append.rs @@ -69,6 +69,23 @@ pub extern "C" fn js_string_append( return js_string_concat(dest as *const StringHeader, src); } + // These identity cases cannot allocate or collect, so handle them before + // opening a runtime handle scope. In particular, short-lived accumulators + // commonly append exactly one freshly-concatenated (shared) suffix to the + // shared empty string; rooting both operands just to return `src` was most + // of #8486's residual overhead. + unsafe { + if (*src).byte_len == 0 { + return dest; + } + // Do not reuse a uniquely-owned source: storing it into `dest` would + // create an unrecorded alias while leaving refcount=1, allowing a + // later append through either binding to mutate the other. + if (*dest).byte_len == 0 && (*src).refcount == 0 { + return src as *mut StringHeader; + } + } + let scope = crate::gc::RuntimeHandleScope::new(); let dest_handle = scope.root_string_ptr(dest as *const StringHeader); let src_handle = scope.root_string_ptr(src); @@ -77,10 +94,6 @@ pub extern "C" fn js_string_append( let dest_blen = (*dest).byte_len; let src_blen = (*src).byte_len; - if src_blen == 0 { - return dest; - } - let new_blen = dest_blen + src_blen; // A high→low surrogate pair can only newly form at the dest|src join @@ -163,3 +176,32 @@ pub extern "C" fn js_string_append( } } } + +/// Append entry point for codegen paths that already tag-checked both handles +/// as heap strings. It keeps the defensive helper as the sole implementation +/// for every mutating or allocating case, but lets the common non-allocating +/// identities avoid repeating generic raw-pointer validation (#8486). +/// +/// # Safety contract +/// +/// Both pointers must be valid `StringHeader` handles. Callers may establish +/// this only from a live `STRING_TAG` value (or a runtime string coercion), and +/// must preserve the usual GC rooting requirements across operand evaluation. +#[no_mangle] +pub extern "C" fn js_string_append_known_heap( + dest: *mut StringHeader, + src: *const StringHeader, +) -> *mut StringHeader { + if std::ptr::eq(dest, src) { + return js_string_append(dest, src); + } + unsafe { + if (*src).byte_len == 0 { + return dest; + } + if (*dest).byte_len == 0 && (*src).refcount == 0 { + return src as *mut StringHeader; + } + } + js_string_append(dest, src) +} diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index a19e56285e..0d3d933949 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -134,7 +134,7 @@ pub use alloc::{ js_string_from_bytes_longlived, js_string_from_bytes_with_capacity, js_string_from_wtf8_bytes, js_string_length, js_string_materialize_to_heap, js_string_new_sso, }; -pub use append::js_string_append; +pub use append::{js_string_append, js_string_append_known_heap}; pub use base64_codec::{js_atob, js_btoa}; pub use char_ops::{ js_string_at, js_string_char_at, js_string_char_code_at, js_string_code_point_at, diff --git a/crates/perry-runtime/src/string/tests.rs b/crates/perry-runtime/src/string/tests.rs index 2030f7d9c1..6c7ea4f3ec 100644 --- a/crates/perry-runtime/src/string/tests.rs +++ b/crates/perry-runtime/src/string/tests.rs @@ -468,6 +468,44 @@ fn test_string_append_shared_no_inplace() { assert_eq!(string_as_str(result), "hello "); // Original unchanged } +#[test] +fn test_string_append_empty_reuses_only_shared_source() { + let empty = js_string_from_bytes(b"".as_ptr(), 0); + let shared = js_string_from_bytes(b"suffix".as_ptr(), 6); + assert_eq!(unsafe { (*shared).refcount }, 0); + + let reused = js_string_append(empty, shared); + assert_eq!(reused, shared); + assert_eq!(unsafe { (*reused).refcount }, 0); + + // A later append must allocate instead of changing the aliased source. + let bang = js_string_from_bytes(b"!".as_ptr(), 1); + let grown = js_string_append(reused, bang); + assert_ne!(grown, shared); + assert_eq!(string_as_str(shared), "suffix"); + assert_eq!(string_as_str(grown), "suffix!"); + + // A unique source cannot be reused: doing so would silently create a + // second owner while leaving its in-place mutation permission intact. + let a = js_string_from_bytes(b"a".as_ptr(), 1); + let b = js_string_from_bytes(b"b".as_ptr(), 1); + let unique = js_string_append(a, b); + assert_eq!(unsafe { (*unique).refcount }, 1); + let empty2 = js_string_from_bytes(b"".as_ptr(), 0); + let copied = js_string_append(empty2, unique); + assert_ne!(copied, unique); + assert_eq!(string_as_str(copied), "ab"); + assert_eq!(string_as_str(unique), "ab"); + + // The tag-checked codegen entry point applies the same ownership rule. + let empty3 = js_string_from_bytes(b"".as_ptr(), 0); + assert_eq!(js_string_append_known_heap(empty3, shared), shared); + let empty4 = js_string_from_bytes(b"".as_ptr(), 0); + let copied_known = js_string_append_known_heap(empty4, unique); + assert_ne!(copied_known, unique); + assert_eq!(string_as_str(copied_known), "ab"); +} + #[test] fn test_string_append_self() { // Self-append (s += s) must always allocate fresh From b21971f7e493af713f11912be4717c9cc2cb7e60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 20 Aug 2026 17:21:40 +0200 Subject: [PATCH 2/2] docs: record empty accumulator fast path --- changelog.d/8487-empty-accumulator-identity.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 changelog.d/8487-empty-accumulator-identity.md diff --git a/changelog.d/8487-empty-accumulator-identity.md b/changelog.d/8487-empty-accumulator-identity.md new file mode 100644 index 0000000000..ff5d8d1224 --- /dev/null +++ b/changelog.d/8487-empty-accumulator-identity.md @@ -0,0 +1,13 @@ +Avoid allocating and copying a completed concat suffix when a string builder +appends it to the shared empty string. The optimization reuses only shared +sources, so a uniquely owned source can never gain an untracked alias with +in-place mutation permission. Codegen's tag-checked heap-string append arms use +the same identity path without repeating generic pointer validation; all other +cases retain the existing `js_string_append` behavior. + +On `iso_miss`, median instructions retired fall from 14.790 G to 12.471 G +(-15.7%), recovering about 70% of #8417's measured regression while keeping +#8394's accumulator-chain fix. The #8394 fixture stays flat at 27.0 M +instructions, and its 2k/4k/8k/16k scaling probe remains 0-1 ms rather than +returning to the pre-#8417 quadratic 17/56/342/1450 ms curve. Peak RSS is flat +for both workloads, and all 19 sweep programs remain byte-exact.