From 13ae975926c08eb84c2c730d598d85f2890fa418 Mon Sep 17 00:00:00 2001 From: Moulins Date: Fri, 24 Oct 2025 02:10:01 +0200 Subject: [PATCH 1/3] avm1: Only keep a reference (instead of a Rc) to registers in `Activation` This avoid a `Rc::clone` call on rescopes (e.g. for `with` or `try/catch`) and simplifies `Activation`'s drop glue. --- core/src/avm1/activation.rs | 23 ++++++++--------------- core/src/avm1/function.rs | 9 ++++++--- core/src/avm1/runtime.rs | 3 +++ 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/core/src/avm1/activation.rs b/core/src/avm1/activation.rs index 09a1fd216287..9a7d9e4a6cb9 100644 --- a/core/src/avm1/activation.rs +++ b/core/src/avm1/activation.rs @@ -24,7 +24,6 @@ use std::borrow::Cow; use std::cell::Cell; use std::cmp::min; use std::fmt; -use std::rc::Rc; use swf::avm1::read::Reader; use swf::avm1::types::*; use url::form_urlencoded; @@ -168,10 +167,7 @@ pub struct Activation<'a, 'gc: 'a> { /// /// Registers are numbered from 1; r0 does not exist. Therefore this vec, /// while nominally starting from zero, actually starts from r1. - /// - /// Registers are stored in a `Rc` so that rescopes (e.g. with) use the - /// same register set. - local_registers: Option>]>>, + local_registers: Option<&'a [Cell>]>, /// The base clip of this stack frame. /// This will be the MovieClip that contains the bytecode. @@ -238,6 +234,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { base_clip: DisplayObject<'gc>, this: Value<'gc>, callee: Option>, + registers: &'a [Cell>], ) -> Self { avm_debug!(context.avm1, "START {id}"); Self { @@ -250,7 +247,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { target_clip: Some(base_clip), this, callee, - local_registers: None, + local_registers: Some(registers).filter(|r| !r.is_empty()), } } @@ -272,7 +269,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { target_clip: self.target_clip, this: self.this, callee: self.callee, - local_registers: self.local_registers.clone(), + local_registers: self.local_registers, } } @@ -367,6 +364,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { active_clip, clip_obj.into(), None, + &[], ); child_activation.run_actions(code) } @@ -403,6 +401,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { active_clip, clip_obj.into(), None, + &[], ); function(&mut activation) } @@ -2245,9 +2244,10 @@ impl<'a, 'gc> Activation<'a, 'gc> { self.base_clip, self.this, self.callee, + &[], ); - activation.local_registers = self.local_registers.clone(); + activation.local_registers = self.local_registers; match catch_vars { CatchVar::Var(name) => { @@ -3053,13 +3053,6 @@ impl<'a, 'gc> Activation<'a, 'gc> { self.this } - pub fn allocate_local_registers(&mut self, num: u8) { - self.local_registers = match num { - 0 => None, - num => Some((0..num).map(|_| Cell::new(Value::Undefined)).collect()), - }; - } - pub fn constant_pool(&self) -> Gc<'gc, Vec>> { self.constant_pool } diff --git a/core/src/avm1/function.rs b/core/src/avm1/function.rs index 502d5eb621d8..fd14c073486c 100644 --- a/core/src/avm1/function.rs +++ b/core/src/avm1/function.rs @@ -14,7 +14,7 @@ use crate::string::{AvmString, StringContext, SwfStrExt as _}; use crate::tag_utils::SwfSlice; use gc_arena::{Collect, Gc, Mutation}; use ruffle_macros::istr; -use std::{borrow::Cow, num::NonZeroU8}; +use std::{borrow::Cow, cell::Cell, num::NonZeroU8}; use swf::{avm1::types::FunctionFlags, SwfStr}; /// Represents a function defined in Ruffle's code. @@ -341,6 +341,10 @@ impl<'gc> Avm1Function<'gc> { this }; + let local_registers: Box<[_]> = (0..self.register_count()) + .map(|_| Cell::new(Value::Undefined)) + .collect(); + let max_recursion_depth = activation.context.avm1.max_recursion_depth(); let mut frame = Activation::from_action( activation.context, @@ -351,10 +355,9 @@ impl<'gc> Avm1Function<'gc> { base_clip, local_this, Some(callee), + &local_registers, ); - frame.allocate_local_registers(self.register_count()); - let mut preload_r = 1; self.load_this(&mut frame, this, &mut preload_r); self.load_arguments(&mut frame, args, arguments_caller, &mut preload_r); diff --git a/core/src/avm1/runtime.rs b/core/src/avm1/runtime.rs index c75b1b72da71..f49803b23aa0 100644 --- a/core/src/avm1/runtime.rs +++ b/core/src/avm1/runtime.rs @@ -175,6 +175,7 @@ impl<'gc> Avm1<'gc> { active_clip, clip_obj.into(), None, + &[], ); if let Err(e) = child_activation.run_actions(code) { root_error_handler(&mut child_activation, e); @@ -213,6 +214,7 @@ impl<'gc> Avm1<'gc> { active_clip, clip_obj.into(), None, + &[], ); function(&mut activation) } @@ -259,6 +261,7 @@ impl<'gc> Avm1<'gc> { active_clip, clip_obj.into(), None, + &[], ); if let Err(e) = child_activation.run_actions(code) { root_error_handler(&mut child_activation, e); From 4ab8f67cc1f626849c9e16568dacce26fa0bdf16 Mon Sep 17 00:00:00 2001 From: Moulins Date: Sun, 26 Oct 2025 02:38:26 +0200 Subject: [PATCH 2/3] avm1: Remove `Option` for `Activation.local_registers` field `Some(&[])` could never happen, and allocating an empty `Box<[_]>` is free, so we can treat `&[]` as "has no local registers". Also remove a misleading comment concerning local register #0 (which *does* exist, even though most AS2 compilers don't like to use it for some reason). --- core/src/avm1/activation.rs | 41 ++++++++++++++----------------------- core/src/avm1/function.rs | 1 + 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/core/src/avm1/activation.rs b/core/src/avm1/activation.rs index 9a7d9e4a6cb9..dcc4c5ebd910 100644 --- a/core/src/avm1/activation.rs +++ b/core/src/avm1/activation.rs @@ -161,13 +161,8 @@ pub struct Activation<'a, 'gc: 'a> { /// Local registers, if any. /// - /// None indicates a function executing out of the global register set. - /// Some indicates the existence of local registers, even if none exist. - /// i.e. None(Vec::new()) means no registers should exist at all. - /// - /// Registers are numbered from 1; r0 does not exist. Therefore this vec, - /// while nominally starting from zero, actually starts from r1. - local_registers: Option<&'a [Cell>]>, + /// An empty slice indicates a function executing out of the global register set. + local_registers: &'a [Cell>], /// The base clip of this stack frame. /// This will be the MovieClip that contains the bytecode. @@ -234,7 +229,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { base_clip: DisplayObject<'gc>, this: Value<'gc>, callee: Option>, - registers: &'a [Cell>], + local_registers: &'a [Cell>], ) -> Self { avm_debug!(context.avm1, "START {id}"); Self { @@ -247,7 +242,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { target_clip: Some(base_clip), this, callee, - local_registers: Some(registers).filter(|r| !r.is_empty()), + local_registers, } } @@ -298,7 +293,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { target_clip: Some(base_clip), this: scope.locals_cell().into(), callee: None, - local_registers: None, + local_registers: &[], context, } } @@ -2399,13 +2394,11 @@ impl<'a, 'gc> Activation<'a, 'gc> { /// Value::Undefined, which is also a valid register value. pub fn current_register(&self, id: u8) -> Value<'gc> { let id = id as usize; - if let Some(local_registers) = &self.local_registers { - if let Some(reg) = local_registers.get(id) { - return reg.get(); - } else if self.context.player_version <= 10 { - // Old FP versions do not fall back to the global register set. - return Value::Undefined; - } + if let Some(reg) = self.local_registers.get(id) { + return reg.get(); + } else if !self.local_registers.is_empty() && self.context.player_version <= 10 { + // Old FP versions do not fall back to the global register set. + return Value::Undefined; } self.context @@ -2430,16 +2423,12 @@ impl<'a, 'gc> Activation<'a, 'gc> { /// /// If a given local register does not exist, this function does nothing. pub fn set_local_register(&mut self, id: u8, value: Value<'gc>) -> bool { - if let Some(local_registers) = &self.local_registers { - if let Some(reg) = local_registers.get(id as usize) { - reg.set(value); - true - } else { - // Old FP versions do not fall back to the global register set. - self.context.player_version <= 10 - } + if let Some(reg) = self.local_registers.get(id as usize) { + reg.set(value); + true } else { - false + // Old FP versions do not fall back to the global register set. + !self.local_registers.is_empty() && self.context.player_version <= 10 } } diff --git a/core/src/avm1/function.rs b/core/src/avm1/function.rs index fd14c073486c..0bb8b3cab18a 100644 --- a/core/src/avm1/function.rs +++ b/core/src/avm1/function.rs @@ -358,6 +358,7 @@ impl<'gc> Avm1Function<'gc> { &local_registers, ); + // Local register #0 is always left free. let mut preload_r = 1; self.load_this(&mut frame, this, &mut preload_r); self.load_arguments(&mut frame, args, arguments_caller, &mut preload_r); From a958f436284670e7839d20a24618156c428d1f96 Mon Sep 17 00:00:00 2001 From: Moulins Date: Sun, 26 Oct 2025 02:56:06 +0200 Subject: [PATCH 3/3] avm1: Don't store owned strings in `ActivationIdentifier` We can instead always store a `&'a str`. --- core/src/avm1/activation.rs | 31 +++++++++++++++---------------- core/src/avm1/function.rs | 2 +- core/src/avm1/runtime.rs | 13 +++++-------- core/src/streams.rs | 3 ++- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/core/src/avm1/activation.rs b/core/src/avm1/activation.rs index dcc4c5ebd910..0ea7ef5786f6 100644 --- a/core/src/avm1/activation.rs +++ b/core/src/avm1/activation.rs @@ -20,7 +20,6 @@ use gc_arena::{Gc, Mutation}; use indexmap::IndexMap; use rand::Rng; use ruffle_macros::istr; -use std::borrow::Cow; use std::cell::Cell; use std::cmp::min; use std::fmt; @@ -63,7 +62,7 @@ enum FrameControl<'gc> { #[derive(Clone)] pub struct ActivationIdentifier<'a> { parent: Option<&'a ActivationIdentifier<'a>>, - name: Cow<'static, str>, + name: &'a str, depth: u16, function_count: u16, special_count: u8, @@ -75,36 +74,36 @@ impl fmt::Display for ActivationIdentifier<'_> { write!(f, "{parent} / ")?; } - f.write_str(&self.name)?; + f.write_str(self.name)?; Ok(()) } } impl<'a> ActivationIdentifier<'a> { - pub fn root>>(name: S) -> Self { + pub fn root(name: &'a str) -> Self { Self { parent: None, - name: name.into(), + name, depth: 0, function_count: 0, special_count: 0, } } - pub fn child>>(&'a self, name: S) -> Self { + pub fn child(&'a self, name: &'a str) -> Self { Self { parent: Some(self), - name: name.into(), + name, depth: self.depth + 1, function_count: self.function_count, special_count: self.special_count, } } - pub fn function<'gc, S: Into>>( + pub fn function<'gc>( &'a self, - name: S, + name: &'a str, reason: ExecutionReason, max_recursion_depth: u16, ) -> Result> { @@ -124,7 +123,7 @@ impl<'a> ActivationIdentifier<'a> { }; Ok(Self { parent: Some(self), - name: name.into(), + name, depth: self.depth + 1, function_count, special_count, @@ -247,9 +246,9 @@ impl<'a, 'gc> Activation<'a, 'gc> { } /// Create a new activation to run a block of code with a given scope. - pub fn with_new_scope<'b, S: Into>>( + pub fn with_new_scope<'b>( &'b mut self, - name: S, + name: &'b str, scope: Gc<'gc, Scope<'gc>>, ) -> Activation<'b, 'gc> { let id = self.id.child(name); @@ -329,9 +328,9 @@ impl<'a, 'gc> Activation<'a, 'gc> { } /// Add a stack frame that executes code in timeline scope - pub fn run_child_frame_for_action>>( + pub fn run_child_frame_for_action( &mut self, - name: S, + name: &str, active_clip: DisplayObject<'gc>, code: SwfSlice, ) -> Result, Error<'gc>> { @@ -365,9 +364,9 @@ impl<'a, 'gc> Activation<'a, 'gc> { } /// Add a stack frame that executes code in initializer scope. - pub fn run_with_child_frame_for_display_object>>( + pub fn run_with_child_frame_for_display_object( &mut self, - name: S, + name: &str, active_clip: DisplayObject<'gc>, swf_version: u8, function: F, diff --git a/core/src/avm1/function.rs b/core/src/avm1/function.rs index 0bb8b3cab18a..8057ce4100d1 100644 --- a/core/src/avm1/function.rs +++ b/core/src/avm1/function.rs @@ -348,7 +348,7 @@ impl<'gc> Avm1Function<'gc> { let max_recursion_depth = activation.context.avm1.max_recursion_depth(); let mut frame = Activation::from_action( activation.context, - activation.id.function(name, reason, max_recursion_depth)?, + activation.id.function(&name, reason, max_recursion_depth)?, swf_version, child_scope, self.constant_pool, diff --git a/core/src/avm1/runtime.rs b/core/src/avm1/runtime.rs index f49803b23aa0..b0d61ff68125 100644 --- a/core/src/avm1/runtime.rs +++ b/core/src/avm1/runtime.rs @@ -12,7 +12,6 @@ use crate::string::{AvmString, StringContext}; use crate::tag_utils::SwfSlice; use crate::{avm1, avm_debug}; use gc_arena::{Collect, Gc, Mutation}; -use std::borrow::Cow; use swf::avm1::read::Reader; use tracing::instrument; @@ -136,9 +135,9 @@ impl<'gc> Avm1<'gc> { /// Add a stack frame that executes code in timeline scope /// /// This creates a new frame stack. - pub fn run_stack_frame_for_action>>( + pub fn run_stack_frame_for_action( active_clip: DisplayObject<'gc>, - name: S, + name: &str, code: SwfSlice, context: &mut UpdateContext<'gc>, ) { @@ -284,11 +283,9 @@ impl<'gc> Avm1<'gc> { return; } - let mut activation = Activation::from_nothing( - context, - ActivationIdentifier::root(name.to_string()), - active_clip, - ); + let name_utf8 = &name.to_utf8_lossy(); + let mut activation = + Activation::from_nothing(context, ActivationIdentifier::root(name_utf8), active_clip); let _ = obj.call_method(name, args, &mut activation, ExecutionReason::Special); } diff --git a/core/src/streams.rs b/core/src/streams.rs index 2fa5792e2843..38dafcc9d2f0 100644 --- a/core/src/streams.rs +++ b/core/src/streams.rs @@ -1369,11 +1369,12 @@ impl<'gc> NetStream<'gc> { match avm_object { Some(NetStreamKind::Avm1(object)) => { let avm_string_name = AvmString::new_utf8_bytes(context.gc(), variable_name); + let activation_name = format!("[FLV {avm_string_name}]"); let root = context.stage.root_clip().expect("root"); let mut activation = Avm1Activation::from_nothing( context, - Avm1ActivationIdentifier::root(format!("[FLV {avm_string_name}]")), + Avm1ActivationIdentifier::root(&activation_name), root, );