[Insight-developers] Performance Impact of using GetInput

Tue Jul 3 10:39:28 EDT 2012

I agree with Hans that we should not delay the release.

I also agree with Brad that we need to address this particular performance
issue. We should make this a high priority for the next release.

Bill

On Tue, Jul 3, 2012 at 10:35 AM, Johnson, Hans J <hans-johnson at uiowa.edu>wrote:

>   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
> 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>
> Date: Tuesday, July 3, 2012 9:26 AM
> To: ITK <insight-developers at itk.org>
> Cc: "Bill at public.kitware.com" <Bill at public.kitware.com>, Hans Johnson <
> hans.j.johnson at gmail.com>, Luis Ibanez <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
>
>
>
>
>
> ------------------------------
> 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.
> ------------------------------
>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.php
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
>
>

-- 
Unpaid intern in BillsBasement at noware dot com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/pipermail/insight-developers/attachments/20120703/97cdb825/attachment-0001.htm>