Trying to add Rayon for multi-threaded NoiseMapBuilder::build function

[barneydmedia]

I was trying to use this crate for building dynamic world maps, but it was a bit slow for my needs. I noticed that it was only using a single core at 100%, so I decided to add Rayon multi threading to NoiseMapBuilder::build to speed up the process. I got this to work somewhat, but the self references inside the function can't be passed between threads (see error below). Multi-threading the other bits of the process only gains about ~20% performance since most of the math actually happens in the self.source_module.get() functions.

The problem I run into is that there is a mutable reference in self.source_module which prevents me from multi-threading the self.source_module.get() calls. Is there a way that I could safely pass/clone/RwLock the self reference that would get me around this block?

Here is the error I get on Rust nightly:

error[E0277]: `(dyn noise_fns::NoiseFn<[f64; 3]> + 'a)` cannot be shared between threads safely
   --> src/utils/noise_map_builder.rs:222:67
    |
222 |             let final_values: Vec<_> = (0..width).into_par_iter().map(|x| {
    |                                                                   ^^^ `(dyn noise_fns::NoiseFn<[f64; 3]> + 'a)` cannot be shared between threads safely
    |
    = help: the trait `std::marker::Sync` is not implemented for `(dyn noise_fns::NoiseFn<[f64; 3]> + 'a)`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&'a (dyn noise_fns::NoiseFn<[f64; 3]> + 'a)`
    = note: required because it appears within the type `utils::noise_map_builder::PlaneMapBuilder<'a>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&mut utils::noise_map_builder::PlaneMapBuilder<'a>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::RwLock<&mut utils::noise_map_builder::PlaneMapBuilder<'a>>`
    = note: required because of the requirements on the impl of `std::marker::Sync` for `std::sync::Arc<std::sync::RwLock<&mut utils::noise_map_builder::PlaneMapBuilder<'a>>>`
    = note: required because it appears within the type `&std::sync::Arc<std::sync::RwLock<&mut utils::noise_map_builder::PlaneMapBuilder<'a>>>`
    = note: required because it appears within the type `[closure@src/utils/noise_map_builder.rs:222:71: 248:14 x_bounds:&(f64, f64), x_step:&f64, locked:&std::sync::Arc<std::sync::RwLock<&mut utils::noise_map_builder::PlaneMapBuilder<'a>>>, is_seamless:&bool, current_y:&f64, x_extent:&f64, y_extent:&f64, y_bounds:&(f64, f64)]`

I'm planning to PR this once it works, FWIW.

[Razaekel]

If you don't mind, you can make a PR with what you have currently and mark it as [WIP], so that I can see how you're going about it currently.

[barneydmedia]

@Razaekel PR is up, thanks for taking a look. I'm new to rust so if something doesn't make sense feel free to point it out.

[Razaekel]

Most of the issues revolve around the use of NoiseFn as a trait object, and Rust complains when you try to send trait objects between threads. As a trait object, it doesn't automatically get marked as Send or Sync so that has to be manually set on NoiseFn, as well as in Displace. Once you get past that, then you run into the issue of Cache needing to be rewritten for threaded operation (ie. replace Cell/RefCell with something else). So for the time being, I just commented out that module.

Give the features/send+sync branch branch a try.

[barneydmedia]

Seems to work great. Anything more I can do?

[Razaekel]

Try and break it, and let me know by filing issues if you do.

[barneydmedia]

I ran all the tests, all the current examples, and re-wrote the Complex World example to not use caches. Specifically, I checked to make sure that there weren't out-of-order data issues with the examples but everything seemed to render ok. The Complex World example of course took forever without caching, but it did eventually render and looked correct.

I also checked activity monitor on the longer running tasks and saw roughly 96% CPU usage, with the remaining ~4% going to kernel tasks. Not sure if it's significant, but 9 threads were spawned on an 8 logical core/4 physical core MBP pro. I expect we will see speedups roughly linear with the number of available processors for most tasks, but this may vary a bit based on Rayon's default behaviors for thread spawning.