Deprecate vtkMutexLock and vtkAtomic ?

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

Should we deprecate it for further removal ?

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

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

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?

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

I may give it a go during the Hackaton.

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.

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

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

indeed.

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

What needs to happen for the other classes can they be replaced similar to what you proposed?

will this work?

I found this as well which I think gets pretty close to making it make sense. https://gitlab.kitware.com/vtk/vtk/-/merge_requests/7224

and this: documentation from gitlab

Making more sense now.

Yes, this is the same idea, more work to be done though.

1 Like

Thank you @mwestphal. I think I was able to resolve my issue following the current updates.

What is missing?

Neverming, looking at @seanm MR, looks like all have been adressed !

1 Like