Should we replace SafeDownCast with dynamic_cast

(Bill Lorensen) #1

Folks,

SafeDownCast was added to vtk way back when dynamic_cast was not available everywhere. Should we replace SafeDownCast in vtk with dynamic_cast? It is more natural for modern C++ and is well-documented. It may be more efficient although I doubt the current implementation is not a significant resource consumer.

See:

vtkKochanekSpline::SafeDownCast(this->ParametricSpline->GetXSpline())->SetDefaultTension(value);

versus

dynamic_cast<vtkKochanekSpline *>(this->ParametricSpline->GetXSpline())->SetDefaultTension(value);

It is used in over 1500 vtk files. but the changes could be automated.

Bill

4 Likes
(Bill Lorensen) #2

I did an interesting experiment. I pushed an MR that replaced the implementation of SafeDownCast to use dynamic_cast. The first problem was using vtkAbstractHyperTreeGridMapper in vtkCellPicker. The hyper class has no subclasses in VTK. Not sure why.

I’m also getting some failed tests. I’ll continue to investigate…

1 Like
(Ken Martin) #3

I like this idea Bill, seems like if we can do it, we should.

(Todd) #4

SafeDownCast is redundant in wrapper code, so no issues there.

(Julien Finet) #5

I was under the impression that there are different options to improve performances of dynamic_cast. (e.g. http://www.stroustrup.com/fast_dynamic_casting.pdf). By keeping SafeDownCast, we let ourselves the option to speedup downcasting some day…

But maybe things have changed with the newests C++, I am not really up to speed here.

(Bill Lorensen) #6

@jjomier I pushed an MR that replaced SafeDownCast with dynamic_cast. As you suspected, there are performance issues. We are better off keeping DafeDownCast. I’m afraid I wasted some of my not-so-costly time. I am closing the MR.

See these results: https://tinodidriksen.com/2010/04/cpp-dynamic-cast-performance/

Bill

(Andrew Maclean) #7

Thanks for experimenting @lorensen we now know there are performance issues!

(Bill Lorensen) #8

Here’s a link to a dashboard that illustrates performance and failure related to my dynamic_cast experiment.

1 Like
(Todd) #9

Did you identify the cause of failure with the dynamic cast on some test machines? It seems odd.

(Bill Lorensen) #10

There were lots of discussion about this at the hackathon. Way above my level of c++ knowledge.