[Insight-developers] Performance Impact of using GetInput

[Johnson, Hans J]

Brad,

My opinion is that this has been an issue for many years, and that the cost benefit is not worth trying to squeeze this into this release cycle.  The results are (or at least should be) the same both before and after this change.

The backlog of new features and other performance improvements are beginning to back up on the gerrit dashboard, so I would definitely vote for cutting ITKv4.2 as soon as possible so that all other projects can start moving forward again.

My guess is that the main culprits of those failing tests are related to following 15 lines of code.

==============================
johnsonhj at neuron$ git grep "\->GetInput()\->GetPixel"
Modules/Filtering/DistanceMap/include/itkSignedMaurerDistanceMapImageFilter.hxx:    if ( this->GetInput()->GetPixel(idx) != this->m_BackgroundValue )
Modules/Filtering/ImageGrid/include/itkCyclicShiftImageFilter.hxx:    outIt.Set( static_cast< OutputImagePixelType >( this->GetInput()->GetPixel( index ) ) );
Modules/Filtering/MathematicalMorphology/include/itkGrayscaleConnectedClosingImageFilter.hxx:  seedValue = this->GetInput()->GetPixel(m_Seed);
Modules/Filtering/MathematicalMorphology/include/itkGrayscaleConnectedOpeningImageFilter.hxx:  seedValue = this->GetInput()->GetPixel(m_Seed);
Modules/Numerics/Statistics/include/itkScalarImageToRunLengthMatrixFilter.hxx:        pixelIntensity = this->GetInput()->GetPixel( index );
Modules/Numerics/Statistics/test/itkSubsampleTest.cxx:  ArrayPixelImageType::PixelType pixel = filter->GetInput()->GetPixel(index);
Modules/Segmentation/Voronoi/include/itkVoronoiPartitioningImageFilter.hxx:    getp = (double)( this->GetInput()->GetPixel(Plist[i]) );
Modules/Segmentation/Voronoi/include/itkVoronoiSegmentationImageFilter.hxx:    getp = (double)( this->GetInput()->GetPixel(Plist[i]) );
~/Dashboard/src/ITK (master)

