camera: make rendering strategy configurable, add workerpool version.

rtchallenge/Cargo.lock

@@ -1,11 +1,35 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
+[[package]]
+name = "addr2line"
+version = "0.15.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e7a2e47a1fbe209ee101dd6d61285226744c6c8d3c21c8dc878ba6cb9f467f3a"
+dependencies = [
+ "gimli",
+]
+
+[[package]]
+name = "adler"
+version = "1.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe"
+
 [[package]]
 name = "adler32"
 version = "1.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "aae1277d39aeec15cb388266ecc24b11c80469deae6067e17a1a7aa9e5c1f234"
 
+[[package]]
+name = "ansi_term"
+version = "0.11.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b"
+dependencies = [
+ "winapi 0.3.9",
+]
+
 [[package]]
 name = "anyhow"
 version = "1.0.41"
@@ -20,7 +44,7 @@ checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8"
 dependencies = [
  "hermit-abi",
  "libc",
- "winapi",
+ "winapi 0.3.9",
 ]
 
 [[package]]
@@ -29,6 +53,21 @@ version = "1.0.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a"
 
+[[package]]
+name = "backtrace"
+version = "0.3.60"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b7815ea54e4d821e791162e078acbebfd6d8c8939cd559c9335dceb1c8ca7282"
+dependencies = [
+ "addr2line",
+ "cc",
+ "cfg-if",
+ "libc",
+ "miniz_oxide 0.4.4",
+ "object",
+ "rustc-demangle",
+]
+
 [[package]]
 name = "bitflags"
 version = "1.2.1"
@@ -68,6 +107,12 @@ dependencies = [
  "rustc_version",
 ]
 
+[[package]]
+name = "cc"
+version = "1.0.69"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e70cc2f62c6ce1868963827bd677764c62d07c3d9a3e1fb1177ee1a9ab199eb2"
+
 [[package]]
 name = "cfg-if"
 version = "1.0.0"
@@ -80,9 +125,25 @@ version = "2.33.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "37e58ac78573c40708d45522f0d80fa2f01cc4f9b4e2bf749807255454312002"
 dependencies = [
+ "ansi_term",
+ "atty",
  "bitflags",
+ "strsim",
  "textwrap",
  "unicode-width",
+ "vec_map",
+]
+
+[[package]]
+name = "core_affinity"
+version = "0.5.10"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7f8a03115cc34fb0d7c321dd154a3914b3ca082ccc5c11d91bf7117dbbe7171f"
+dependencies = [
+ "kernel32-sys",
+ "libc",
+ "num_cpus",
+ "winapi 0.2.8",
 ]
 
 [[package]]
@@ -212,12 +273,60 @@ version = "1.6.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457"
 
+[[package]]
+name = "enum-utils"
+version = "0.1.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ed327f716d0d351d86c9fd3398d20ee39ad8f681873cc081da2ca1c10fed398a"
+dependencies = [
+ "enum-utils-from-str",
+ "failure",
+ "proc-macro2",
+ "quote",
+ "serde_derive_internals",
+ "syn",
+]
+
+[[package]]
+name = "enum-utils-from-str"
+version = "0.1.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d49be08bad6e4ca87b2b8e74146987d4e5cb3b7512efa50ef505b51a22227ee1"
+dependencies = [
+ "proc-macro2",
+ "quote",
+]
+
+[[package]]
+name = "failure"
+version = "0.1.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d32e9bd16cc02eae7db7ef620b392808b89f6a5e16bb3497d159c6b92a0f4f86"
+dependencies = [
+ "backtrace",
+]
+
+[[package]]
+name = "gimli"
+version = "0.24.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0e4075386626662786ddb0ec9081e7c7eeb1ba31951f447ca780ef9f5d568189"
+
 [[package]]
 name = "half"
 version = "1.7.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "62aca2aba2d62b4a7f5b33f3712cb1b0692779a56fb510499d5c0aa594daeaf3"
 
+[[package]]
+name = "heck"
+version = "0.3.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6d621efb26863f0e9924c6ac577e8275e5e6b77455db64ffa6c65c904e9e132c"
+dependencies = [
+ "unicode-segmentation",
+]
+
 [[package]]
 name = "hermit-abi"
 version = "0.1.19"
@@ -260,6 +369,16 @@ dependencies = [
  "wasm-bindgen",
 ]
 
+[[package]]
+name = "kernel32-sys"
+version = "0.2.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7507624b29483431c0ba2d82aece8ca6cdba9382bff4ddd0f7490560c056098d"
+dependencies = [
+ "winapi 0.2.8",
+ "winapi-build",
+]
+
 [[package]]
 name = "lazy_static"
 version = "1.4.0"
