avm2: Make several slot operations faster

- `get_slot` and `set_slot_no_coerce` now panic instead of returning Err
- `get_slot` is now inlined
This commit is contained in:
Lord-McSweeney 2024-08-27 10:50:11 -07:00 committed by Lord-McSweeney
parent 7214440418
commit fc820b7710
5 changed files with 36 additions and 37 deletions

View File

@ -1754,7 +1754,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
fn op_get_slot(&mut self, index: u32) -> Result<FrameControl<'gc>, Error<'gc>> { fn op_get_slot(&mut self, index: u32) -> Result<FrameControl<'gc>, Error<'gc>> {
let object = self.pop_stack().coerce_to_object_or_typeerror(self, None)?; let object = self.pop_stack().coerce_to_object_or_typeerror(self, None)?;
let value = object.get_slot(index)?; let value = object.get_slot(index);
self.push_stack(value); self.push_stack(value);
@ -1774,7 +1774,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
let value = self.pop_stack(); let value = self.pop_stack();
let object = self.pop_stack().coerce_to_object_or_typeerror(self, None)?; let object = self.pop_stack().coerce_to_object_or_typeerror(self, None)?;
object.set_slot_no_coerce(index, value, self.context.gc_context)?; object.set_slot_no_coerce(index, value, self.context.gc_context);
Ok(FrameControl::Continue) Ok(FrameControl::Continue)
} }
@ -1783,7 +1783,6 @@ impl<'a, 'gc> Activation<'a, 'gc> {
let value = self let value = self
.global_scope() .global_scope()
.map(|global| global.get_slot(index)) .map(|global| global.get_slot(index))
.transpose()?
.unwrap_or(Value::Undefined); .unwrap_or(Value::Undefined);
self.push_stack(value); self.push_stack(value);

View File

@ -242,7 +242,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
) -> Result<Value<'gc>, Error<'gc>> { ) -> Result<Value<'gc>, Error<'gc>> {
match self.vtable().get_trait(multiname) { match self.vtable().get_trait(multiname) {
Some(Property::Slot { slot_id }) | Some(Property::ConstSlot { slot_id }) => { Some(Property::Slot { slot_id }) | Some(Property::ConstSlot { slot_id }) => {
self.base().get_slot(slot_id) Ok(self.base().get_slot(slot_id))
} }
Some(Property::Method { disp_id }) => { Some(Property::Method { disp_id }) => {
// avmplus has a special case for XML and XMLList objects, so we need one as well // avmplus has a special case for XML and XMLList objects, so we need one as well
@ -348,8 +348,11 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
let value = self let value = self
.vtable() .vtable()
.coerce_trait_value(slot_id, value, activation)?; .coerce_trait_value(slot_id, value, activation)?;
self.base() self.base()
.set_slot(slot_id, value, activation.context.gc_context) .set_slot(slot_id, value, activation.context.gc_context);
Ok(())
} }
Some(Property::Method { .. }) => { Some(Property::Method { .. }) => {
// Similar to the get_property special case for XML/XMLList. // Similar to the get_property special case for XML/XMLList.
@ -431,8 +434,11 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
let value = self let value = self
.vtable() .vtable()
.coerce_trait_value(slot_id, value, activation)?; .coerce_trait_value(slot_id, value, activation)?;
self.base() self.base()
.set_slot(slot_id, value, activation.context.gc_context) .set_slot(slot_id, value, activation.context.gc_context);
Ok(())
} }
Some(Property::Method { .. }) => { Some(Property::Method { .. }) => {
return Err(error::make_reference_error( return Err(error::make_reference_error(
@ -496,7 +502,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
) -> Result<Value<'gc>, Error<'gc>> { ) -> Result<Value<'gc>, Error<'gc>> {
match self.vtable().get_trait(multiname) { match self.vtable().get_trait(multiname) {
Some(Property::Slot { slot_id }) | Some(Property::ConstSlot { slot_id }) => { Some(Property::Slot { slot_id }) | Some(Property::ConstSlot { slot_id }) => {
let obj = self.base().get_slot(slot_id)?.as_callable( let obj = self.base().get_slot(slot_id).as_callable(
activation, activation,
Some(multiname), Some(multiname),
Some(Value::from(self.into())), Some(Value::from(self.into())),
@ -545,7 +551,8 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
/// Retrieve a slot by its index. /// Retrieve a slot by its index.
#[no_dynamic] #[no_dynamic]
fn get_slot(self, id: u32) -> Result<Value<'gc>, Error<'gc>> { #[inline(always)]
fn get_slot(self, id: u32) -> Value<'gc> {
let base = self.base(); let base = self.base();
base.get_slot(id) base.get_slot(id)
@ -562,19 +569,16 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
let value = self.vtable().coerce_trait_value(id, value, activation)?; let value = self.vtable().coerce_trait_value(id, value, activation)?;
let base = self.base(); let base = self.base();
base.set_slot(id, value, activation.gc()) base.set_slot(id, value, activation.gc());
Ok(())
} }
#[no_dynamic] #[no_dynamic]
fn set_slot_no_coerce( fn set_slot_no_coerce(self, id: u32, value: Value<'gc>, mc: &Mutation<'gc>) {
self,
id: u32,
value: Value<'gc>,
mc: &Mutation<'gc>,
) -> Result<(), Error<'gc>> {
let base = self.base(); let base = self.base();
base.set_slot(id, value, mc) base.set_slot(id, value, mc);
} }
/// Call a method by its index. /// Call a method by its index.
@ -1029,7 +1033,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
for (name, prop) in vtable.public_properties() { for (name, prop) in vtable.public_properties() {
match prop { match prop {
Property::Slot { slot_id } | Property::ConstSlot { slot_id } => { Property::Slot { slot_id } | Property::ConstSlot { slot_id } => {
values.push((name, self.base().get_slot(slot_id)?)); values.push((name, self.base().get_slot(slot_id)));
} }
Property::Virtual { get: Some(get), .. } => { Property::Virtual { get: Some(get), .. } => {
values.push((name, self.call_method(get, &[], activation)?)) values.push((name, self.call_method(get, &[], activation)?))

View File

@ -68,7 +68,7 @@ impl<'gc> ErrorObject<'gc> {
// by hardcoded slot id. Our `Error` class definition should fully match // by hardcoded slot id. Our `Error` class definition should fully match
// Flash Player, and we have lots of test coverage around error, so // Flash Player, and we have lots of test coverage around error, so
// there should be very little risk to doing this. // there should be very little risk to doing this.
let name = match self.base().get_slot(0)? { let name = match self.base().get_slot(0) {
Value::String(string) => string, Value::String(string) => string,
Value::Null => "null".into(), Value::Null => "null".into(),
Value::Undefined => "undefined".into(), Value::Undefined => "undefined".into(),
@ -78,7 +78,7 @@ impl<'gc> ErrorObject<'gc> {
)) ))
} }
}; };
let message = match self.base().get_slot(1)? { let message = match self.base().get_slot(1) {
Value::String(string) => string, Value::String(string) => string,
Value::Null => "null".into(), Value::Null => "null".into(),
Value::Undefined => "undefined".into(), Value::Undefined => "undefined".into(),

View File

@ -316,32 +316,28 @@ impl<'gc> ScriptObjectWrapper<'gc> {
} }
} }
pub fn get_slot(&self, id: u32) -> Result<Value<'gc>, Error<'gc>> { #[inline(always)]
pub fn get_slot(&self, id: u32) -> Value<'gc> {
self.0 self.0
.slots .slots
.get(id as usize) .get(id as usize)
.cloned() .cloned()
.map(|s| s.get()) .map(|s| s.get())
.ok_or_else(|| format!("Slot index {id} out of bounds!").into()) .expect("Slot index out of bounds")
} }
/// Set a slot by its index. /// Set a slot by its index.
pub fn set_slot( pub fn set_slot(&self, id: u32, value: Value<'gc>, mc: &Mutation<'gc>) {
&self, let slot = self
id: u32, .0
value: Value<'gc>, .slots
mc: &Mutation<'gc>, .get(id as usize)
) -> Result<(), Error<'gc>> { .expect("Slot index out of bounds");
if let Some(slot) = self.0.slots.get(id as usize) {
Gc::write(mc, self.0); Gc::write(mc, self.0);
// SAFETY: We just triggered a write barrier on the Gc. // SAFETY: We just triggered a write barrier on the Gc.
let slot_write = unsafe { Write::assume(slot) }; let slot_write = unsafe { Write::assume(slot) };
slot_write.unlock().set(value); slot_write.unlock().set(value);
Ok(())
} else {
Err(format!("Slot index {id} out of bounds!").into())
}
} }
/// Retrieve a bound method from the method table. /// Retrieve a bound method from the method table.

View File

@ -364,7 +364,7 @@ impl Avm2ObjectWindow {
label_col(&mut row); label_col(&mut row);
row.col(|ui| { row.col(|ui| {
let value = object.get_slot(slot_id); let value = object.get_slot(slot_id);
ValueResultWidget::new(activation, value).show(ui, messages); ValueResultWidget::new(activation, Ok(value)).show(ui, messages);
}); });
row.col(|_| {}); row.col(|_| {});
}); });