[Insight-developers] Performance Impact of using GetInput

Tue Jul 3 14:03:51 EDT 2012

Clever way of identifying the problem!  100000 is possibly catching only
the low hanging fruit.  There may be even more culprits.

I concur with the other community members that these fixes are not
show-stoppers for 4.2, and we can hammer them all out for an nice 4.3.

A 4.1.1 release would be nice as would a 3.20.2 release, but it is hard to
find the resources for them...  I believe the Debian gentleman and ladies
are packaging 4.1 for wheezy, and they might appreciate an easy-to-apply
patch, though.

Thanks,
Matt

On Tue, Jul 3, 2012 at 2:51 PM, Bradley Lowekamp <blowekamp at mail.nih.gov>
wrote:
> 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
>
>
>
>
>
> _______________________________________________
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/pipermail/insight-developers/attachments/20120703/3a5f9bb9/attachment.htm>