@@ -305,6 +424,16 @@ dependencies = [
  "adler32",
 ]
 
+[[package]]
+name = "miniz_oxide"
+version = "0.4.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a92518e98c078586bc6c934028adcca4c92a53d6a958196de835170a01d84e4b"
+dependencies = [
+ "adler",
+ "autocfg",
+]
+
 [[package]]
 name = "num-traits"
 version = "0.2.14"
@@ -324,6 +453,15 @@ dependencies = [
  "libc",
 ]
 
+[[package]]
+name = "object"
+version = "0.25.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a38f2be3697a57b4060074ff41b44c16870d916ad7877c17696e063257482bc7"
+dependencies = [
+ "memchr",
+]
+
 [[package]]
 name = "oorandom"
 version = "11.1.3"
@@ -367,7 +505,31 @@ dependencies = [
  "bitflags",
  "crc32fast",
  "deflate",
- "miniz_oxide",
+ "miniz_oxide 0.3.7",
+]
+
+[[package]]
+name = "proc-macro-error"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c"
+dependencies = [
+ "proc-macro-error-attr",
+ "proc-macro2",
+ "quote",
+ "syn",
+ "version_check",
+]
+
+[[package]]
+name = "proc-macro-error-attr"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "version_check",
 ]
 
 [[package]]
@@ -439,12 +601,24 @@ name = "rtchallenge"
 version = "0.1.0"
 dependencies = [
  "anyhow",
+ "core_affinity",
  "criterion",
+ "enum-utils",
+ "num_cpus",
  "png",
  "rayon",
+ "serde",
+ "serde_json",
+ "structopt",
  "thiserror",
 ]
 
+[[package]]
+name = "rustc-demangle"
+version = "0.1.20"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "dead70b0b5e03e9c814bcb6b01e03e68f7c57a80aa48c72ec92152ab3e818d49"
+
 [[package]]
 name = "rustc_version"
 version = "0.4.0"
@@ -486,6 +660,9 @@ name = "serde"
 version = "1.0.126"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ec7505abeacaec74ae4778d9d9328fe5a5d04253220a85c4ee022239fc996d03"
+dependencies = [
+ "serde_derive",
+]
 
 [[package]]
 name = "serde_cbor"
@@ -508,6 +685,17 @@ dependencies = [
  "syn",
 ]
 
+[[package]]
+name = "serde_derive_internals"
+version = "0.25.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1dbab34ca63057a1f15280bdf3c39f2b1eb1b54c17e98360e511637aef7418c6"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "serde_json"
 version = "1.0.64"
@@ -519,6 +707,36 @@ dependencies = [
  "serde",
 ]
 
