[vtk-developers] Change in VTK[master]: Added support for atomic integers.

[Berk Geveci]

H Sean,

I moved this discussion to the VTK developers so that we may get feedback
from other.

First of all, I apologize. I should have sent an e-mail describing the
intention of these changes. That would have clarified certain things
better. My larger goal is to eventually introduce a framework for
developing scalable multi-threaded algorithms in VTK. We have been working
with EDF and Inria who has been developing the framework and my role is to
introduce these changes to VTK. These changes are the first steps towards
that goal.

Let me try to answer your questions within this context:

> Also, all these different ways to increment an integer, wow! :)  How
about using C++11
> std::atomic and falling back to the platform-specific stuff as a last
resort?

As I mentioned, my goal is to introduce a larger framework multi-threaded
computing in VTK. Something similar to Intel TBB but abstracted such that
we can use other backends such as X-Kaapi, OpenMP or even our own if we
choose so. So in preparation for that, I had to create the infrastructure
for various backend, switching between them etc. While doing that, I
thought to also use it to support "native" atomic operations for each
backend. Part of the reason behind this is that each implementation seems
to work on a subset of our target architectures. We have native atomic
operations for Mac, gcc and Windows. These may or may not work on other
compilers (Intel, XLC and PGI for example). Intel TBB supports another
subset. OpenMP 3.1 supports atomics the way we need them but is only
supported on some platforms. Ditto for X-Kaapi. I don't even know how many
of the compilers support std::atomic. Oh btw, the Boost atomic is even less
portable than others :-)

This will continue to be the case as we add more functionality such as
task-stealing parallel for, tasks etc. Each library seems to support only a
subset of platforms.

Keep in mind that we want to support lots of platforms from various
desktops to supercomputers to potentially embedded systems.

I am open to suggestions to make this more robust. As this morning
dashboard shows, it is not easy to get this to work on all platforms and
I'd rather use one standard implementation if possible.

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

It was deliberate change based on the documentation, which says:

     These functions are thread and multiprocessor safe.  For each
function, there is a
     version that does and another that does not incorporate a memory
barrier.  Barriers
     strictly order memory access on a weakly-ordered architecture such as
PPC.  All
     loads and stores executed in sequential program order before the
barrier will com-
     plete before any load or store executed after the barrier.  On a
uniprocessor, the
     barrier operation is typically a nop.  On a multiprocessor, the
barrier can be
     quite expensive.

     Most code will want to use the barrier functions to ensure that memory
shared
     between threads is properly synchronized.  For example, if you want to
initialize a
     shared data structure and then atomically increment a variable to
indicate that the
     initialization is complete, then you must use
OSAtomicIncrement32Barrier() to
     ensure that the stores to your data structure complete before the
atomic add.
     Likewise, the consumer of that data structure must use
OSAtomicDecrement32Bar-
     rier(), in order to ensure that their loads of the structure are not
executed
     before the atomic decrement.  On the other hand, if you are simply
incrementing a
     global counter, then it is safe and potentially much faster to use
OSAtomicIncre-
     ment32().  If you are unsure which version to use, prefer the barrier
variants as
     they are safer.

What we are doing in vtkTimeStamp falls into "simply incrementing a global
counter", which justifies using a much faster implementation in my opinion.
What do you think? We'll probably have to revisit what AtomicInts do under
covers if people start using them for other purposes than counters.

Best,
-berk

On Tue, Aug 20, 2013 at 11:11 AM, Sean McBride (Code Review) <
review at kitware.com> wrote:
>
> Sean McBride has posted comments on this change.
>
> Change subject: Added support for atomic integers.
> ......................................................................
>
>
> Patch Set 6:
>
> Was the OSAtomicIncrement32/64Barrier() call deliberately weakened to
OSAtomicIncrement32/64()?  The Windows InterlockIncrement() is documented
to use a memory barrier, and VTK on OS X did before, so...
>
> Also, all these different ways to increment an integer, wow! :)  How
about using C++11 std::atomic and falling back to the platform-specific
stuff as a last resort?
>
> --
> To view, visit http://review.source.kitware.com/12253
>
>
> To unsubscribe, visit http://review.source.kitware.com/settings
>
> Gerrit-MessageType: comment
> Gerrit-Change-Id: Ie606ef9202f9588616a0b5412123512ffe536be6
> Gerrit-PatchSet: 6
> Gerrit-Project: VTK
> Gerrit-Branch: master
> Gerrit-Owner: Berk Geveci <berk.geveci at kitware.com>
> Gerrit-Reviewer: Berk Geveci <berk.geveci at kitware.com>
> Gerrit-Reviewer: Kitware Robot <kwrobot at kitware.com>
> Gerrit-Reviewer: Robert Maynard <robert.maynard at kitware.com>
> Gerrit-Reviewer: Utkarsh Ayachit <utkarsh.ayachit at kitware.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20130820/21b1905b/attachment.html>