Rust micro optimisations

[kirushik]

This is a follow-up to my #16, and would only be ready for merging after that PR lands (would be glad to rebase/adjust as needed when that happens).

Here we're entering the questionable territory: hardcoding ASCII assumptions into the Calculator (so u8 can be used instead of char; in my defence, some other implementations also implicitly assume the same), reusing the same ci per row in Mandelbrot (should be fine, but still a logic change), non-copying (so potentially less multithreadable in the future) Option<&V> in Cache. Nothing too crazy (at least nothing I won't be willing to commit into production code myself), but your call in the end.

Commits in this PR are independently droppable, in case you object.

[kostya]

Thanks that good changes, I will write about all 3 pr.
I merged 10 commits from it into branch update_rust.

3 still not merging:
GameOfLife 3159ea4 - this is actually slower, and changing to flat array is micro optimization. current implementation with lifetime I think is ok and closest to the crystal version
lzw c0eec97 - questionable need to think about it more, all others use strings objects and moving to vector questionable. I need to think more about since C++ use vector, but crystal string.
parsedom 7f91477 - questionable, it is faster, but copying into vec and modify it, it not sure that what people would do in the production and prefer to stay with serde even if it slower. May be serde can copy magic from simd json and became faster later?

about calculator and u8, yes this is questinable, and I found that different languages used u8 or unicode char. so there is already inconsistence in implementations. which I would fix later over all languages

for me it is changed from 58.4416s to 53.4252s

[kostya]

funny note about Mandelbrot, your change should not affect performance because it is just LICM (which should be done by compiler easily), but it show some weak spot in the rust compiler. may be need to revert as it is micro optimization, we doing the work which compiler should.

[kirushik]

WRT the GameOfLife commit — I've tried once again, and I see ~10% improvement on my local machine. Might lead us into a moderately-interesting rabbithole about OS and CPU architectures and minor differences in Rust compiler versions?

But my bigger point is that the original code is just not sound; if you'd run it under Miri, you'll get a runtime failure both under StackedBorrows and TreeBorrows (-Zmiri-tree-borrows) aliasing model:

error: Undefined Behavior: trying to retag from <1146451> for SharedReadOnly permission
  at alloc384065[0x260], but that tag does not exist in the borrow stack
  --> src/benchmarks/game_of_life.rs:87:49
   |  let neighbor = unsafe { &*cells_ref[ny][nx] };
help: <1146451> was later invalidated by a Unique retag
  --> src/benchmarks/game_of_life.rs:75:46
   |  let cell = &mut self.cells[y][x];

[kostya]

about game of life, changing to flat array is not equal with any other lang. so most close idiomatic rust version should be:

diff --git a/rust/src/benchmarks/game_of_life.rs b/rust/src/benchmarks/game_of_life.rs
index e0de361..e96e30c 100644
--- a/rust/src/benchmarks/game_of_life.rs
+++ b/rust/src/benchmarks/game_of_life.rs
@@ -1,13 +1,13 @@
 use super::super::{helper, Benchmark};
 use crate::config_i64;

-struct Cell<'a> {
+struct Cell {
     alive: bool,
     next_state: bool,
-    neighbors: Vec<&'a Cell<'a>>,
+    neighbors: Vec<(usize, usize)>,
 }

-impl<'a> Cell<'a> {
+impl Cell {
     fn new(alive: bool) -> Self {
         Self {
             alive,
@@ -16,17 +16,21 @@ impl<'a> Cell<'a> {
         }
     }

-    fn add_neighbor(&mut self, cell: &'a Cell<'a>) {
-        self.neighbors.push(cell);
+    fn add_neighbor(&mut self, x: usize, y: usize) {
+        self.neighbors.push((x, y));
     }

-    fn compute_next_state(&mut self) {
-        let alive_neighbors = self.neighbors.iter().filter(|n| n.alive).count();
+    fn compute_next_state(&self, cells: &[Vec<Cell>]) -> bool {
+        let alive_neighbors = self
+            .neighbors
+            .iter()
+            .filter(|&&(x, y)| cells[y][x].alive)
+            .count();

         if self.alive {
-            self.next_state = alive_neighbors == 2 || alive_neighbors == 3
+            alive_neighbors == 2 || alive_neighbors == 3
         } else {
-            self.next_state = alive_neighbors == 3
+            alive_neighbors == 3
         }
     }

@@ -35,47 +39,31 @@ impl<'a> Cell<'a> {
     }
 }

-struct Grid<'a> {
+struct Grid {
     width: usize,
     height: usize,
-    cells: Vec<Vec<Cell<'a>>>,
+    cells: Vec<Vec<Cell>>,
 }

-impl<'a> Grid<'a> {
+impl Grid {
     fn new(width: usize, height: usize) -> Self {
         let mut grid = Grid {
             width,
             height,
-            cells: Vec::with_capacity(height),
+            cells: (0..height)
+                .map(|_| (0..width).map(|_| Cell::new(false)).collect())
+                .collect(),
         };

-        for _ in 0..height {
-            let mut row = Vec::with_capacity(width);
-            for _ in 0..width {
-                row.push(Cell::new(false));
-            }
-            grid.cells.push(row);
-        }
-
         grid.link_neighbors();
         grid
     }

     fn link_neighbors(&mut self) {
-        let cells_ref: Vec<Vec<*const Cell<'a>>> = (0..self.height)
-            .map(|y| {
-                (0..self.width)
-                    .map(|x| &self.cells[y][x] as *const Cell)
-                    .collect()
-            })
-            .collect();
-
         for y in 0..self.height {
             for x in 0..self.width {
-                let cell = &mut self.cells[y][x];
-
-                for dy in -1..=1 {
-                    for dx in -1..=1 {
+                for dy in -1..=1_i32 {
+                    for dx in -1..=1_i32 {
                         if dx == 0 && dy == 0 {
                             continue;
                         }
@@ -84,8 +72,7 @@ impl<'a> Grid<'a> {
                             ((y as i32 + dy + self.height as i32) % self.height as i32) as usize;
                         let nx = ((x as i32 + dx + self.width as i32) % self.width as i32) as usize;

-                        let neighbor = unsafe { &*cells_ref[ny][nx] };
-                        cell.add_neighbor(neighbor);
+                        self.cells[y][x].add_neighbor(nx, ny);
                     }
                 }
             }
@@ -93,16 +80,18 @@ impl<'a> Grid<'a> {
     }

     fn next_generation(&mut self) {
-        self.cells.iter_mut().for_each(|row| {
-            row.iter_mut().for_each(|cell| {
-                cell.compute_next_state();
-            });
-        });
-        self.cells.iter_mut().for_each(|row| {
-            row.iter_mut().for_each(|cell| {
+        for y in 0..self.height {
+            for x in 0..self.width {
+                let next_state = self.cells[y][x].compute_next_state(&self.cells);
+                self.cells[y][x].next_state = next_state;
+            }
+        }
+
+        for row in &mut self.cells {
+            for cell in row {
                 cell.update();
-            });
-        });
+            }
+        }
     }

     fn count_alive(&self) -> u32 {
@@ -128,7 +117,7 @@ impl<'a> Grid<'a> {
 }

 pub struct GameOfLife {
-    grid: Grid<'static>,
+    grid: Grid,
 }

 impl GameOfLife {

but this version gives 1.7s, vs 0.87s in lifetimes version.