From 4f88d2c101af1dc5e7e5a43e064c0da571d3b61d Mon Sep 17 00:00:00 2001 From: Bill Thiede Date: Sun, 18 Jul 2021 11:48:12 -0700 Subject: [PATCH] camera: make rendering strategy configurable, add workerpool version. --- rtchallenge/Cargo.lock | 256 +++++++++++++++++++++++++++++++++++++- rtchallenge/Cargo.toml | 8 +- rtchallenge/src/camera.rs | 163 +++++++++++++++++++++--- 3 files changed, 402 insertions(+), 25 deletions(-) diff --git a/rtchallenge/Cargo.lock b/rtchallenge/Cargo.lock index 3c6a1a4..251ac66 100644 --- a/rtchallenge/Cargo.lock +++ b/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]] diff --git a/rtchallenge/Cargo.toml b/rtchallenge/Cargo.toml index 6962e19..68c1e5e 100644 --- a/rtchallenge/Cargo.toml +++ b/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" @@ -22,4 +28,4 @@ name = "matrices" harness = false [profile.release] -debug = true \ No newline at end of file +debug = true diff --git a/rtchallenge/src/camera.rs b/rtchallenge/src/camera.rs index bbe9af2..966e56c 100644 --- a/rtchallenge/src/camera.rs +++ b/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 { + 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)] - fn render_parallel_one_tread_per_core(&self, w: &World) -> Canvas { - let image_mu = Mutex::new(Canvas::new(self.hsize, self.vsize, BLACK)); + /// This render function spins up one thread per core, and pins the thread + /// to the core. It then sends work requests to the worker threads, + /// 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| { - let mut row_image = Canvas::new(self.hsize, 1, BLACK); - for x in 0..self.hsize { - let ray = self.ray_for_pixel(x, y); - let color = w.color_at(&ray); - row_image.set(x, 0, color); + // Create copy of world and camera we can share with all workers. + // It's probably okay to clone camera, but world could get large (think + // textures and high poly count models). + // TODO(wathiede): prevent second copy of world when they start getting + // large. + 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::>(); + 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. - 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)); + drop(pixel_req_tx); + })); + + // 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") + } + + // Wait for all the threads to exit. + for thr in handles { + thr.join().expect("thread join"); + } + image } - #[allow(dead_code)] + + /// 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>>, + output_chan: &SyncSender, +) { + 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"); + } + }, + } + } +}