<div dir="ltr">> >Keep in mind that we want to support lots of platforms from various<br>> >desktops to supercomputers to potentially embedded systems.<br>><br>> All the more reason to use std::atomic, no?  It's new, so isn't widely supported yet, but going forward it seems preferable.  Newest gcc and clang both claim <div>

> to fully support C++11 now.</div><div><br></div><div>Very good point. I'll read up on std::atomic and add it as the main option when not using OpenMP 3.1, TBB or Kaapi. I would still rather match the atomic implementation to the parallel library when possible. It doesn't sound like clang support std::atomic btw. Neither does it support other atomic implementations. So it would fall back to the Mac OS functions.</div>

<div><br>> >> Was the OSAtomicIncrement32/64Barrier() call deliberately weakened to<br>> >> OSAtomicIncrement32/64()?  The Windows InterlockIncrement() is documented<br>> >to use a<br>> >> memory barrier, and VTK on OS X did before, so...<br>

> ><br>> >It was deliberate change based on the documentation, which says:<br>> ><br>> > *SNIP*<br>> ><br>> >What we are doing in vtkTimeStamp falls into "simply incrementing a global<br>

> >counter", which justifies using a much faster implementation in my opinion.<br>> >What do you think? We'll probably have to revisit what AtomicInts do under<br>> >covers if people start using them for other purposes than counters.<br>

><br>> Well, I am not a multithreading expert, but I know enough to know that it's playing with fire. :)  The docs you  quote do say "Most code should use the barrier </div><div>> functions to ensure that memory shared between threads is properly synchronized".</div>

<div><br></div><div>I don't think that it is as bad as they make it sound. They are encouraging you to be conservative. Everything should be OK as long as you are not trying to use vtkTimeStamp to order the execution of multiple threads. A common design pattern (the one they are referring to) is something like this:</div>

<div><br></div><div style>Atomic int foo set to 0</div><div style><br></div><div>Thread A:</div><div><br></div><div>Initialize data structure X</div><div style>Atomically increment foo</div><div style><br></div><div style>

Thread B:</div><div style><br></div><div style>Wait until foo is 1</div><div style>Use data structure X</div><div style><br></div><div style>If you don't use the Barrier version of the function, there is no guarantee that the initialization of X will be done before foo is set to 1, which makes what thread B does unsafe. The Barrier version guarantees it.</div>

<div style><br></div><div style>Given how vtkTimeStamp is designed, it would be foolish to use it for a purpose like this. The following would work fine without the Barrier:</div><div style><br></div><div style>Thread A and B:</div>

<div style><br></div><div style>vtkNew<vtkObjectTypeFoo> obj;</div><div style>mtime = obj.GetMTime();</div><div style><br></div><div style>// do something that might change obj</div><div style><br></div><div style>
if (obj.GetMTime() > mtime)</div>
<div style>   // do something else</div><div style><br></div><div style>As far as I know, this is the only design pattern the time stamp is designed for. Am I missing something?</div><div><br>> If you think the weaker non-barrier version is sufficient, why on Windows do you use InterlockedIncrement() and not InterlockedIncrementNoFence()?<br>

<br></div><div style>Because I didn't know about InterlockedIncrementNoFence() :-)</div><div style><br></div><div style>Having said all of the above, I am leaning more towards changing to the Barrier version now. The performance difference does not seem to be significant (I will test more) and I can't foresee how people will use AtomicInts in the future. By the way, std::atomic seems to be much more thorough about it:</div>

<div style><br></div><div style><a href="http://en.cppreference.com/w/cpp/atomic/memory_order">http://en.cppreference.com/w/cpp/atomic/memory_order</a><br></div><div style><br></div><div style>It sounds like the Barrier versions are equivalent to std::memory_order_seq_cst.</div>

<div style><br></div><div style>The GCC ones also use full barriers.</div><div style><br></div><div style>-berk</div><div style><br></div></div>