Noise with multi-threaded raytracer

This is my first multi-threaded implementation, so it's probably a beginners mistake. The threads handle the rendering of every second row of pixels (so all rendering is handled within each thread). The problem persists if the threads render the upper and lower parts of the screen respectively.

Both threads read from the same variables, can this cause any problems? From what I've understood only writing can cause concurrency problems...

Can calling the same functions cause any concurrency problems? And again, from what I've understood this shouldn't be a problem...

The only time both threads 开发者_StackOverflowwrite to the same variable is when saving the calculated pixel color. This is stored in an array, but they never write to the same indices in that array. Can this cause a problem?

Multi-threaded rendered image (Spam prevention stops me from posting images directly..)

Ps. I use the exactly same implementation in both cases, the ONLY difference is a single vs. two threads created for the rendering.

---

Both threads read from the same variables, can this cause any problems? From what I've understood only writing can cause concurrency problems...

This should be ok. Obviously, as long the data is initialized before the two threads start reading and destroyed after both threads have finished.

Can calling the same functions cause any concurrency problems? And again, from what I've understood this shouldn't be a problem...

Yes and no. Too hard to tell without the code. What does the function do? Does it rely on shared state (e.g. static variables, global variables, singletons...)? If yes, then this is definitely a problem. If there is never any shared state, then you're ok.

The only time both threads write to the same variable is when saving the calculated pixel color. This is stored in an array, but they never write to the same indices in that array. Can this cause a problem?

Maybe sometimes. An array of what? It's probably safe if sizeof(element) == sizeof(void*), but the C++ standard is mute on multithreading, so it doesn't force your compiler to force your hardware to make this safe. It's possible that your platform could be biting you here (e.g. 64bit machine and one thread writing 32bits which might overwrite an adjacent 32bit value), but this isn't an uncommon pattern. Usually you're better off using synchronization to be sure.

You can solve this in a couple of ways:

• Each thread builds its own data, then it is aggregated when they complete.
• You can protect the shared data with a mutex.

The lack of commitment in my answers are what make multi-threaded programming hard :P

For example, from Intel® 64 and IA-32 Architectures Software Developer's Manuals, describes how different platforms gaurantee different levels of atomicity:

7.1.1 Guaranteed Atomic Operations

The Intel486 processor (and newer processors since) guarantees that the following basic memory operations will always be carried out atomically:

• Reading or writing a byte
• Reading or writing a word aligned on a 16-bit boundary
• Reading or writing a doubleword aligned on a 32-bit boundary

The Pentium processor (and newer processors since) guarantees that the following additional memory operations will always be carried out atomically:

• Reading or writing a quadword aligned on a 64-bit boundary
• 16-bit accesses to uncached memory locations that fit within a 32-bit data bus

The P6 family processors (and newer processors since) guarantee that the following additional memory operation will always be carried out atomically:

• Unaligned 16-, 32-, and 64-bit accesses to cached memory that fit within a cache line

Accesses to cacheable memory that are split across bus widths, cache lines, and page boundaries are not guaranteed to be atomic by the Intel Core 2 Duo, Intel Atom, Intel Core Duo, Pentium M, Pentium 4, Intel Xeon, P6 family, Pentium, and Intel486 processors. The Intel Core 2 Duo, Intel Atom, Intel Core Duo, Pentium M, Pentium 4, Intel Xeon, and P6 family processors provide bus control signals that permit external memory subsystems to make split accesses atomic; however, nonaligned data accesses will seriously impact the performance of the processor and should be avoided.

---

I have solved the problem, I did it by building up the data separately for each thread just as Stephen suggested (the elements where not of void* size). Thanks for a very detailed answer!