Remove baseline benchmark and switch to tile size 16

Fix potential over-the-bounds slicing in rotate.rs introduced in #474

codecs/rotate/.gitignore

@@ -0,0 +1,2 @@
+target
+Cargo.lock

codecs/rotate/Cargo.toml

@@ -0,0 +1,14 @@
+[package]
+name = "rotate"
+version = "0.1.0"
+authors = ["Surma <surma@google.com>"]
+edition = "2018"
+
+[lib]
+name = "rotate"
+path = "rotate.rs"
+crate-type = ["cdylib", "rlib"]
+
+[profile.release]
+lto = true
+opt-level = "s"

codecs/rotate/benchmark.js

@@ -0,0 +1,45 @@
+// THIS IS NOT A NODE SCRIPT
+// This is a d8 script. Please install jsvu[1] and install v8.
+// Then run `npm run --silent benchmark`.
+// [1]: https://github.com/GoogleChromeLabs/jsvu
+async function init() {
+  //  Adjustable constants.
+  const imageDimensions = 4096;
+  const iterations = new Array(100);
+
+  // Constants. Don’t change.
+  const imageByteSize = imageDimensions * imageDimensions * 4;
+  const wasmPageSize = 64 * 1024;
+
+  const buffer = readbuffer("rotate.wasm");
+  const { instance } = await WebAssembly.instantiate(buffer);
+
+  const pagesAvailable = Math.floor(
+    instance.exports.memory.buffer.byteLength / wasmPageSize
+  );
+  const pagesNeeded = Math.floor((imageByteSize * 2 + 4) / wasmPageSize) + 1;
+  const additionalPagesNeeded = pagesNeeded - pagesAvailable;
+  if (additionalPagesNeeded > 0) {
+    instance.exports.memory.grow(additionalPagesNeeded);
+  }
+
+  [0, 90, 180, 270].forEach(rotation => {
+    print(`\n${rotation} degrees`);
+    print(`==============================`);
+    for (let i = 0; i < 100; i++) {
+      const start = Date.now();
+      instance.exports.rotate(imageDimensions, imageDimensions, rotation);
+      iterations[i] = Date.now() - start;
+    }
+    const average = iterations.reduce((sum, c) => sum + c) / iterations.length;
+    const stddev = Math.sqrt(
+      iterations
+        .map(i => Math.pow(i - average, 2))
+        .reduce((sum, c) => sum + c) / iterations.length
+    );
+    print(`n = ${iterations.length}`);
+    print(`Average: ${average}`);
+    print(`StdDev: ${stddev}`);
+  });
+}
+init().catch(e => console.error(e.stack));

codecs/rotate/build.sh

@@ -7,12 +7,11 @@ echo "Compiling wasm"
 echo "============================================="
 (
   rustup run nightly \
-    rustc \
-    --target=wasm32-unknown-unknown \
-    -C opt-level=3 \
-    -o rotate.wasm \
-    rotate.rs
-    wasm-strip rotate.wasm
+    cargo build \
+    --target wasm32-unknown-unknown \
+    --release
+  cp target/wasm32-unknown-unknown/release/rotate.wasm .
+  wasm-strip rotate.wasm
 )
 echo "============================================="
 echo "Compiling wasm  done"

codecs/rotate/package.json

@@ -2,6 +2,10 @@
   "name": "rotate",
   "scripts": {
     "build:image": "docker build -t squoosh-rotate .",
-    "build": "docker run --rm -v $(pwd):/src squoosh-rotate ./build.sh"
+    "build": "docker run --rm -v $(pwd):/src squoosh-rotate ./build.sh",
+    "benchmark": "echo File size after gzip && npm run benchmark:filesize && echo Optimizing && npm run -s benchmark:optimizing",
+    "benchmark:baseline": "v8 --liftoff --no-wasm-tier-up --no-opt ./benchmark.js",
+    "benchmark:optimizing": "v8 --no-liftoff --no-wasm-tier-up ./benchmark.js",
+    "benchmark:filesize": "cat rotate.wasm | gzip -c9n | wc -c"
   }
 }

codecs/rotate/rotate.rs

@@ -1,81 +1,113 @@
-#![no_std]
-#![no_main]
+use std::slice::{from_raw_parts, from_raw_parts_mut};
 
