From 05769e026b9b1c596bbcb99aadd4d4252ccd1dff Mon Sep 17 00:00:00 2001 From: Andrew Imm Date: Mon, 24 Aug 2026 10:38:26 -0700 Subject: [PATCH 1/4] React Compiler: Make AbstractValue Copy to reduce heap allocation (#37225) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary By replicating the `IndexSet` behavior with a simple inline array, this is able to avoid any heap allocation / memory thrash for `AbstractValue`. It also reduces the size of AbstractValue from 72 bytes + all of the heap allocations, to only 18 bytes. ## How did you test this change? Ran all of the fixtures to confirm byte output is identical. Effects on memory and compile time: | Benchmark | Peak allocation | Allocation count | Wall time | |------------------|-----------------------------|------------------|-----------| | legacy/image.tsx | 58.21 -> 33.40 MiB (-42.6%) | -51.1% | -39.5% | | next-client | 58.21 -> 33.40 (-42.6%) | -40.5% | -25.1% | | devtools | 26.39 -> 16.29 (-38.3%) | -26.7% | -14.8% | | fixtures | 17.35 → 9.41 (-45.8%) | -9.9% | -9.0% | --- .../src/infer_mutation_aliasing_effects.rs | 155 ++++++++++++------ 1 file changed, 109 insertions(+), 46 deletions(-) diff --git a/compiler/crates/react_compiler_inference/src/infer_mutation_aliasing_effects.rs b/compiler/crates/react_compiler_inference/src/infer_mutation_aliasing_effects.rs index 83b06418738..b6d9cab4e39 100644 --- a/compiler/crates/react_compiler_inference/src/infer_mutation_aliasing_effects.rs +++ b/compiler/crates/react_compiler_inference/src/infer_mutation_aliasing_effects.rs @@ -11,7 +11,7 @@ //! creation, aliasing, mutation, freezing, and error conditions for each //! instruction and terminal in the HIR. -use indexmap::{IndexMap, IndexSet}; +use indexmap::IndexMap; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use react_compiler_diagnostics::CompilerDiagnostic; @@ -69,7 +69,7 @@ pub fn infer_mutation_aliasing_effects( value_id, AbstractValue { kind: ValueKind::Context, - reason: hashset_of(ValueReason::Other), + reason: ValueReasonSet::single(ValueReason::Other), }, ); initial_state.define(ctx_place.identifier, value_id); @@ -78,12 +78,12 @@ pub fn infer_mutation_aliasing_effects( let param_kind: AbstractValue = if is_function_expression { AbstractValue { kind: ValueKind::Mutable, - reason: hashset_of(ValueReason::Other), + reason: ValueReasonSet::single(ValueReason::Other), } } else { AbstractValue { kind: ValueKind::Frozen, - reason: hashset_of(ValueReason::ReactiveFunctionArgument), + reason: ValueReasonSet::single(ValueReason::ReactiveFunctionArgument), } }; @@ -103,7 +103,7 @@ pub fn infer_mutation_aliasing_effects( value_id, AbstractValue { kind: ValueKind::Mutable, - reason: hashset_of(ValueReason::Other), + reason: ValueReasonSet::single(ValueReason::Other), }, ); initial_state.define(ref_place.identifier, value_id); @@ -258,16 +258,88 @@ impl ValueId { // AbstractValue // ============================================================================= -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] struct AbstractValue { kind: ValueKind, - reason: IndexSet, + reason: ValueReasonSet, +} + +/// Capacity of [`ValueReasonSet`]. A set holds at most one of each `ValueReason` +/// variant, of which there are currently 12; the extra slots are headroom so +/// that adding variants upstream cannot overflow the set. +const VALUE_REASON_CAPACITY: usize = 16; + +/// An insertion-ordered set of [`ValueReason`]s, stored inline. +/// +/// This is a deliberate replacement for `IndexSet`, enabling insertion-order +/// memory while avoiding any heap allocation. At `AbstractValue`'s scale, this +/// has a dramatic impact on heap memory and wall time. +/// This takes advantage of the format of the data it's actually storing. A set +/// can hold at most one of each variant, so the members fit into a fixed inline +/// array. `ValueReason` is implemented as a single byte, so this struct is +/// ~18 bytes on the stack. +/// +/// Insertion order is preserved deliberately: [`primary_reason`] returns the +/// first non-`Other` member, matching the iteration order of the `Set` used by +/// the TypeScript implementation this is ported from. +#[derive(Debug, Clone, Copy)] +struct ValueReasonSet { + /// Members in insertion order. Only the first `len` entries are meaningful. + members: [ValueReason; VALUE_REASON_CAPACITY], + len: u8, } -fn hashset_of(r: ValueReason) -> IndexSet { - let mut s = IndexSet::default(); - s.insert(r); - s +impl Default for ValueReasonSet { + fn default() -> Self { + ValueReasonSet { + members: [ValueReason::Other; VALUE_REASON_CAPACITY], + len: 0, + } + } +} + +impl ValueReasonSet { + fn single(reason: ValueReason) -> Self { + let mut set = Self::default(); + set.insert(reason); + set + } + + fn contains(&self, reason: ValueReason) -> bool { + self.members[..self.len as usize].contains(&reason) + } + + fn iter(&self) -> impl Iterator + '_ { + self.members[..self.len as usize].iter().copied() + } + + /// Appends `reason` if not already present, preserving insertion order. + fn insert(&mut self, reason: ValueReason) { + if self.contains(reason) { + return; + } + debug_assert!( + (self.len as usize) < VALUE_REASON_CAPACITY, + "ValueReasonSet capacity must cover every ValueReason variant" + ); + if (self.len as usize) < VALUE_REASON_CAPACITY { + self.members[self.len as usize] = reason; + self.len += 1; + } + } + + /// True when every member of `other` is also a member of `self`. + fn is_superset_of(&self, other: &ValueReasonSet) -> bool { + other.iter().all(|reason| self.contains(reason)) + } + + /// Adds every member of `other`, keeping `self`'s existing order and + /// appending newcomers in `other`'s order — matching `IndexSet::insert`. + fn union_with(&mut self, other: &ValueReasonSet) { + for reason in other.iter() { + self.insert(reason); + } + } } // ============================================================================= @@ -315,7 +387,7 @@ impl InferenceState { } return AbstractValue { kind: ValueKind::Mutable, - reason: hashset_of(ValueReason::Other), + reason: ValueReasonSet::single(ValueReason::Other), }; } }; @@ -332,7 +404,7 @@ impl InferenceState { } merged_kind.unwrap_or_else(|| AbstractValue { kind: ValueKind::Mutable, - reason: hashset_of(ValueReason::Other), + reason: ValueReasonSet::single(ValueReason::Other), }) } @@ -360,7 +432,7 @@ impl InferenceState { vid, AbstractValue { kind: ValueKind::Mutable, - reason: hashset_of(ValueReason::Other), + reason: ValueReasonSet::single(ValueReason::Other), }, ); } @@ -438,7 +510,7 @@ impl InferenceState { value_id, AbstractValue { kind: ValueKind::Frozen, - reason: hashset_of(reason), + reason: ValueReasonSet::single(reason), }, ); // Note: In TS, this also transitively freezes FunctionExpression captures @@ -493,7 +565,7 @@ impl InferenceState { if let Some(other_value) = other.values.get(id) { let merged = merge_abstract_values(this_value, other_value); if merged.kind != this_value.kind - || !is_superset(&this_value.reason, &merged.reason) + || !this_value.reason.is_superset_of(&merged.reason) { let nv = next_values.get_or_insert_with(|| self.values.clone()); nv.insert(*id, merged); @@ -566,13 +638,6 @@ impl InferenceState { } } -fn is_superset( - a: &IndexSet, - b: &IndexSet, -) -> bool { - b.iter().all(|x| a.contains(x)) -} - #[derive(Debug, Clone, Copy)] enum MutateVariant { Mutate, @@ -738,13 +803,11 @@ fn hash_effect(effect: &AliasingEffect) -> String { fn merge_abstract_values(a: &AbstractValue, b: &AbstractValue) -> AbstractValue { let kind = merge_value_kinds(a.kind, b.kind); - if kind == a.kind && kind == b.kind && is_superset(&a.reason, &b.reason) { + if kind == a.kind && kind == b.kind && a.reason.is_superset_of(&b.reason) { return a.clone(); } - let mut reason = a.reason.clone(); - for r in &b.reason { - reason.insert(*r); - } + let mut reason = a.reason; + reason.union_with(&b.reason); AbstractValue { kind, reason } } @@ -1233,7 +1296,7 @@ fn apply_signature( vid, AbstractValue { kind: ValueKind::Mutable, - reason: hashset_of(ValueReason::Other), + reason: ValueReasonSet::single(ValueReason::Other), }, ); state.define(instr.lvalue.identifier, vid); @@ -1341,7 +1404,7 @@ fn apply_effect( value_id, AbstractValue { kind, - reason: hashset_of(reason), + reason: ValueReasonSet::single(reason), }, ); state.define(into.identifier, value_id); @@ -1370,7 +1433,7 @@ fn apply_effect( value_id, AbstractValue { kind: from_value.kind, - reason: from_value.reason.clone(), + reason: from_value.reason, }, ); state.define(into.identifier, value_id); @@ -1487,7 +1550,7 @@ fn apply_effect( } else { ValueKind::Frozen }, - reason: IndexSet::default(), + reason: ValueReasonSet::default(), }, ); state.define(into.identifier, value_id); @@ -1599,7 +1662,7 @@ fn apply_effect( value_id, AbstractValue { kind: from_value.kind, - reason: from_value.reason.clone(), + reason: from_value.reason, }, ); state.define(into.identifier, value_id); @@ -1615,7 +1678,7 @@ fn apply_effect( value_id, AbstractValue { kind: from_value.kind, - reason: from_value.reason.clone(), + reason: from_value.reason, }, ); state.define(into.identifier, value_id); @@ -3410,8 +3473,8 @@ fn compute_effects_for_aliasing_signature( /// since the primary reason is always inserted first, this effectively /// picks the most specific non-Other reason. We replicate this by /// preferring any non-Other reason over Other. -fn primary_reason(reasons: &IndexSet) -> ValueReason { - for &r in reasons { +fn primary_reason(reasons: &ValueReasonSet) -> ValueReason { + for r in reasons.iter() { if r != ValueReason::Other { return r; } @@ -3420,32 +3483,32 @@ fn primary_reason(reasons: &IndexSet) -> ValueReason } fn get_write_error_reason(abstract_value: &AbstractValue) -> String { - if abstract_value.reason.contains(&ValueReason::Global) { + if abstract_value.reason.contains(ValueReason::Global) { "Modifying a variable defined outside a component or hook is not allowed. Consider using an effect".to_string() - } else if abstract_value.reason.contains(&ValueReason::JsxCaptured) { + } else if abstract_value.reason.contains(ValueReason::JsxCaptured) { "Modifying a value used previously in JSX is not allowed. Consider moving the modification before the JSX".to_string() - } else if abstract_value.reason.contains(&ValueReason::Context) { + } else if abstract_value.reason.contains(ValueReason::Context) { "Modifying a value returned from 'useContext()' is not allowed.".to_string() } else if abstract_value .reason - .contains(&ValueReason::KnownReturnSignature) + .contains(ValueReason::KnownReturnSignature) { "Modifying a value returned from a function whose return value should not be mutated" .to_string() } else if abstract_value .reason - .contains(&ValueReason::ReactiveFunctionArgument) + .contains(ValueReason::ReactiveFunctionArgument) { "Modifying component props or hook arguments is not allowed. Consider using a local variable instead".to_string() - } else if abstract_value.reason.contains(&ValueReason::State) { + } else if abstract_value.reason.contains(ValueReason::State) { "Modifying a value returned from 'useState()', which should not be modified directly. Use the setter function to update instead".to_string() - } else if abstract_value.reason.contains(&ValueReason::ReducerState) { + } else if abstract_value.reason.contains(ValueReason::ReducerState) { "Modifying a value returned from 'useReducer()', which should not be modified directly. Use the dispatch function to update instead".to_string() - } else if abstract_value.reason.contains(&ValueReason::Effect) { + } else if abstract_value.reason.contains(ValueReason::Effect) { "Modifying a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the modification before calling useEffect()".to_string() - } else if abstract_value.reason.contains(&ValueReason::HookCaptured) { + } else if abstract_value.reason.contains(ValueReason::HookCaptured) { "Modifying a value previously passed as an argument to a hook is not allowed. Consider moving the modification before calling the hook".to_string() - } else if abstract_value.reason.contains(&ValueReason::HookReturn) { + } else if abstract_value.reason.contains(ValueReason::HookReturn) { "Modifying a value returned from a hook is not allowed. Consider moving the modification into the hook where the value is constructed".to_string() } else { "This modifies a variable that React considers immutable".to_string() From 8c94e0e1d90d5aa76d343a2a524bd22cba608f5f Mon Sep 17 00:00:00 2001 From: Andrew Imm Date: Mon, 24 Aug 2026 10:38:39 -0700 Subject: [PATCH 2/4] React Compiler: Avoid unnecessary clone in effect inference (#37207) ## Summary In the mutation / aliasing inference, the state is used twice, and both get cloned. Only one clone is necessary, the second can be safely moved. This has minimal impact on peak memory usage, but it does reduce wall time by *10%* on real Next.js benchmark apps. ## How did you test this change? Ran against benchmark apps to confirm identical byte output. Ran against the 1800+ Rust Compiler test fixtures. --- .../src/infer_mutation_aliasing_effects.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/crates/react_compiler_inference/src/infer_mutation_aliasing_effects.rs b/compiler/crates/react_compiler_inference/src/infer_mutation_aliasing_effects.rs index b6d9cab4e39..3c0bbedd386 100644 --- a/compiler/crates/react_compiler_inference/src/infer_mutation_aliasing_effects.rs +++ b/compiler/crates/react_compiler_inference/src/infer_mutation_aliasing_effects.rs @@ -185,7 +185,7 @@ pub fn infer_mutation_aliasing_effects( }; states_by_block.insert(block_id, incoming_state.clone()); - let mut state = incoming_state.clone(); + let mut state = incoming_state; infer_block(&mut context, &mut state, block_id, func, env)?; From b939280bb3e2d79f62fd62aa264cbf2a3d5b90a2 Mon Sep 17 00:00:00 2001 From: Andrew Imm Date: Mon, 24 Aug 2026 10:38:48 -0700 Subject: [PATCH 3/4] React Compiler: avoid deep ast clone and quadratic-time codegen (#37206) tl;dr this reduces peak memory allocation by 5-15%, and reduces codegen time by 30-70% depending on payload. On heavy components with deep ASTs the impact is more exaggerated. ## Summary Codegen of temp vars records the expressions they replaced, so that they can be unwound. In the TS version this uses a `Map` and stores pointers to AST nodes - relatively cheap. For borrow-checking reasons, the Rust version clones the AST. This results in recurring deep clones, making codegen accidentally quadratic and using significant amounts of memory. This introduces a convenience data structure for emitting temp vars in an unwindable manner, without heavy AST allocation. It also avoids a separate AST deep clone when propagating null values. ## How did you test this change? All fixtures pass with byte-identical outputs. Ran this against real codebases and pathological benchmark cases, confirming byte-identical output as well. --- .../src/propagate_scope_dependencies_hir.rs | 34 ++-- .../src/codegen_reactive_function.rs | 170 +++++++++++++++--- .../validate_preserved_manual_memoization.rs | 13 +- 3 files changed, 171 insertions(+), 46 deletions(-) diff --git a/compiler/crates/react_compiler_inference/src/propagate_scope_dependencies_hir.rs b/compiler/crates/react_compiler_inference/src/propagate_scope_dependencies_hir.rs index d0538aad52f..2590a32125e 100644 --- a/compiler/crates/react_compiler_inference/src/propagate_scope_dependencies_hir.rs +++ b/compiler/crates/react_compiler_inference/src/propagate_scope_dependencies_hir.rs @@ -1472,32 +1472,36 @@ fn recursively_propagate_non_null( } // Compute intersection of 'done' neighbors only (filter out 'active' = cycle nodes) - let done_neighbor_sets: Vec> = neighbors - .iter() - .filter(|n| traversal_state.get(n) == Some(&TraversalState::Done)) - .filter_map(|n| working.get(n).cloned()) - .collect(); + let neighbor_intersection = { + let done_neighbor_sets: Vec<&BTreeSet> = neighbors + .iter() + .filter(|n| traversal_state.get(n) == Some(&TraversalState::Done)) + .filter_map(|n| working.get(n)) + .collect(); - let neighbor_intersection = if done_neighbor_sets.is_empty() { - BTreeSet::new() - } else { - let mut iter = done_neighbor_sets.into_iter(); - let first = iter.next().unwrap(); - iter.fold(first, |acc, s| acc.intersection(&s).copied().collect()) + match done_neighbor_sets.split_first() { + None => BTreeSet::new(), + Some((first, rest)) => rest.iter().fold((*first).clone(), |acc, s| { + acc.intersection(s).copied().collect() + }), + } }; - let prev_objects = working.get(&node_id).cloned().unwrap_or_default(); + // Temporarily remove the previous set out of the map so it can be safely + // borrowed and compared without a heavy deep clone. + let prev_objects = working.remove(&node_id).unwrap_or_default(); let mut merged: BTreeSet = prev_objects .union(&neighbor_intersection) .copied() .collect(); reduce_maybe_optional_chains(&mut merged, registry); - working.insert(node_id, merged.clone()); - traversal_state.insert(node_id, TraversalState::Done); - // Compare with previous value — can't just check size due to reduce_maybe_optional_chains changed |= prev_objects != merged; + + working.insert(node_id, merged); + traversal_state.insert(node_id, TraversalState::Done); + changed } diff --git a/compiler/crates/react_compiler_reactive_scopes/src/codegen_reactive_function.rs b/compiler/crates/react_compiler_reactive_scopes/src/codegen_reactive_function.rs index 9a722b105a0..c9063c5bd1a 100644 --- a/compiler/crates/react_compiler_reactive_scopes/src/codegen_reactive_function.rs +++ b/compiler/crates/react_compiler_reactive_scopes/src/codegen_reactive_function.rs @@ -555,14 +555,121 @@ pub fn codegen_function( // Context // ============================================================================= -type Temporaries = FxHashMap>; - #[derive(Clone)] enum ExpressionOrJsxText { Expression(Expression), JsxText(JSXText), } +/// The entry a write to [`Temporaries`] displaced, kept so the write can be +/// undone. +/// +/// The expression is boxed because `ExpressionOrJsxText` is ~900 bytes (it +/// inlines an `Expression`). Unboxed, the undo log would be a `Vec` of +/// ~900-byte slots that are almost always `Absent`, costing more peak heap than +/// the copy it replaces on shallow functions. Boxed, an entry is 16 bytes and +/// only allocates when a write actually displaces a buffered expression. +enum Displaced { + /// The key was not present before the write. + Absent, + /// The key was present as a declared temporary with no buffered value. + Empty, + /// The key was present with this buffered value. + Value(Box), +} + +/// A position in a [`Temporaries`] undo log, produced by [`Temporaries::mark`]. +#[derive(Clone, Copy)] +struct TempMark(usize); + +/// Expressions buffered for temporaries that have not been emitted yet, plus an +/// undo log allowing a nested block or scope to be codegen'd and its additions +/// discarded. +/// +/// The TypeScript implementation snapshots this with `new Map(cx.temp)`, which +/// is a *shallow* copy: it duplicates references, not the AST nodes behind them. +/// The equivalent Rust `.clone()` deep-copies every buffered `Expression` tree, +/// which made codegen quadratic in component size and dominated both allocation +/// volume and peak heap. +/// +/// `TS CodegenReactiveFunction.codegenBlock` asserts that pre-existing entries +/// are never mutated ("Expected temporary value to be unchanged"), so a +/// snapshot's only job is to discard entries added by the nested block. Because +/// entries are only ever inserted (never removed, nor mutated in place), +/// rewinding an insert log restores the map exactly, with no copying. +/// +/// All writes go through [`Temporaries::set`] so the log cannot drift out of +/// sync with the map. +#[derive(Default)] +struct Temporaries { + values: FxHashMap>, + journal: Vec<(DeclarationId, Displaced)>, +} + +impl Temporaries { + fn get(&self, declaration_id: DeclarationId) -> Option<&Option> { + self.values.get(&declaration_id) + } + + fn contains_key(&self, declaration_id: DeclarationId) -> bool { + self.values.contains_key(&declaration_id) + } + + /// Buffers `value` for `declaration_id`, journaling the displaced entry. + /// `HashMap::insert` returns that entry by move, so journaling costs no + /// clones. + fn set(&mut self, declaration_id: DeclarationId, value: Option) { + let displaced = match self.values.insert(declaration_id, value) { + None => Displaced::Absent, + Some(None) => Displaced::Empty, + Some(Some(previous)) => Displaced::Value(Box::new(previous)), + }; + self.journal.push((declaration_id, displaced)); + } + + /// Marks the current state, for a later [`Temporaries::rewind`]. + fn mark(&self) -> TempMark { + TempMark(self.journal.len()) + } + + /// Restores the state captured by `mark`, discarding every write since. + fn rewind(&mut self, mark: TempMark) { + while self.journal.len() > mark.0 { + let (declaration_id, displaced) = self.journal.pop().unwrap(); + match displaced { + Displaced::Absent => { + self.values.remove(&declaration_id); + } + Displaced::Empty => { + self.values.insert(declaration_id, None); + } + Displaced::Value(previous) => { + self.values.insert(declaration_id, Some(*previous)); + } + } + } + } + + /// Hands the buffered expressions to a nested function's context, which may + /// read them but must not leak its own additions back out. + /// + /// The borrower gets a fresh log, so [`Temporaries::reclaim`] can undo + /// exactly the borrower's writes rather than the lender's whole history. + fn lend(&mut self) -> Temporaries { + Temporaries { + values: std::mem::take(&mut self.values), + journal: Vec::new(), + } + } + + /// Takes back expressions handed out by [`Temporaries::lend`], discarding + /// every write the borrower made. + fn reclaim(&mut self, mut lent: Temporaries) { + lent.rewind(TempMark(0)); + self.values = lent.values; + } +} + struct Context<'env> { env: &'env mut Environment, #[allow(dead_code)] @@ -594,7 +701,7 @@ impl<'env> Context<'env> { fn_name, next_cache_index: 0, declarations: FxHashSet::default(), - temp: FxHashMap::default(), + temp: Temporaries::default(), object_methods: FxHashMap::default(), unique_identifiers, fbt_operands, @@ -653,8 +760,8 @@ fn codegen_reactive_function( ParamPattern::Place(p) => p, ParamPattern::Spread(sp) => &sp.place, }; - let ident = &cx.env.identifiers[place.identifier.0 as usize]; - cx.temp.insert(ident.declaration_id, None); + let declaration_id = cx.env.identifiers[place.identifier.0 as usize].declaration_id; + cx.temp.set(declaration_id, None); cx.declare(place.identifier); } @@ -732,9 +839,9 @@ fn convert_parameter( // ============================================================================= fn codegen_block(cx: &mut Context, block: &ReactiveBlock) -> Result { - let temp_snapshot: Temporaries = cx.temp.clone(); + let mark = cx.temp.mark(); let result = codegen_block_no_reset(cx, block)?; - cx.temp = temp_snapshot; + cx.temp.rewind(mark); Ok(result) } @@ -758,9 +865,9 @@ fn codegen_block_no_reset( scope, instructions, }) => { - let temp_snapshot = cx.temp.clone(); + let mark = cx.temp.mark(); codegen_reactive_scope(cx, &mut statements, *scope, instructions)?; - cx.temp = temp_snapshot; + cx.temp.rewind(mark); } ReactiveStatement::Terminal(term_stmt) => { let stmt = codegen_terminal(cx, &term_stmt.terminal)?; @@ -1258,8 +1365,9 @@ fn codegen_terminal( } => { let catch_param = match handler_binding.as_ref() { Some(binding) => { - let ident = &cx.env.identifiers[binding.identifier.0 as usize]; - cx.temp.insert(ident.declaration_id, None); + let declaration_id = + cx.env.identifiers[binding.identifier.0 as usize].declaration_id; + cx.temp.set(declaration_id, None); Some(PatternLike::Identifier(convert_identifier( binding.identifier, cx.env, @@ -1724,8 +1832,10 @@ fn codegen_store_or_declare( // Register temporaries for unnamed pattern operands for place in react_compiler_hir::visitors::each_pattern_operand(&lvalue.pattern) { let ident = &cx.env.identifiers[place.identifier.0 as usize]; - if kind != InstructionKind::Reassign && ident.name.is_none() { - cx.temp.insert(ident.declaration_id, None); + let declaration_id = ident.declaration_id; + let is_unnamed = ident.name.is_none(); + if kind != InstructionKind::Reassign && is_unnamed { + cx.temp.set(declaration_id, None); } } let rhs = codegen_place_to_expression(cx, val)?; @@ -1838,11 +1948,10 @@ fn emit_store( ReactiveValue::Instruction(InstructionValue::StoreContext { .. }) ); if !is_store_context { - let ident = &cx.env.identifiers[lvalue_place.identifier.0 as usize]; - cx.temp.insert( - ident.declaration_id, - Some(ExpressionOrJsxText::Expression(expr)), - ); + let declaration_id = + cx.env.identifiers[lvalue_place.identifier.0 as usize].declaration_id; + cx.temp + .set(declaration_id, Some(ExpressionOrJsxText::Expression(expr))); return Ok(None); } else { let stmt = @@ -1886,9 +1995,10 @@ fn codegen_instruction( })); }; let ident = &cx.env.identifiers[lvalue.identifier.0 as usize]; + let declaration_id = ident.declaration_id; if ident.name.is_none() { // temporary - cx.temp.insert(ident.declaration_id, Some(value)); + cx.temp.set(declaration_id, Some(value)); return Ok(Statement::EmptyStatement(EmptyStatement { base: BaseNode::typed("EmptyStatement"), })); @@ -2663,9 +2773,16 @@ fn codegen_function_expression( cx.unique_identifiers.clone(), cx.fbt_operands.clone(), ); - inner_cx.temp = cx.temp.clone(); + // The inner function reads the enclosing temporaries but must not leak its + // own back out. Lend the map to `inner_cx` and rewind its writes on the way + // out, rather than deep-cloning every buffered expression tree. The map is + // restored on the error path too, so `cx` is never left empty. + inner_cx.temp = cx.temp.lend(); - let fn_result = codegen_reactive_function(&mut inner_cx, &reactive_fn_mut)?; + let fn_result = codegen_reactive_function(&mut inner_cx, &reactive_fn_mut); + + cx.temp.reclaim(std::mem::take(&mut inner_cx.temp)); + let fn_result = fn_result?; let value = match expr_type { FunctionExpressionType::ArrowFunctionExpression => { @@ -2798,9 +2915,12 @@ fn codegen_object_expression( cx.unique_identifiers.clone(), cx.fbt_operands.clone(), ); - inner_cx.temp = cx.temp.clone(); + inner_cx.temp = cx.temp.lend(); + + let fn_result = codegen_reactive_function(&mut inner_cx, &reactive_fn_mut); - let fn_result = codegen_reactive_function(&mut inner_cx, &reactive_fn_mut)?; + cx.temp.reclaim(std::mem::take(&mut inner_cx.temp)); + let fn_result = fn_result?; ast_properties.push(ast_expr::ObjectExpressionProperty::ObjectMethod( ast_expr::ObjectMethod { @@ -3309,14 +3429,14 @@ fn codegen_place_to_expression( fn codegen_place(cx: &mut Context, place: &Place) -> Result { let ident = &cx.env.identifiers[place.identifier.0 as usize]; - if let Some(tmp) = cx.temp.get(&ident.declaration_id) { + if let Some(tmp) = cx.temp.get(ident.declaration_id) { if let Some(val) = tmp { return Ok(val.clone()); } // tmp is None — means declared but no temp value, fall through } // Check if it's an unnamed identifier without a temp - if ident.name.is_none() && !cx.temp.contains_key(&ident.declaration_id) { + if ident.name.is_none() && !cx.temp.contains_key(ident.declaration_id) { return Err(invariant_err( &format!( "[Codegen] No value found for temporary, identifier id={}", diff --git a/compiler/crates/react_compiler_validation/src/validate_preserved_manual_memoization.rs b/compiler/crates/react_compiler_validation/src/validate_preserved_manual_memoization.rs index b757a927dd9..e209441d3b9 100644 --- a/compiler/crates/react_compiler_validation/src/validate_preserved_manual_memoization.rs +++ b/compiler/crates/react_compiler_validation/src/validate_preserved_manual_memoization.rs @@ -142,18 +142,19 @@ fn visit_scope(scope_block: &ReactiveScopeBlock, state: &mut VisitorState) { if let Some(ref memo_state) = state.manual_memo_state { if let Some(ref deps_from_source) = memo_state.deps_from_source { let scope = &state.env.scopes[scope_block.scope.0 as usize]; + // `dependencies` still has to be cloned because `env` is passed + // mutably below. `temporaries`, `decls` and `deps_from_source` do + // not: they live in fields disjoint from `env`, so they can simply + // be borrowed. let deps = scope.dependencies.clone(); let memo_loc = memo_state.loc; - let decls = memo_state.decls.clone(); - let deps_from_source = deps_from_source.clone(); - let temporaries = state.temporaries.clone(); for dep in &deps { validate_inferred_dep( dep.identifier, &dep.path, - &temporaries, - &decls, - &deps_from_source, + &state.temporaries, + &memo_state.decls, + deps_from_source, state.env, memo_loc, ); From 3d050805e802e6c340d2f0c0962dd5a1616a44a4 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Mon, 24 Aug 2026 19:46:15 +0200 Subject: [PATCH 4/4] [Fizz] Construct the render lifetime controller only when it is needed (#37357) This follows #37315, which added the render lifetime controller to bound the abort listener that `attachAbortSignal` attaches to a caller's signal. `RequestInstance` constructed one for every request, so a render that is given no signal allocated a controller, aborted it on completion, and nothing ever observed either. The controller is now created in `attachAbortSignal`, and the three places that end the lifetime go through `endRenderLifetime`, which does nothing when there is no controller. Callers that pass a signal are unaffected. Callers that do not no longer allocate one, and `signal` is optional in every browser, edge and static entry point, while `renderToPipeableStream` and `resumeToPipeableStream` accept no signal at all. They also no longer reach `AbortController` at all, which matters more than the allocation. Fizz had no runtime dependency on it before #37315, and an unconditional one reaches environments that provide the API through a polyfill. An incomplete polyfill can then fail a render that never asked for abort support. The new test asserts that no controller is constructed when no signal is passed. It fails with the eager construction restored, since nothing else in the suite would notice a regression to it. --- .../ReactDOMFizzStaticBrowser-test.js | 26 +++++++++++++++++++ packages/react-server/src/ReactFizzServer.js | 23 +++++++++++----- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 6be61ffc2ca..efd69d0dc5e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -1584,6 +1584,32 @@ describe('ReactDOMFizzStaticBrowser', () => { expect(lifetimes[0].aborted).toBe(true); }); + it('constructs no abort controller when no signal is passed', async () => { + // The render lifetime exists only to bound the caller's abort listener, + // so a render that is given no signal has nothing to bound and should not + // pay for a controller. It also means such a render never requires + // AbortController to exist or to work. + const RealAbortController = globalThis.AbortController; + let constructed = 0; + globalThis.AbortController = class extends RealAbortController { + constructor() { + super(); + constructed++; + } + }; + + try { + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream(
hello world
), + ); + await readContent(stream); + } finally { + globalThis.AbortController = RealAbortController; + } + + expect(constructed).toBe(0); + }); + it('attaches no listener when the signal is already aborted', async () => { const controller = new AbortController(); controller.abort(); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index a863863ea60..1f249556b94 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -435,7 +435,9 @@ export opaque type Request = { onFatalError: (error: mixed) => void, // Aborted once the render ends, whether it completed, failed fatally or was // aborted. Bounds the lifetime of anything that must not outlive the render. - renderLifetimeController: AbortController, + // Null until attachAbortSignal creates it, so a render that is given no + // signal constructs no controller. + renderLifetimeController: null | AbortController, // Form state that was the result of an MPA submission, if it was provided. formState: null | ReactFormState, // DEV-only, warning dedupe @@ -589,7 +591,7 @@ function RequestInstance( this.onShellReady = onShellReady === undefined ? noop : onShellReady; this.onShellError = onShellError === undefined ? noop : onShellError; this.onFatalError = onFatalError === undefined ? noop : onFatalError; - this.renderLifetimeController = new AbortController(); + this.renderLifetimeController = null; this.formState = formState === undefined ? null : formState; if (__DEV__) { this.didWarnForKey = null; @@ -1455,7 +1457,7 @@ function fatalError( } onFatalError(error); } - request.renderLifetimeController.abort(RENDER_ENDED); + endRenderLifetime(request); if (request.destination !== null) { request.status = CLOSED; closeWithError(request.destination, error); @@ -6352,7 +6354,7 @@ function flushCompletedQueues( } } // We're done. - request.renderLifetimeController.abort(RENDER_ENDED); + endRenderLifetime(request); request.status = CLOSED; close(destination); // We need to stop flowing now because we do not want any async contexts which might call @@ -6516,6 +6518,13 @@ function finishAbort(request: Request, abortableTasks: Set): void { } } +function endRenderLifetime(request: Request): void { + const renderLifetimeController = request.renderLifetimeController; + if (renderLifetimeController !== null) { + renderLifetimeController.abort(RENDER_ENDED); + } +} + // Aborts the request when the caller's signal aborts. The render lifetime // bounds the listener, so the runtime removes the listener as soon as the // render ends. From that point on abort() returns early, so the listener has @@ -6533,12 +6542,14 @@ export function attachAbortSignal(request: Request, signal: AbortSignal): void { abort(request, signal.reason); return; } + const renderLifetimeController = new AbortController(); + request.renderLifetimeController = renderLifetimeController; signal.addEventListener( 'abort', () => { abort(request, signal.reason); }, - {signal: request.renderLifetimeController.signal}, + {signal: renderLifetimeController.signal}, ); } @@ -6552,7 +6563,7 @@ export function abort(request: Request, reason: mixed): void { // can be aborted. in practice this makes abort callable at most once per render. return; } - request.renderLifetimeController.abort(RENDER_ENDED); + endRenderLifetime(request); const isRecoverableReason = typeof reason === 'object' && reason !== null &&