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
