[vtk-developers] vtkMath inline choices...

[David Cole]

On Mon, Jan 31, 2011 at 11:04 AM, Sean McBride <sean at rogue-research.com>wrote:

> On Sun, 30 Jan 2011 11:24:13 -0700, David Gobbi said:
>
> >The main issue with performance and IsNan() is more about how it is used
> >than about whether it is inlined.  My guess is that Sean saw a performance
> >hit because IsNan() was used inside vtkLookupTable, which has some of the
> >hottest loops in all of VTK.  The IsNan() check was applied to all
> >datatypes, even though it is not relevant for integer data.
> >
> >I've patched vtkLookupTable so that it overloads vtkLinearLookup() and the
> >IsNan check is only done on float and double.  The new code gives me a 40%
> >speedup in color mapping a 16-bit image to RGBA.  The patch can be seen
> >here: http://vtk.org/gitweb?p=VTK.git;a=commitdiff;h=1c348fcb.  I merged
> it
> >directly into master.
>
> David,
>
> Yes, exactly.  Thanks for improving it.  In fact I have a local branch
> with changes similar to what you just did, but I wanted to go slowly,
> and thought the leaf of the problem (isnan) was a good place to start.
>
> This part:
>
> +  // calling isnan() instead of vtkMath::IsNan() improves performance
> +#ifdef VTK_HAS_ISNAN
> +  if (isnan(v))
> +#else
> +  if (vtkMath::IsNan(v))
> +#endif
>
> I'm sure you agree is silly, and can be done away with once we fix
> vtkMath::IsNan().
>
> Try as I may, I can't figure out how to add a comment in Gerrit.


You have to be signed in to make comments on Gerrit. Once you're signed in,
if you double click on a "changed line of code" while reviewing, it will pop
up an in-place text edit window in which you may type whatever you like.

Others can then see what "inline" comments you've made.

Or... from the top page where you are looking at the whole gerrit change,
there should be a "Review" button, which allows you to make general comments
on the overall change.


HTH,
David


 My
> patch did not include the installation of vtkMathConfigure.h because I
> don't know how to do that.  Marcus' suggestion of using vtkConfigure.h
> is maybe a better idea anyway?  What do we prefer?  I can update my patch.
>
> Cheers,
>
> --
> ____________________________________________________________
> Sean McBride, B. Eng                 sean at rogue-research.com
> Rogue Research                        www.rogue-research.com
> Mac Software Developer              Montréal, Québec, Canada
>
>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://www.vtk.org/mailman/listinfo/vtk-developers
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20110131/79270aae/attachment.html>