3

I'm implementing my path tracer using C++ and OpenMP partially based on the design of Small Paint (using a globally available RND function to generate random floats) and PBRTv2 (uses a single RNG across all threads). I notice that due to the fact there can can be multiple threads accessing the same RNG. Race condition and CPU cache invalidation will occur caused by the RNG writing to memory to store/update it's current state. Thus dragging the performance down and mess up the RNG(makes it unpredictable).

How come this is in one of the top grade educational renderer? Or it's just a minor issue that isn't important?
If it's important, is there a known good way to fix this without breaking PBRT's rendering/threading architecture?

trichoplax is on Codidact now
  • 6,292
  • 3
  • 23
  • 69
Mary Chang
  • 651
  • 4
  • 14

1 Answers1

3

You need to implement your own RNG class and not use the clib RNG implementation that shares the state across threads. What I'm using is a simple & efficient multiply-with-carry RNG by George Marsaglia, that I initialize with a seed based on pixel coordinates (e.g. simple x+y*width) that I'm calculating (state per pixel instead of global state)

JarkkoL
  • 3,636
  • 13
  • 14
  • This is more than thread safety. A mutex would make any global RNG thread safe but still slow and unpredictable with multiple threads. Per-pixel seed is indeed the way to go for reproducibility, although per-thread state is enough to fix the performance problem. – Olivier Dec 29 '16 at 15:33
  • With thread-safe I don't mean use of mutex etc. but just making of an RNG class that has no static members. The clib version is bad as it has a global state shared across threads. I'll fix the answer to be more clear – JarkkoL Dec 29 '16 at 15:51
  • PBRT already has that so the problem is likely how it is used (shared ?). I unfortunately don't know the PBRT code well enough to know what's going on. – Olivier Dec 29 '16 at 17:04
  • A thread-safe RNG is not enough because that it only solves the race condition. But it serializes the execution and cache invalidation will still be present. – Mary Chang Dec 31 '16 at 04:22
  • I'm not suggesting thread-safe implentation of a RNG class but implemeting RNG class that you use thread-safely, by not sharing the state across threads. – JarkkoL Dec 31 '16 at 04:48