-use core::panic::PanicInfo;
-use core::slice::from_raw_parts_mut;
+// This function is taken from Zachary Dremann
+// https://github.com/GoogleChromeLabs/squoosh/pull/462
+trait HardUnwrap<T> {
+    fn unwrap_hard(self) -> T;
+}
+
+impl<T> HardUnwrap<T> for Option<T> {
+    #[cfg(not(debug_assertions))]
+    #[inline]
+    fn unwrap_hard(self) -> T {
+        match self {
+            Some(t) => t,
+            None => std::process::abort(),
+        }
+    }
+
+    #[cfg(debug_assertions)]
+    fn unwrap_hard(self) -> T {
+        self.unwrap()
+    }
+}
+
+const TILE_SIZE: usize = 16;
+
+fn get_buffers<'a>(width: usize, height: usize) -> (&'a [u32], &'a mut [u32]) {
+    let num_pixels = width * height;
+    let in_b: &[u32];
+    let out_b: &mut [u32];
+    unsafe {
+        in_b = from_raw_parts::<u32>(8 as *const u32, num_pixels);
+        out_b = from_raw_parts_mut::<u32>((num_pixels * 4 + 8) as *mut u32, num_pixels);
+    }
+    return (in_b, out_b);
+}
+
+#[inline(never)]
+fn rotate_0(width: usize, height: usize) {
+    let (in_b, out_b) = get_buffers(width, height);
+    for (in_p, out_p) in in_b.iter().zip(out_b.iter_mut()) {
+        *out_p = *in_p;
+    }
+}
+
+#[inline(never)]
+fn rotate_90(width: usize, height: usize) {
+    let (in_b, out_b) = get_buffers(width, height);
+    let new_width = height;
+    let _new_height = width;
+    for y_start in (0..height).step_by(TILE_SIZE) {
+        for x_start in (0..width).step_by(TILE_SIZE) {
+            for y in y_start..(y_start + TILE_SIZE).min(height) {
+                let in_offset = y * width;
+                let in_bounds = if x_start + TILE_SIZE < width {
+                    (in_offset + x_start)..(in_offset + x_start + TILE_SIZE)
+                } else {
+                    (in_offset + x_start)..(in_offset + width)
+                };
+                let in_chunk = in_b.get(in_bounds).unwrap_hard();
+                for (x, in_p) in in_chunk.iter().enumerate() {
+                    let new_x = (new_width - 1) - y;
+                    let new_y = x + x_start;
+                    *out_b.get_mut(new_y * new_width + new_x).unwrap_hard() = *in_p;
+                }
+            }
+        }
+    }
+}
+
+#[inline(never)]
+fn rotate_180(width: usize, height: usize) {
+    let (in_b, out_b) = get_buffers(width, height);
+    for (in_p, out_p) in in_b.iter().zip(out_b.iter_mut().rev()) {
+        *out_p = *in_p;
+    }
+}
+
+#[inline(never)]
+fn rotate_270(width: usize, height: usize) {
+    let (in_b, out_b) = get_buffers(width, height);
+    let new_width = height;
+    let new_height = width;
+    for y_start in (0..height).step_by(TILE_SIZE) {
+        for x_start in (0..width).step_by(TILE_SIZE) {
+            for y in y_start..(y_start + TILE_SIZE).min(height) {
+                let in_offset = y * width;
+                let in_bounds = if x_start + TILE_SIZE < width {
+                    (in_offset + x_start)..(in_offset + x_start + TILE_SIZE)
+                } else {
+                    (in_offset + x_start)..(in_offset + width)
+                };
+                let in_chunk = in_b.get(in_bounds).unwrap_hard();
+                for (x, in_p) in in_chunk.iter().enumerate() {
+                    let new_x = y;
+                    let new_y = new_height - 1 - (x_start + x);
+                    *out_b.get_mut(new_y * new_width + new_x).unwrap_hard() = *in_p;
+                }
+            }
+        }
+    }
+}
 
 #[no_mangle]
