avm1: Switch to SmallVec for ActionPush

While switching swf to return slices, I noticed ActionPush was
potentially allocating a huge vector by mistake.
Switch to SmallVec<[Value; 4]> to fix this and avoid the
allocation in general (this was fairly high up in the profiler).

TODO: Return an iterator instead of any sort of vec.

Cargo.lock

@@ -3397,6 +3397,7 @@ dependencies = [
  "log",
  "num-derive",
  "num-traits",
+ "smallvec 1.4.2",
  "xz2",
 ]
 

swf/Cargo.toml

@@ -16,6 +16,7 @@ num-derive = "0.3"
 num-traits = "0.2"
 libflate = {version = "1.0", optional = true}
 log = "0.4"
+smallvec = "1.4.2"
 flate2 = {version = "1.0", optional = true}
 xz2 = {version = "0.1.6", optional = true}
 
@@ -24,4 +25,4 @@ approx = "0.4.0"
 
 [features]
 default = ["flate2"]
-lzma = ["xz2"]
+lzma = ["xz2"]

swf/src/avm1/read.rs

@@ -4,6 +4,7 @@ use crate::avm1::opcode::OpCode;
 use crate::avm1::types::*;
 use crate::error::{Error, Result};
 use crate::read::SwfRead;
+use smallvec::SmallVec;
 use std::io::Cursor;
 
 #[allow(dead_code)]
