Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions changelog.d/index-store-skips-a-dead-string-addref.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
An indexed array store no longer emits `js_string_addref_if_heap_string` for a
value that provably cannot be a heap string.

The plain slot-store emitters pass `layout_note_needed` for *both* the layout
note and the string-addref demote, so any store that needs a layout note also
paid the addref call — including `sieve[j] = false`, where the value is a
boolean. The flagged emitter that separates the two already existed and
`array_push` already used it; this adds the scalar-aware twin and threads
`store_needs_string_addref` (the same predicate the push path trusts) from the
one caller that has the value expression in hand.

`benchmarks/suite/11_prime_sieve.ts` goes from 8 emitted addref calls to 1.

Measured on an idle Mac mini, both binaries built in one run, interleaved, min of
five, self-timed: a boolean-store loop 215 → 207 ms, and `11_prime_sieve` 28 → 26
ms. Roughly 4% each — small, and worth saying plainly that it is not where either
benchmark's gap lives: the same measurement puts Node at 12 ms on that
boolean-store loop against perry's 207, so the per-store typed-feedback guard
**call** is the cost that matters there. This change removes provably dead work
beside it.

Verified against Node on a differential aimed at the exact hazard the addref
exists to prevent — a refcount-1 string stored into a slot and then mutated
through the source local (directly, and through a loop-written array), booleans /
numbers / `null` / `undefined` stored into an array, the sieve shape itself, and a
string slot overwritten by a boolean and back. Byte-identical. 31 `perry-codegen`
suites pass.
17 changes: 13 additions & 4 deletions crates/perry-codegen/src/expr/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
use anyhow::{anyhow, Result};

use super::{
emit_array_numeric_write_note_on_block, emit_jsvalue_slot_store_on_block,
emit_jsvalue_slot_store_scalar_aware_on_block, emit_write_barrier_slot_on_block,
emit_array_numeric_write_note_on_block,
emit_jsvalue_slot_store_scalar_aware_with_flags_on_block,
emit_jsvalue_slot_store_with_flags_on_block, emit_write_barrier_slot_on_block,
emit_write_barrier_slot_value_and_generation_tested, nanbox_pointer_inline,
raw_f64_layout_fact, FnCtx,
};
Expand Down Expand Up @@ -84,6 +85,12 @@ pub(crate) fn lower_index_set_fast(
val_double: &str,
local_id: u32,
layout_note_needed: bool,
// #9186: whether the stored value can be a heap string, decided by the
// caller with `store_needs_string_addref`. The plain slot-store emitters
// tie the addref demote to `layout_note_needed`, so a boolean or `null`
// written into an array that needs a layout note paid an
// `js_string_addref_if_heap_string` call it can never use.
string_addref_needed: bool,
write_barrier_needed: bool,
value_is_numeric: bool,
require_numeric_layout: bool,
Expand Down Expand Up @@ -419,12 +426,13 @@ pub(crate) fn lower_index_set_fast(
// array element: the slot holds a valid value, so the scalar-aware
// note skips the GC layout hashmap on scalar-over-scalar stores
// (#5094 — ~9× on bench_numeric_array_downgrade).
let value_bits = emit_jsvalue_slot_store_scalar_aware_on_block(
let value_bits = emit_jsvalue_slot_store_scalar_aware_with_flags_on_block(
blk,
&element_ptr,
val_double,
&arr_handle,
&idx_i32,
string_addref_needed,
layout_note_needed,
&arr_handle,
&element_addr,
Expand Down Expand Up @@ -634,12 +642,13 @@ pub(crate) fn lower_index_set_fast(
let blk = ctx.block();
let (element_addr, element_ptr) = element_slot(blk, &arr_handle, &idx_i32);
{
let value_bits = emit_jsvalue_slot_store_on_block(
let value_bits = emit_jsvalue_slot_store_with_flags_on_block(
blk,
&element_ptr,
val_double,
&arr_handle,
&idx_i32,
string_addref_needed,
layout_note_needed,
&arr_handle,
&element_addr,
Expand Down
3 changes: 3 additions & 0 deletions crates/perry-codegen/src/expr/index_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1166,13 +1166,16 @@ pub(crate) fn lower(
if ctx.locals.contains_key(&id) {
let value_is_canonical_raw_f64 =
crate::type_analysis::expr_produces_canonical_raw_f64(ctx, value);
let string_addref_needed =
crate::expr::store_needs_string_addref(ctx, value);
lower_index_set_fast(
ctx,
&arr_box,
&idx_double,
&val_double,
id,
layout_note_needed,
string_addref_needed,
write_barrier_needed,
value_is_numeric,
require_numeric_layout,
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ pub(crate) use v8_interop::{
pub(crate) use write_barrier::{
emit_array_numeric_write_note_on_block, emit_jsvalue_slot_store_on_block,
emit_jsvalue_slot_store_pointer_tested, emit_jsvalue_slot_store_scalar_aware_on_block,
emit_jsvalue_slot_store_scalar_aware_with_flags_on_block,
emit_jsvalue_slot_store_with_flags_on_block, emit_jsvalue_slot_store_with_value_bits_on_block,
emit_layout_note_slot_on_block, emit_may_carry_heap_pointer_check,
emit_root_heap_word_store_on_block, emit_root_nanbox_store_on_block, emit_write_barrier,
Expand Down
38 changes: 38 additions & 0 deletions crates/perry-codegen/src/expr/write_barrier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,44 @@ pub(crate) fn emit_jsvalue_slot_store_with_value_bits_on_block(
/// This is the dominant per-write cost on downgraded `any[]` numeric loops
/// (#5094) and gives ~9× on `bench_numeric_array_downgrade` without regressing
/// `bench_object_property`.
/// As [`emit_jsvalue_slot_store_scalar_aware_on_block`], but with the
/// string-addref demote gated independently of the layout note — the
/// scalar-aware twin of [`emit_jsvalue_slot_store_with_flags_on_block`].
///
/// The plain entry point below ties the two together, which costs an
/// unconditional `js_string_addref_if_heap_string` call on every store that
/// needs a layout note but writes a value that provably is not a heap string:
/// `sieve[j] = false` in `benchmarks/suite/11_prime_sieve.ts` pays one per
/// element for a boolean.
#[allow(clippy::too_many_arguments)]
pub(crate) fn emit_jsvalue_slot_store_scalar_aware_with_flags_on_block(
blk: &mut LlBlock,
slot_ptr: &str,
value_double: &str,
layout_parent_bits: &str,
slot_index: &str,
string_addref_needed: bool,
layout_note_needed: bool,
barrier_parent_bits: &str,
slot_addr: &str,
write_barrier_needed: bool,
) -> Option<String> {
emit_jsvalue_slot_store_on_block_inner(
blk,
slot_ptr,
value_double,
layout_parent_bits,
slot_index,
string_addref_needed,
layout_note_needed,
barrier_parent_bits,
slot_addr,
write_barrier_needed,
true,
None,
)
}

pub(crate) fn emit_jsvalue_slot_store_scalar_aware_on_block(
blk: &mut LlBlock,
slot_ptr: &str,
Expand Down
Loading