johnsonhj at neuron$
~/Dashboard/src/ITK (master)
johnsonhj at neuron$ git grep "\->GetInput()\->Transform"
Modules/Core/Mesh/include/itkBinaryMask3DMeshSource.hxx:      this->GetInput()->TransformContinuousIndexToPhysicalPoint(indTemp,new_p);
Modules/Nonunit/Review/include/itkScalarChanAndVeseDenseLevelSetImageFilter.hxx:    this->GetInput()->TransformPhysicalPointToIndex(origin, start);
Modules/Nonunit/Review/include/itkScalarChanAndVeseSparseLevelSetImageFilter.hxx:    this->GetInput()->TransformPhysicalPointToIndex(origin, start);
Modules/Nonunit/Review/include/itkStochasticFractalDimensionImageFilter.hxx:          this->GetInput()->TransformIndexToPhysicalPoint(It.GetIndex(i), point1);
Modules/Nonunit/Review/include/itkStochasticFractalDimensionImageFilter.hxx:              this->GetInput()->TransformIndexToPhysicalPoint(It.GetIndex(j), point2);
Modules/Numerics/Statistics/include/itkScalarImageToRunLengthMatrixFilter.hxx:      this->GetInput()->TransformIndexToPhysicalPoint(
Modules/Numerics/Statistics/include/itkScalarImageToRunLengthMatrixFilter.hxx:      this->GetInput()->TransformIndexToPhysicalPoint( lastGoodIndex, point );
==============================

Hans
--
Hans J. Johnson, Ph.D.
hans-johnson at uiowa.edu<mailto:hans-johnson at uiowa.edu>
Assistant Professor of Psychiatry
University of Iowa Carver College of Medicine
W278 GH, 200 Hawkins Drive
Iowa City, Iowa 52242
Phone:  319-353-8587

From: Bradley Lowekamp <blowekamp at mail.nih.gov<mailto:blowekamp at mail.nih.gov>>
Date: Tuesday, July 3, 2012 9:26 AM
To: ITK <insight-developers at itk.org<mailto:insight-developers at itk.org>>
Cc: "Bill at public.kitware.com<mailto:Bill at public.kitware.com>" <Bill at public.kitware.com<mailto:Bill at public.kitware.com>>, Hans Johnson <hans.j.johnson at gmail.com<mailto:hans.j.johnson at gmail.com>>, Luis Ibanez <luis.ibanez at kitware.com<mailto:luis.ibanez at kitware.com>>
Subject: [Insight-developers] Performance Impact of using GetInput

Hello,

A user yesterday, was reporting that going from ITK 3.20 to ITK 4.1, the SignedMaurerDistanceMapImageFilter was running more that 2x-3x the time. With a little bit of poking around and sampling the run time, I was able to develop the following patch:

http://review.source.kitware.com/#/c/6367/

I find that difference to be quite significant difference, and is on the level of a bug.

The lead me to wonder how wide spread is this incorrect usage. So I added an atomic counter to the GetInput, and GetOutput methods, and when they exceed a threshold, an exception is throw. This is to detect when these methods may be used in an inner loop.

http://review.source.kitware.com/#/c/6369/


I get the following test failure (where previously there was none):


97% tests passed, 71 tests failed out of 2382

The following tests FAILED:
160 - itkN4BiasFieldCorrectionImageFilterTest1 (Failed)
161 - itkN4BiasFieldCorrectionImageFilterTest2 (Failed)
311 - itkMultiThreaderEnvTest88 (Failed)
313 - itkMultiThreaderEnvTest123 (Failed)
398 - itkFFTConvolutionImageFilterTest4x4Mean (Failed)
399 - itkFFTConvolutionImageFilterTest4x5Mean (Failed)
400 - itkFFTConvolutionImageFilterTest5x5Mean (Failed)
401 - itkFFTConvolutionImageFilterTest4x4MeanValidRegion (Failed)
402 - itkFFTConvolutionImageFilterTest4x5MeanValidRegion (Failed)
403 - itkFFTConvolutionImageFilterTest5x5MeanValidRegion (Failed)
420 - itkRichardsonLucyDeconvolutionImageFilterGaussianKernelTest (Failed)
421 - itkRichardsonLucyDeconvolutionImageFilterIrregularKernelTest (Failed)
422 - itkLandweberDeconvolutionImageFilterGaussianKernelTest (Failed)
423 - itkLandweberDeconvolutionImageFilterIrregularKernelTest (Failed)
425 - itkProjectedLandweberDeconvolutionImageFilterGaussianKernelTest (Failed)
426 - itkProjectedLandweberDeconvolutionImageFilterIrregularKernelTest (Failed)
427 - itkInverseDeconvolutionImageFilterGaussianKernelTest (Failed)
428 - itkInverseDeconvolutionImageFilterIrregularKernelTest (Failed)
429 - itkTikhonovDeconvolutionImageFilterGaussianKernelTest (Failed)
430 - itkTikhonovDeconvolutionImageFilterIrregularKernelTest (Failed)
431 - itkWienerDeconvolutionImageFilterGaussianKernelTest (Failed)
432 - itkWienerDeconvolutionImageFilterIrregularKernelTest (Failed)
433 - itkParametricBlindLeastSquaresDeconvolutionImageFilterTest (Failed)
436 - itkDeformableSimplexMesh3DBalloonForceFilterTest (Failed)
440 - itkPatchBasedDenoisingImageFilterTest0 (Failed)
441 - itkPatchBasedDenoisingImageFilterTestGaussian (Failed)
442 - itkPatchBasedDenoisingImageFilterTestRician (Failed)
443 - itkPatchBasedDenoisingImageFilterTestPoisson (Failed)
521 - itkDisplacementFieldToBSplineImageFilterTest (Failed)
524 - itkContourMeanDistanceImageFilterTest (Failed)
525 - itkContourDirectedMeanDistanceImageFilterTest (Failed)
530 - itkHausdorffDistanceImageFilterTest (Failed)
532 - itkSignedMaurerDistanceMapImageFilterTest1 (Failed)
533 - itkSignedMaurerDistanceMapImageFilterTest2 (Failed)
656 - itkFastMarchingImageFilterTest_torus_multipleSeeds_NoTopo (Failed)
657 - itkFastMarchingImageFilterTest_torus_multipleSeeds_StrictTopo (Failed)
658 - itkFastMarchingImageFilterTest_torus_multipleSeeds_NoHandlesTopo (Failed)
659 - itkFastMarchingImageFilterTest_wm_multipleSeeds_NoTopo (Failed)
660 - itkFastMarchingImageFilterTest_wm_multipleSeeds_StrictTopo (Failed)
661 - itkFastMarchingImageFilterTest_wm_multipleSeeds_NoHandlesTopo (Failed)
1072 - itkBSplineControlPointImageFilterTest2 (Failed)
1079 - itkCyclicShiftImageFilterTest0 (Failed)
1080 - itkCyclicShiftImageFilterTest1 (Failed)
1081 - itkCyclicShiftImageFilterTest2 (Failed)
1082 - itkCyclicShiftImageFilterTest3 (Failed)
1083 - itkCyclicShiftImageFilterTest4 (Failed)
1084 - itkCyclicShiftImageFilterTest5 (Failed)
1085 - itkCyclicShiftImageFilterTest6 (Failed)
1195 - itkModulusImageFilterTest (Failed)
1377 - itkExtensionVelocitiesImageFilterTest (Failed)
1378 - itkCannySegmentationLevelSetImageFilterTest (Failed)
1412 - itkTwoLevelSetsv4DenseImage2DTest (Failed)
1471 - itkSimplexMeshVolumeCalculatorTest (Failed)
1659 - itkBinaryMask3DQuadEdgeMeshSourceTest (Failed)
1747 - itkPointSetToPointSetRegistrationTest (Failed)
1774 - itkDiffeomorphicDemonsRegistrationFilterTest01 (Failed)
1775 - itkDiffeomorphicDemonsRegistrationFilterTest02 (Failed)
1776 - itkDiffeomorphicDemonsRegistrationFilterTest03 (Failed)
1777 - itkDiffeomorphicDemonsRegistrationFilterTest04 (Failed)
1778 - itkDiffeomorphicDemonsRegistrationFilterTest05 (Failed)
1779 - itkDiffeomorphicDemonsRegistrationFilterTest06 (Failed)
1780 - itkDiffeomorphicDemonsRegistrationFilterTest07 (Failed)
1781 - itkDiffeomorphicDemonsRegistrationFilterTest08 (Failed)
1782 - itkDiffeomorphicDemonsRegistrationFilterTest09 (Failed)
1783 - itkDiffeomorphicDemonsRegistrationFilterTest10 (Failed)
1784 - itkDiffeomorphicDemonsRegistrationFilterTest11 (Failed)
1802 - itkFastSymmetricForcesDemonsRegistrationFilterTest (Failed)
2166 - itkVoronoiSegmentationImageFilterTest (Failed)


How big of a deal if most of the filters here are running 2x+ slower then what they should be? Is it big enough to delay the Release and do another RC with the fixes?

I have also been looking at the methods used in GetInput, specifically the methods used to create the std::string... It seems to be if we change the return value to a const std::string &, then we could keep a static internal table of the common value and return reference to the static table to even, references to what is in the std::map, the would reduce the need for mallocs for std::string.

Thoughts on what to do?

Brad


========================================================

Bradley Lowekamp

Medical Science and Computing for

Office of High Performance Computing and Communications

National Library of Medicine

blowekamp at mail.nih.gov<mailto:blowekamp at mail.nih.gov>





________________________________
Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
________________________________
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/pipermail/insight-developers/attachments/20120703/efe0a718/attachment.htm>