avm2: Make several slot operations faster

- `get_slot` and `set_slot_no_coerce` now panic instead of returning Err
- `get_slot` is now inlined

core/src/avm2/activation.rs

@@ -1754,7 +1754,7 @@ impl<'a, 'gc> Activation<'a, '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 value = object.get_slot(index)?;
+        let value = object.get_slot(index);
 
         self.push_stack(value);
 
@@ -1774,7 +1774,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
         let value = self.pop_stack();
         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)
     }
@@ -1783,7 +1783,6 @@ impl<'a, 'gc> Activation<'a, 'gc> {
         let value = self
             .global_scope()
             .map(|global| global.get_slot(index))
-            .transpose()?
             .unwrap_or(Value::Undefined);
 
         self.push_stack(value);

core/src/avm2/object.rs

@@ -242,7 +242,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
     ) -> Result<Value<'gc>, Error<'gc>> {
         match self.vtable().get_trait(multiname) {
             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 }) => {
                 // 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
                     .vtable()
                     .coerce_trait_value(slot_id, value, activation)?;
+
                 self.base()
-                    .set_slot(slot_id, value, activation.context.gc_context)
+                    .set_slot(slot_id, value, activation.context.gc_context);
+
+                Ok(())
             }
             Some(Property::Method { .. }) => {
                 // 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
                     .vtable()
                     .coerce_trait_value(slot_id, value, activation)?;
+
                 self.base()
-                    .set_slot(slot_id, value, activation.context.gc_context)
+                    .set_slot(slot_id, value, activation.context.gc_context);
+
+                Ok(())
             }
             Some(Property::Method { .. }) => {
                 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>> {
         match self.vtable().get_trait(multiname) {
             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,
                     Some(multiname),
                     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.
     #[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();
 
         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 base = self.base();
 
-        base.set_slot(id, value, activation.gc())
+        base.set_slot(id, value, activation.gc());
+
+        Ok(())
     }
 
     #[no_dynamic]
-    fn set_slot_no_coerce(
-        self,
-        id: u32,
-        value: Value<'gc>,
-        mc: &Mutation<'gc>,
-    ) -> Result<(), Error<'gc>> {
+    fn set_slot_no_coerce(self, id: u32, value: Value<'gc>, mc: &Mutation<'gc>) {
         let base = self.base();
 
-        base.set_slot(id, value, mc)
+        base.set_slot(id, value, mc);
     }
 
     /// 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() {
             match prop {
                 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), .. } => {
                     values.push((name, self.call_method(get, &[], activation)?))

core/src/avm2/object/error_object.rs

@@ -68,7 +68,7 @@ impl<'gc> ErrorObject<'gc> {
         // by hardcoded slot id. Our `Error` class definition should fully match
         // Flash Player, and we have lots of test coverage around error, so
         // 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::Null => "null".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::Null => "null".into(),
             Value::Undefined => "undefined".into(),

core/src/avm2/object/script_object.rs

@@ -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
             .slots
             .get(id as usize)
             .cloned()
             .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.
-    pub fn set_slot(
-        &self,
-        id: u32,
-        value: Value<'gc>,
-        mc: &Mutation<'gc>,
-    ) -> Result<(), Error<'gc>> {
-        if let Some(slot) = self.0.slots.get(id as usize) {
-            Gc::write(mc, self.0);
-            // SAFETY: We just triggered a write barrier on the Gc.
-            let slot_write = unsafe { Write::assume(slot) };
-            slot_write.unlock().set(value);
+    pub fn set_slot(&self, id: u32, value: Value<'gc>, mc: &Mutation<'gc>) {
+        let slot = self
+            .0
+            .slots
+            .get(id as usize)
+            .expect("Slot index out of bounds");
 
-            Ok(())
-        } else {
-            Err(format!("Slot index {id} out of bounds!").into())
-        }
+        Gc::write(mc, self.0);
+        // SAFETY: We just triggered a write barrier on the Gc.
+        let slot_write = unsafe { Write::assume(slot) };
+        slot_write.unlock().set(value);
     }
 
     /// Retrieve a bound method from the method table.

core/src/debug_ui/avm2.rs

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