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 <[email protected]> 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 <[email protected]> > 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@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@neuron$ >> ~/Dashboard/src/ITK (master) >> johnsonhj@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. >> [email protected] >> 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 <[email protected]> >> Date: Tuesday, July 3, 2012 9:26 AM >> To: ITK <[email protected]> >> Cc: "[email protected]" <[email protected]>, Hans Johnson >> <[email protected]>, Luis Ibanez <[email protected]> >> 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 >> [email protected] >> >> >> >> >> >> ________________________________ >> 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 > > [email protected] > > > > > > _______________________________________________ > 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 >
_______________________________________________ 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
