6 ms·

Is the do_something_quickly example actually buggy? while (!need_to_stop) /* BUGGY!!! */ do_something_quickly(); Can the optimiser really unroll the lo

by Gupie 7y ago

Is the do_something_quickly example actually buggy? while (!need_to_stop) /* BUGGY!!! */ do_something_quickly(); Can the optimiser really unroll the loop and only check the need_to_stop variable once!? if (!need_to_stop) for (;;) { do_something_quickly(); do_something_quickly(); do_something_quickly(); do_something_quickly(); .... Is this the same for C++ or is does the new memory model prevent it?

nybble41 7y ago
> Can the optimiser really unroll the loop and only check the need_to_stop variable once!? Yes. The compiler can see that need_to_stop is not changed by any of the code inside the loop, and since it wasn't qualified with "volatile" or using explicit atomic operations the compiler is free to assume that it won't be changed asynchronously. That makes need_to_stop a constant for the duration of the loop, so it should have the same effect (given those assumptions) whether the variable is checked once or before each call to do_something_quickly(). Checking once is considerably more efficient.
- nsajko 7y ago
  Are you sure atomics can help in this case? EDIT: also, volatile is a bad idea, too, unless you are doing something low-level like a kernel or hacking on memory mapped IO pins. It does not synchronize accesses between threads.
  - plorkyeran 7y ago
    C11 atomics include a memory barrier, so making need_to_stop atomic would indeed suffice.
  - nybble41 7y ago
    In addition to what plorkyeran said: While volatile alone doesn't synchronize accesses between threads, it is enough to ensure that the variable is read separately in each iteration of the loop—though it might just be read from the L1 cache and not take into account writes from other cores.
- gwd 7y ago
  > the compiler is free to assume that it won't be changed asynchronously. This seems like it might possibly have been a reasonable assumption 30 years ago, when multi-processor machines were tens of thousands of dollars; but now that your watch has 4 cores, non-local variables changing should be considered the norm.
  - jcranmer 7y ago
    The modern language standards (C++11/C11 and newer) explicitly take multicore machines into account, and the memory model was specifically designed knowing the capabilities of hardware. The language standard says that the programmer is not permitted to write data races, even if they are benign. You either need to provide synchronization in the form of locks or explicit memory barriers, or you need to use special atomic variables whose semantics allow concurrent access without synchronization. Without these rules (or something substantially similar), compilers would instead be forced to add sequentially-consistent memory barriers around every memory access they couldn't prove was only within a single thread (i.e., add it to every single memory access). The resulting performance would be completely unacceptable.
nsajko 7y ago
The code is simply incorrect. Use locking (pthread_mutex_lock, pthread_mutex_trylock, pthread_mutex_unlock) if you need to do something like that. C and C++ have the same memory model, but this would be wrong in any language.
- gwd 7y ago
  Oh, right! All Linux has to do then is link pthread into the kernel and all their problems will magically go away.</snark> The original purpose of C was to write operating systems; at some point that was basically discarded, and it is now incredibly difficult to write an operating system safely in C anymore. It's like juggling lightsabers.
  - nsajko 7y ago
    If somebody does not get basic concurrency, it seems appropriate to point them to POSIX Threads' locks, they are obviously not ready to start hacking on the kernel: https://www.kernel.org/doc/htmldocs/kernel-locking/locks.html https://www.kernel.org/doc/htmldocs/kernel-locking/locks.htm...
    - CodeMage 7y ago
      Yes, but it's much more helpful -- and less abrasive -- to do so after answering their actual question.
  - jcranmer 7y ago
    The Linux kernel has chosen to adopt a memory model and a threading model. This was done even before C++11/C11 defined a memory model, and the two systems are quite similar and can mostly be built on top of one another (READ_ONCE/WRITE_ONCE is sort of the odd man out, but it works if you assume the semantics of volatile are "do not eliminate or rematerialize this load/store," which is a correct model if you look at compiler implementations). In practice, the memory model of real hardware systems do a much more thorough job of screwing you up than compiler optimizations, so any practical operating system already needs to build a memory model that can cope with hardware reordering. The only interaction you need with the compiler is a few hooks to stop optimizations, which are quite easy to build in (an empty volatile asm that clobbers memory will prevent reordering of memory operations, for example), and newer language standards actually give you specific mechanisms that give you the building blocks without the need to resort to inline asm tricks.

by Gupie 7y ago

nybble41 7y ago

> Can the optimiser really unroll the loop and only check the need_to_stop variable once!? Yes. The compiler can see that need_to_stop is not changed by any of the code inside the loop, and since it wasn't qualified with "volatile" or using explicit atomic operations the compiler is free to assume that it won't be changed asynchronously. That makes need_to_stop a constant for the duration of the loop, so it should have the same effect (given those assumptions) whether the variable is checked once or before each call to do_something_quickly(). Checking once is considerably more efficient.

nsajko 7y ago
Are you sure atomics can help in this case? EDIT: also, volatile is a bad idea, too, unless you are doing something low-level like a kernel or hacking on memory mapped IO pins. It does not synchronize accesses between threads.
- plorkyeran 7y ago
  C11 atomics include a memory barrier, so making need_to_stop atomic would indeed suffice.
- nybble41 7y ago
  In addition to what plorkyeran said: While volatile alone doesn't synchronize accesses between threads, it is enough to ensure that the variable is read separately in each iteration of the loop—though it might just be read from the L1 cache and not take into account writes from other cores.
gwd 7y ago
> the compiler is free to assume that it won't be changed asynchronously. This seems like it might possibly have been a reasonable assumption 30 years ago, when multi-processor machines were tens of thousands of dollars; but now that your watch has 4 cores, non-local variables changing should be considered the norm.
- jcranmer 7y ago
  The modern language standards (C++11/C11 and newer) explicitly take multicore machines into account, and the memory model was specifically designed knowing the capabilities of hardware. The language standard says that the programmer is not permitted to write data races, even if they are benign. You either need to provide synchronization in the form of locks or explicit memory barriers, or you need to use special atomic variables whose semantics allow concurrent access without synchronization. Without these rules (or something substantially similar), compilers would instead be forced to add sequentially-consistent memory barriers around every memory access they couldn't prove was only within a single thread (i.e., add it to every single memory access). The resulting performance would be completely unacceptable.

nsajko 7y ago

The code is simply incorrect. Use locking (pthread_mutex_lock, pthread_mutex_trylock, pthread_mutex_unlock) if you need to do something like that. C and C++ have the same memory model, but this would be wrong in any language.

gwd 7y ago
Oh, right! All Linux has to do then is link pthread into the kernel and all their problems will magically go away.</snark> The original purpose of C was to write operating systems; at some point that was basically discarded, and it is now incredibly difficult to write an operating system safely in C anymore. It's like juggling lightsabers.
- nsajko 7y ago
  If somebody does not get basic concurrency, it seems appropriate to point them to POSIX Threads' locks, they are obviously not ready to start hacking on the kernel: https://www.kernel.org/doc/htmldocs/kernel-locking/locks.html https://www.kernel.org/doc/htmldocs/kernel-locking/locks.htm...
  - CodeMage 7y ago
    Yes, but it's much more helpful -- and less abrasive -- to do so after answering their actual question.
- jcranmer 7y ago
  The Linux kernel has chosen to adopt a memory model and a threading model. This was done even before C++11/C11 defined a memory model, and the two systems are quite similar and can mostly be built on top of one another (READ_ONCE/WRITE_ONCE is sort of the odd man out, but it works if you assume the semantics of volatile are "do not eliminate or rematerialize this load/store," which is a correct model if you look at compiler implementations). In practice, the memory model of real hardware systems do a much more thorough job of screwing you up than compiler optimizations, so any practical operating system already needs to build a memory model that can cope with hardware reordering. The only interaction you need with the compiler is a few hooks to stop optimizations, which are quite easy to build in (an empty volatile asm that clobbers memory will prevent reordering of memory operations, for example), and newer language standards actually give you specific mechanisms that give you the building blocks without the need to resort to inline asm tricks.