GetCellPoints and thread safety

Development
1

After upgrading from vtk 8.0 to 9.2.6 several of our tests started becoming flaky.
It turns out that in some cases (e.g. interpolation) we are calling GetCellPoints(vtkIdType id, vtkIdType npts, vtkIdType* pts) of a vtkUnstructuredGrid in different threads (tbb::task_group, parall_for, omp, etc).

Reading the docs it seems that we could use the variant that fills a vtkIdList, i.e. GetCellPoints(vtkIdType id, vtkIdList* ids). But the docs say this method is thread-safe IF FIRST CALLED FROM A SINGLE THREAD. I understand that the first call initializes some internal data structure.

  • Looking at the code, I cannot see what is being initialized in the first call.
  • Also, the API would feel less hacky if we could call e.g. a method InitializeForThreadSafety() or similar.

Digging a bit deeper I found this fix:

  vtkSmartPointer<vtkCellArrayIterator> iter;
  if (cells->IsStorageShareable())
  {
    // much faster and thread-safe if storage is shareable
    cells->GetCellAtId(tag.GetCellId(), numPts, pts);
  }
  else
  {
    // guaranteed thread safe
    iter = vtk::TakeSmartPointer(cells->NewIterator());
    iter->GetCellAtId(tag.GetCellId(), numPts, pts);
  }

Is there a recommended thread safe “nearly” random access API to get the cell points?
How costly is the vtkCellArrayIterator? In the fix linked above, it says GetCellAtId is much faster if the data is sharable.

2

The thread-safety commit that you linked seems to be for vtkPolyData, not for vtkUnstructuredGrid. Different data classes have different thread-safety concerns, as fiddly as that may seem.

For vtkUnstructuredGrid, GetCellPoints(vtkIdType id, vtkIdList* ids) is thread safe, as is any variant of the GetCellPoints() method that uses a vtkIdList.

For vtkPolyData, this method is only thread-safe if you call BuildCells() first, before starting the multithreading.

As far as I know, there are two “initialization” methods related to thread safety:

  1. BuildCells() for GetCellPoints()
  2. BuildLinks() for GetPointCells()

You’re right that the “if first called from a single thread” condition in the documentation isn’t great, since that isn’t nearly as explicit as having a proper initialization method. And BuildCells() could actually be such an initialization method, except that it isn’t part of the vtkDataSet API (it’s only for vtkPolyData).

A better approach might be to get a vtkCellArray from whatever data object you’re working with, and then send that to your threads instead of sending the data object itself.

1 Like
3

Thanks @dgobbi. It seems our second thread safety issue is due to vtkPointSet::ComputeBounds not being thread safe.

We were building a vtkCellLocator in each thread (because in vtk 8.0 vtkCellLocator was not thread safe). vtkCellLocator::BuildLocator calls dataset->ComputeBounds(). In our testsuite vtkCellLocator::FindClosestPoint sometimes did not find a closest point (cell_id < 0). Building a single shared locator before the parallel section fixes the issue.

4

This is the fastest and thread safe GetCellPoints function:

VTK: vtkUnstructuredGrid Class Reference

BuildCells is required by vtkPolyData so that the above GetCellPoints is thread safe.

A quick way to initialize a vtkDataSet (including a vtkPolyData) is to perform GetCell(0), which internally calls BuildCells if needed.

Running many multithreaded operations at the same time on the same object is indeed not thread-safe that’s why vtkPointSet::ComputeBounds is not threads safe, but now vtkCellLocator is thread safe, so you are good! Consider using vtkStaticCellLocator for improved built time performance, or vtkCellTreeLocator for better query time but slower built time.

1 Like