+[[package]]
+name = "strsim"
+version = "0.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a"
+
+[[package]]
+name = "structopt"
+version = "0.3.22"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "69b041cdcb67226aca307e6e7be44c8806423d83e018bd662360a93dabce4d71"
+dependencies = [
+ "clap",
+ "lazy_static",
+ "structopt-derive",
+]
+
+[[package]]
+name = "structopt-derive"
+version = "0.4.15"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7813934aecf5f51a54775e00068c237de98489463968231a51746bbbc03f9c10"
+dependencies = [
+ "heck",
+ "proc-macro-error",
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "syn"
 version = "1.0.73"
@@ -569,6 +787,12 @@ dependencies = [
  "serde_json",
 ]
 
+[[package]]
+name = "unicode-segmentation"
+version = "1.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8895849a949e7845e06bd6dc1aa51731a103c42707010a5b591c0038fb73385b"
+
 [[package]]
 name = "unicode-width"
 version = "0.1.8"
@@ -581,6 +805,18 @@ version = "0.2.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3"
 
+[[package]]
+name = "vec_map"
+version = "0.8.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f1bddf1187be692e79c5ffeab891132dfb0f236ed36a43c7ed39f1165ee20191"
+
+[[package]]
+name = "version_check"
+version = "0.9.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5fecdca9a5291cc2b8dcf7dc02453fee791a280f3743cb0905f8822ae463b3fe"
+
 [[package]]
 name = "walkdir"
 version = "2.3.2"
@@ -588,7 +824,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "808cf2735cd4b6866113f648b791c6adc5714537bc222d9347bb203386ffda56"
 dependencies = [
  "same-file",
- "winapi",
+ "winapi 0.3.9",
  "winapi-util",
 ]
 
@@ -656,6 +892,12 @@ dependencies = [
  "wasm-bindgen",
 ]
 
+[[package]]
+name = "winapi"
+version = "0.2.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "167dc9d6949a9b857f3451275e911c3f44255842c1f7a76f33c55103a909087a"
+
 [[package]]
 name = "winapi"
 version = "0.3.9"
@@ -666,6 +908,12 @@ dependencies = [
  "winapi-x86_64-pc-windows-gnu",
 ]
 
+[[package]]
+name = "winapi-build"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2d315eee3b34aca4797b2da6b13ed88266e6d612562a0c46390af8299fc699bc"
+
 [[package]]
 name = "winapi-i686-pc-windows-gnu"
 version = "0.4.0"
@@ -678,7 +926,7 @@ version = "0.1.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "70ec6ce85bb158151cae5e5c87f95a8e97d2c0c4b001223f33a334e3ce5de178"
 dependencies = [
- "winapi",
+ "winapi 0.3.9",
 ]
 
 [[package]]

rtchallenge/Cargo.toml

@@ -11,9 +11,15 @@ disable_inverse_cache = []
 
 [dependencies]
 anyhow = "1.0.41"
+core_affinity = "0.5.10"
 criterion = "0.3.4"
+enum-utils = "0.1.2"
+num_cpus = "1.13.0"
 png = "0.16.8"
 rayon = "1.5.1"
+serde = { version = "1.0", features = ["derive"] }
+serde_json = "1.0.64"
+structopt = "0.3.22"
 thiserror = "1.0.25"
 
 

rtchallenge/src/camera.rs

@@ -1,9 +1,33 @@
-use std::sync::Mutex;
+use std::{
+    str::FromStr,
+    sync::{
+        mpsc::{sync_channel, Receiver, SyncSender},
+        {Arc, Mutex},
+    },
+    thread,
+};
 
 use rayon::iter::{IntoParallelIterator, ParallelIterator};
+use serde::Deserialize;
+use structopt::StructOpt;
 
 use crate::{canvas::Canvas, matrices::Matrix4x4, rays::Ray, tuples::Tuple, world::World, BLACK};
 
+#[derive(Copy, Clone, StructOpt, Debug, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+pub enum RenderStrategy {
+    Serial,
+    Rayon,
+    WorkerPool,
+}
+impl FromStr for RenderStrategy {
+    type Err = serde_json::error::Error;
+    fn from_str(s: &str) -> Result<RenderStrategy, serde_json::error::Error> {
+        Ok(serde_json::from_str(&format!("\"{}\"", s))?)
+    }
+}
+
+#[derive(Clone)]
 pub struct Camera {
     hsize: usize,
     vsize: usize,
@@ -13,6 +37,15 @@ pub struct Camera {
     pixel_size: f32,
     half_width: f32,
     half_height: f32,
+    pub render_strategy: RenderStrategy,
+}
+
+enum Request {
+    Line { width: usize, y: usize },
+}
+
+enum Response {
+    Line { y: usize, pixels: Canvas },
 }
 
 impl Camera {
@@ -59,6 +92,7 @@ impl Camera {
             pixel_size,
             half_height,
             half_width,
+            render_strategy: RenderStrategy::WorkerPool,
         }
     }
     pub fn hsize(&self) -> usize {
@@ -176,31 +210,88 @@ impl Camera {
     /// assert_eq!(image.get(5, 5), Color::new(0.38066, 0.47583, 0.2855));
     /// ```
     pub fn render(&self, w: &World) -> Canvas {
-        self.render_serial(w)
+        use RenderStrategy::*;
+
+        match self.render_strategy {
+            Serial => self.render_serial(w),
+            Rayon => self.render_parallel_rayon(w),
+            WorkerPool => self.render_parallel_one_tread_per_core(w),
+        }
     }
 
-    #[allow(dead_code)]
+    /// This render function spins up one thread per core, and pins the thread
-    fn render_parallel_one_tread_per_core(&self, w: &World) -> Canvas {
+    /// to the core.  It then sends work requests to the worker threads,
-        let image_mu = Mutex::new(Canvas::new(self.hsize, self.vsize, BLACK));
+    /// requesting a full line of the image by rendered.  The main thread
+    /// collects results and stores them in the canvas returned to the user.
+    fn render_parallel_one_tread_per_core(&self, world: &World) -> Canvas {
+        let mut image = Canvas::new(self.hsize, self.vsize, BLACK);
+        let num_threads = num_cpus::get();
+        let (pixel_req_tx, pixel_req_rx) = sync_channel(2 * num_threads);
+        let (pixel_resp_tx, pixel_resp_rx) = sync_channel(2 * num_threads);
+        let pixel_req_rx = Arc::new(Mutex::new(pixel_req_rx));
 
-        (0..self.vsize).into_par_iter().for_each(|y| {
+        // Create copy of world and camera we can share with all workers.
-            let mut row_image = Canvas::new(self.hsize, 1, BLACK);
+        // It's probably okay to clone camera, but world could get large (think
-            for x in 0..self.hsize {
+        // textures and high poly count models).
-                let ray = self.ray_for_pixel(x, y);
+        // TODO(wathiede): prevent second copy of world when they start getting
-                let color = w.color_at(&ray);
+        // large.
-                row_image.set(x, 0, color);
+        let world = Arc::new(world.clone());
+        let camera = Arc::new(self.clone());
+
+        let core_ids = core_affinity::get_core_ids().unwrap();
+        println!("Creating {} render threads", core_ids.len());
+        // Create a worker thread for each CPU core and pin the thread to the core.
+        let mut handles = core_ids
+            .into_iter()
+            .enumerate()
+            .map(|(i, id)| {
+                let w = Arc::clone(&world);
+                let c = Arc::clone(&camera);
+                let pixel_req_rx = pixel_req_rx.clone();
+                let pixel_resp_tx = pixel_resp_tx.clone();
+                thread::spawn(move || {
+                    core_affinity::set_for_current(id);
+                    render_worker(i, &c, &w, pixel_req_rx, &pixel_resp_tx);
+                })
+            })
+            .collect::<Vec<_>>();
+        drop(pixel_req_rx);
+        drop(pixel_resp_tx);
+
+        // Send render requests over channels to worker threads.
+        let (w, h) = (camera.hsize, camera.vsize);
+        handles.push(thread::spawn(move || {
+            for y in 0..h {
+                pixel_req_tx
+                    .send(Request::Line { width: w, y })
+                    .expect("failed to send line request");
             }
-            // TODO(wathiede): create a row based setter for memcpying the row as a whole.
+            drop(pixel_req_tx);
-            let mut image = image_mu.lock().expect("failed to lock image mutex");
+        }));
-            for x in 0..self.hsize {
+
-                image.set(x, y, row_image.get(x, 0));
+        // Read responses from channel and blit image data.
+        for resp in pixel_resp_rx {
+            match resp {
+                Response::Line { y, pixels } => {
+                    for x in 0..camera.hsize {
+                        image.set(x, y, pixels.get(x, 0));
                     }
-        });
-        image_mu
-            .into_inner()
-            .expect("failed to get image out of mutex")
                 }
-    #[allow(dead_code)]
+            }
+        }
+
+        // Wait for all the threads to exit.
+        for thr in handles {
+            thr.join().expect("thread join");
+        }
+        image
+    }
+
+    /// This renderer use rayon to split each row into a seperate thread.  It
+    /// seems to have really bad performance (only ~6x speedup over serial), and
+    /// the flame graph looks a mess.  A strength over
+    /// `render_parallel_one_tread_per_core` is that it doesn't require `Camera`
+    /// and `World` to be cloneable.
     fn render_parallel_rayon(&self, w: &World) -> Canvas {
         let image_mu = Mutex::new(Canvas::new(self.hsize, self.vsize, BLACK));
 
@@ -222,6 +313,7 @@ impl Camera {
             .expect("failed to get image out of mutex")
     }
 
+    /// Reference render implementation from the book.  Single threaded, nothing fancy.
     fn render_serial(&self, w: &World) -> Canvas {
         let mut image = Canvas::new(self.hsize, self.vsize, BLACK);
         for y in 0..self.vsize {
@@ -234,3 +326,34 @@ impl Camera {
         image
     }
 }
+
+fn render_worker(
+    tid: usize,
+    c: &Camera,
+    w: &World,
+    input_chan: Arc<Mutex<Receiver<Request>>>,
+    output_chan: &SyncSender<Response>,
+) {
+    loop {
+        let job = { input_chan.lock().unwrap().recv() };
+        match job {
+            Err(err) => {
+                eprintln!("Shutting down render_worker {}: {}", tid, err);
+                return;
+            }
+            Ok(req) => match req {
+                Request::Line { width, y } => {
+                    let mut pixels = Canvas::new(width, 1, BLACK);
+                    for x in 0..width {
+                        let ray = c.ray_for_pixel(x, y);
+                        let color = w.color_at(&ray);
+                        pixels.set(x, 0, color);
+                    }
+                    output_chan
+                        .send(Response::Line { y, pixels })
+                        .expect("failed to send pixel response");
+                }
+            },
+        }
+    }
+}