-fn rotate(input_width: isize, input_height: isize, rotate: isize) {
-  let mut i = 0isize;
-
-  // In the straight-copy case, d1 is x, d2 is y.
-  // x starts at 0 and increases.
-  // y starts at 0 and increases.
-  let mut d1_start: isize = 0;
-  let mut d1_limit: isize = input_width;
-  let mut d1_advance: isize = 1;
-  let mut d1_multiplier: isize = 1;
-  let mut d2_start: isize = 0;
-  let mut d2_limit: isize = input_height;
-  let mut d2_advance: isize = 1;
-  let mut d2_multiplier: isize = input_width;
-
-  if rotate == 90 {
-    // d1 is y, d2 is x.
-    // y starts at its max value and decreases.
-    // x starts at 0 and increases.
-    d1_start = input_height - 1;
-    d1_limit = input_height;
-    d1_advance = -1;
-    d1_multiplier = input_width;
-    d2_start = 0;
-    d2_limit = input_width;
-    d2_advance = 1;
-    d2_multiplier = 1;
-  } else if rotate == 180 {
-    // d1 is x, d2 is y.
-    // x starts at its max and decreases.
-    // y starts at its max and decreases.
-    d1_start = input_width - 1;
-    d1_limit = input_width;
-    d1_advance = -1;
-    d1_multiplier = 1;
-    d2_start = input_height - 1;
-    d2_limit = input_height;
-    d2_advance = -1;
-    d2_multiplier = input_width;
-  } else if rotate == 270 {
-    // d1 is y, d2 is x.
-    // y starts at 0 and increases.
-    // x starts at its max and decreases.
-    d1_start = 0;
-    d1_limit = input_height;
-    d1_advance = 1;
-    d1_multiplier = input_width;
-    d2_start = input_width - 1;
-    d2_limit = input_width;
-    d2_advance = -1;
-    d2_multiplier = 1;
-  }
-
-  let num_pixels = (input_width * input_height) as usize;
-  let in_b: &mut [u32];
-  let out_b: &mut [u32];
-  unsafe {
-    in_b = from_raw_parts_mut::<u32>(4 as *mut u32, num_pixels);
-    out_b = from_raw_parts_mut::<u32>((input_width * input_height * 4 + 4) as *mut u32, num_pixels);
-  }
-
-  for d2 in 0..d2_limit {
-    for d1 in 0..d1_limit {
-      let in_idx = (d1_start + d1 * d1_advance) * d1_multiplier + (d2_start + d2 * d2_advance) * d2_multiplier;
-      out_b[i as usize] = in_b[in_idx as usize];
-      i += 1;
+fn rotate(width: usize, height: usize, rotate: usize) {
+    match rotate {
+        0 => rotate_0(width, height),
+        90 => rotate_90(width, height),
+        180 => rotate_180(width, height),
+        270 => rotate_270(width, height),
+        _ => std::process::abort(),
     }
-  }
-}
-
-#[panic_handler]
-fn panic(_info: &PanicInfo) -> ! {
-  loop {}
 }

package.json

@@ -1,7 +1,7 @@
 {
   "private": true,
   "name": "squoosh",
-  "version": "1.3.2",
+  "version": "1.3.3",
   "license": "apache-2.0",
   "scripts": {
     "start": "webpack-dev-server --host 0.0.0.0 --hot",
@@ -16,19 +16,19 @@
     }
   },
   "devDependencies": {
-    "@types/node": "10.12.23",
+    "@types/node": "10.12.26",
     "@types/pretty-bytes": "5.1.0",
     "@types/webassembly-js-api": "0.0.2",
     "@webcomponents/custom-elements": "1.2.1",
     "@webpack-cli/serve": "0.1.3",
     "assets-webpack-plugin": "3.9.7",
-    "chokidar": "2.1.0",
+    "chokidar": "2.1.2",
     "chalk": "2.4.2",
     "classnames": "2.2.6",
     "clean-webpack-plugin": "1.0.1",
     "comlink": "3.1.1",
-    "copy-webpack-plugin": "4.6.0",
-    "critters-webpack-plugin": "2.2.0",
+    "copy-webpack-plugin": "5.0.0",
+    "critters-webpack-plugin": "2.3.0",
     "css-loader": "1.0.1",
     "ejs": "2.6.1",
     "escape-string-regexp": "1.0.5",
@@ -67,9 +67,9 @@
     "typescript": "3.2.4",
     "url-loader": "1.1.2",
     "webpack": "4.28.0",
-    "webpack-bundle-analyzer": "3.0.3",
+    "webpack-bundle-analyzer": "3.0.4",
     "webpack-cli": "3.2.3",
-    "webpack-dev-server": "3.1.14",
-    "worker-plugin": "3.0.0"
+    "webpack-dev-server": "3.2.0",
+    "worker-plugin": "3.1.0"
   }
 }

src/codecs/rotate/processor.ts

@@ -11,7 +11,7 @@ export async function rotate(
 
   // Number of wasm memory pages (á 64KiB) needed to store the image twice.
   const bytesPerImage = data.width * data.height * 4;
-  const numPagesNeeded = Math.ceil((bytesPerImage * 2 + 4) / (64 * 1024));
+  const numPagesNeeded = Math.ceil((bytesPerImage * 2 + 8) / (64 * 1024));
   // Only count full pages, just to be safe.
   const numPagesAvailable = Math.floor(instance.exports.memory.buffer.byteLength / (64 * 1024));
   const additionalPagesToAllocate = numPagesNeeded - numPagesAvailable;
@@ -20,13 +20,13 @@ export async function rotate(
     instance.exports.memory.grow(additionalPagesToAllocate);
   }
   const view = new Uint8ClampedArray(instance.exports.memory.buffer);
-  view.set(data.data, 4);
+  view.set(data.data, 8);
 
   instance.exports.rotate(data.width, data.height, opts.rotate);
 
   const flipDimensions = opts.rotate % 180 !== 0;
   return new ImageData(
-    view.slice(bytesPerImage + 4, bytesPerImage * 2 + 4),
+    view.slice(bytesPerImage + 8, bytesPerImage * 2 + 8),
     flipDimensions ? data.height : data.width,
     flipDimensions ? data.width : data.height,
   );