[Insight-developers] Performance Impact of using GetInput

Tue Jul 3 10:51:49 EDT 2012

Ok, 

If you haven't notice I tend to iterate on things until I think they are right. So yea, release early and often, is a philosophy I should keep in mind.

I believe that these performance issue were introduced with changes to ITKv4. I hope that ITKv4 doesn't give the impression of being slower than v3.

Should we at least prepare this as a patch for a 4.1.1 release?

Brad

On Jul 3, 2012, at 10:39 AM, Bill Lorensen wrote:

> 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
> 

========================================================
Bradley Lowekamp  
Medical Science and Computing for
Office of High Performance Computing and Communications
National Library of Medicine 
blowekamp at mail.nih.gov

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/pipermail/insight-developers/attachments/20120703/a1fe29bc/attachment.htm>