@@ -282,7 +283,7 @@ impl<'a> Reader<'a> {
 
     fn read_push(&mut self, length: usize) -> Result<Action<'a>> {
         let end_pos = self.pos() + length;
-        let mut values = Vec::with_capacity(end_pos);
+        let mut values = SmallVec::new();
         while self.pos() < end_pos {
             values.push(self.read_push_value()?);
         }
@@ -390,12 +391,21 @@ impl<'a> Reader<'a> {
 pub mod tests {
     use super::*;
     use crate::test_data;
+    use smallvec::smallvec;
 
     #[test]
     fn read_action() {
         for (swf_version, expected_action, action_bytes) in test_data::avm1_tests() {
+            // TODO: Limitations in SmallVec prevent this from compiling when it should be safe.
+            // SmallVec is invariant over T, when it should be covariant.
+            // This code works with Vec, which is properly covariant.
+            // see https://github.com/servo/rust-smallvec/issues/146
+            // This should be fixed when const generics are stable.
+            // This code should be safe and it's just for testing.
+            let expected_action: Action<'_> = unsafe { std::mem::transmute(expected_action) };
+
             let mut reader = Reader::new(&action_bytes[..], swf_version);
-            let parsed_action = reader.read_action().unwrap().unwrap();
+            let parsed_action: Action<'_> = reader.read_action().unwrap().unwrap();
             if parsed_action != expected_action {
                 // Failed, result doesn't match.
                 panic!(
@@ -411,7 +421,8 @@ pub mod tests {
     fn read_parse_error() {
         let action_bytes = [0xff, 0xff, 0xff, 0x00, 0x00];
         let mut reader = Reader::new(&action_bytes[..], 5);
-        match reader.read_action() {
+        let action = reader.read_action();
+        match action {
             Err(crate::error::Error::Avm1ParseError { .. }) => (),
             result => {
                 panic!("Expected Avm1ParseError, got {:?}", result);
@@ -440,7 +451,7 @@ pub mod tests {
         if let Action::DefineFunction { actions, .. } = action {
             let mut reader = Reader::new(actions, 5);
             let action = reader.read_action().unwrap().unwrap();
-            assert_eq!(action, Action::Push(vec![Value::Str("test")]));
+            assert_eq!(action, Action::Push(smallvec![Value::Str("test")]));
         }
     }
 
@@ -451,6 +462,9 @@ pub mod tests {
         let action_bytes = [0x96, 2, 0, 2, 3, 3]; // Extra 3 at the end shouldn't be read.
         let mut reader = Reader::new(&action_bytes[..], 5);
         let action = reader.read_action().unwrap().unwrap();
-        assert_eq!(action, Action::Push(vec![Value::Null, Value::Undefined]));
+        assert_eq!(
+            action,
+            Action::Push(smallvec![Value::Null, Value::Undefined])
+        );
     }
 }

swf/src/avm1/types.rs

@@ -1,3 +1,5 @@
+use smallvec::SmallVec;
+
 #[derive(Clone, Debug, PartialEq)]
 pub enum Action<'a> {
     Add,
@@ -82,7 +84,7 @@ pub enum Action<'a> {
     Play,
     Pop,
     PreviousFrame,
-    Push(Vec<Value<'a>>),
+    Push(SmallVec<[Value<'a>; 4]>),
     PushDuplicate,
     RandomNumber,
     RemoveSprite,

swf/src/test_data.rs

@@ -8,6 +8,7 @@ use crate::read::tests::{read_tag_bytes_from_file, read_tag_bytes_from_file_with
 use crate::tag_code::TagCode;
 use crate::types::*;
 use crate::write::write_swf;
+use smallvec::smallvec;
 use std::fs::File;
 use std::vec::Vec;
 
@@ -2730,64 +2731,68 @@ pub fn avm1_tests() -> Vec<Avm1TestData> {
         (3, Action::PreviousFrame, vec![0x05]),
         (
             4,
-            Action::Push(vec![Value::Str("test")]),
+            Action::Push(smallvec![Value::Str("test")]),
             vec![0x96, 6, 0, 0, 116, 101, 115, 116, 0],
         ),
         (
             4,
-            Action::Push(vec![Value::Float(0.0)]),
+            Action::Push(smallvec![Value::Float(0.0)]),
             vec![0x96, 5, 0, 1, 0, 0, 0, 0],
         ),
         (
             5,
-            Action::Push(vec![Value::Double(1.5)]),
+            Action::Push(smallvec![Value::Double(1.5)]),
             vec![0x96, 9, 0, 6, 0, 0, 248, 63, 0, 0, 0, 0],
         ),
-        (5, Action::Push(vec![Value::Null]), vec![0x96, 1, 0, 2]),
-        (5, Action::Push(vec![Value::Undefined]), vec![0x96, 1, 0, 3]),
+        (5, Action::Push(smallvec![Value::Null]), vec![0x96, 1, 0, 2]),
         (
             5,
-            Action::Push(vec![Value::Null, Value::Undefined]),
+            Action::Push(smallvec![Value::Undefined]),
+            vec![0x96, 1, 0, 3],
+        ),
+        (
+            5,
+            Action::Push(smallvec![Value::Null, Value::Undefined]),
             vec![0x96, 2, 0, 2, 3],
         ),
         (
             5,
-            Action::Push(vec![Value::Register(1)]),
+            Action::Push(smallvec![Value::Register(1)]),
             vec![0x96, 2, 0, 4, 1],
         ),
         (
             5,
-            Action::Push(vec![Value::Bool(false)]),
+            Action::Push(smallvec![Value::Bool(false)]),
             vec![0x96, 2, 0, 5, 0],
         ),
         (
             5,
-            Action::Push(vec![Value::Bool(true)]),
+            Action::Push(smallvec![Value::Bool(true)]),
             vec![0x96, 2, 0, 5, 1],
         ),
         (
             5,
-            Action::Push(vec![Value::Double(0.0)]),
+            Action::Push(smallvec![Value::Double(0.0)]),
             vec![0x96, 9, 0, 6, 0, 0, 0, 0, 0, 0, 0, 0],
         ),
         (
             5,
-            Action::Push(vec![Value::Int(31)]),
+            Action::Push(smallvec![Value::Int(31)]),
             vec![0x96, 5, 0, 7, 31, 0, 0, 0],
         ),
         (
             5,
-            Action::Push(vec![Value::Int(-50)]),
+            Action::Push(smallvec![Value::Int(-50)]),
             vec![0x96, 5, 0, 7, 206, 255, 255, 255],
         ),
         (
             5,
-            Action::Push(vec![Value::ConstantPool(77)]),
+            Action::Push(smallvec![Value::ConstantPool(77)]),
             vec![0x96, 2, 0, 8, 77],
         ),
         (
             5,
-            Action::Push(vec![Value::ConstantPool(257)]),
+            Action::Push(smallvec![Value::ConstantPool(257)]),
             vec![0x96, 3, 0, 9, 1, 1],
         ),
         (4, Action::RandomNumber, vec![0x30]),