Deprecate vtkMutexLock and vtkAtomic ?

(Mathieu Westphal (Kitware)) #1

vtkMutexLock is useless since C++11 support and std::mutex.

Should we deprecate it for further removal ?

(Mathieu Westphal (Kitware)) #2

The same could be said about vtkAtomic and std::atomic. The work to remove it may be bigger though.

(Dave DeMarle) #3

If we can do so without much upgrade pain for applications that use VTK then I say heck yeah.

(Joachim Pouderoux (Kitware)) #4

Always remember that standard is one thing, implementation is another.
See https://en.cppreference.com/w/cpp/compiler_support#cpp11

However, regarding mutex and atomic, I think they are now quite well supported.
vtkAtomic has different implementations depending on the selected SMP backend. Could there a benefit to use TBB atomics instead of C++11 ones?

(Mathieu Westphal (Kitware)) #5

@will.schroeder : do you have an advice here ?

(Mathieu Westphal (Kitware)) #6

I may give it a go during the Hackaton.

(Robert Maynard) #7

I expect that std::atomic will be an easy replacement for vtkAtomic when T is a primitive type. The open question is how well specified std::atomic<void*> is, as vtkAtomic also supports that.

(Mathieu Westphal (Kitware)) #8

one information : std::mutex does not support forward declaration

(Robert Maynard) #9

It is undefined behavior to forward declare anything from that exists in the std:: namespace.

(Mathieu Westphal (Kitware)) #10

indeed.

(Mathieu Westphal (Kitware)) #11

here is a WIP MR : https://gitlab.kitware.com/vtk/vtk/merge_requests/5424
It replace vtkMutexLock by std::mutex. Seems to work fine so far.

The porting is quite simple : #include <mutex>, replace vtkMutexLock by std::mutex , replace Lock() by lock() and Unlock() by unlock() and you’re done.

However there are other classes that would need to be considered as well :
vtkCriticalSection
vtkSimpleCriticalSection
vtkConditionVariable

1 Like