I agree with Hans that we should not delay the release.<br><br>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.<br><br>Bill<br><br>
<div class="gmail_quote">On Tue, Jul 3, 2012 at 10:35 AM, Johnson, Hans J <span dir="ltr"><<a href="mailto:hans-johnson@uiowa.edu" target="_blank">hans-johnson@uiowa.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="font-size:14px;font-family:Calibri,sans-serif;word-wrap:break-word">
<div>
<div>
<div>Brad,</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>My guess is that the main culprits of those failing tests are related to following 15 lines of code. </div>
<div>
<div><br>
</div>
<div>==============================</div>
</div>
<div>
<div>johnsonhj@neuron$ git grep "\->GetInput()\->GetPixel"</div>
<div>Modules/Filtering/DistanceMap/include/itkSignedMaurerDistanceMapImageFilter.hxx: if ( this->GetInput()->GetPixel(idx) != this->m_BackgroundValue )</div>
<div>Modules/Filtering/ImageGrid/include/itkCyclicShiftImageFilter.hxx: outIt.Set( static_cast< OutputImagePixelType >( this->GetInput()->GetPixel( index ) ) );</div>
<div>Modules/Filtering/MathematicalMorphology/include/itkGrayscaleConnectedClosingImageFilter.hxx: seedValue = this->GetInput()->GetPixel(m_Seed);</div>
<div>Modules/Filtering/MathematicalMorphology/include/itkGrayscaleConnectedOpeningImageFilter.hxx: seedValue = this->GetInput()->GetPixel(m_Seed);</div>
<div>Modules/Numerics/Statistics/include/itkScalarImageToRunLengthMatrixFilter.hxx: pixelIntensity = this->GetInput()->GetPixel( index );</div>
<div>Modules/Numerics/Statistics/test/itkSubsampleTest.cxx: ArrayPixelImageType::PixelType pixel = filter->GetInput()->GetPixel(index);</div>
<div>Modules/Segmentation/Voronoi/include/itkVoronoiPartitioningImageFilter.hxx: getp = (double)( this->GetInput()->GetPixel(Plist[i]) );</div>
<div>Modules/Segmentation/Voronoi/include/itkVoronoiSegmentationImageFilter.hxx: getp = (double)( this->GetInput()->GetPixel(Plist[i]) );</div>
<div>~/Dashboard/src/ITK (master)</div>
<div><br>
</div>
<div>johnsonhj@neuron$ </div>
<div>~/Dashboard/src/ITK (master)</div>
<div>johnsonhj@neuron$ git grep "\->GetInput()\->Transform"</div>
<div>Modules/Core/Mesh/include/itkBinaryMask3DMeshSource.hxx: this->GetInput()->TransformContinuousIndexToPhysicalPoint(indTemp,new_p);</div>
<div>Modules/Nonunit/Review/include/itkScalarChanAndVeseDenseLevelSetImageFilter.hxx: this->GetInput()->TransformPhysicalPointToIndex(origin, start);</div>
<div>Modules/Nonunit/Review/include/itkScalarChanAndVeseSparseLevelSetImageFilter.hxx: this->GetInput()->TransformPhysicalPointToIndex(origin, start);</div>
<div>Modules/Nonunit/Review/include/itkStochasticFractalDimensionImageFilter.hxx: this->GetInput()->TransformIndexToPhysicalPoint(It.GetIndex(i), point1);</div>
<div>Modules/Nonunit/Review/include/itkStochasticFractalDimensionImageFilter.hxx: this->GetInput()->TransformIndexToPhysicalPoint(It.GetIndex(j), point2);</div>
<div>Modules/Numerics/Statistics/include/itkScalarImageToRunLengthMatrixFilter.hxx: this->GetInput()->TransformIndexToPhysicalPoint(</div>
<div>Modules/Numerics/Statistics/include/itkScalarImageToRunLengthMatrixFilter.hxx: this->GetInput()->TransformIndexToPhysicalPoint( lastGoodIndex, point );</div>
</div>
<div>==============================</div>
<div><br>
</div>
<div>Hans</div>
<div>-- </div>
<div>
<div>
<div>
<div>
<div style="font-family:Calibri;font-size:15px"><font face="Arial"><span style="font-size:10pt">Hans J. Johnson, Ph.D.</span></font></div>
<div style="font-family:Calibri;font-size:15px"><font face="Arial"><span style="font-size:10pt"><a href="mailto:hans-johnson@uiowa.edu" target="_blank">hans-johnson@uiowa.edu</a></span></font></div>
<div style="font-family:Calibri;font-size:15px"><font face="Arial"><span style="font-size:10pt">Assistant Professor of Psychiatry</span></font></div>
<div style="font-family:Calibri;font-size:15px"><span style="font-size:13px;font-family:Arial">University of Iowa Carver College of Medicine</span></div>
<div style="font-family:Calibri;font-size:15px"><font face="Arial"><span style="font-size:10pt"><span style="font-family:Calibri;font-size:15px">
<div><span style="font-size:13px;font-family:Arial">W278 GH, 200 Hawkins Drive</span></div>
</span></span></font></div>
<div style="font-family:Calibri;font-size:15px"><font face="Arial"><span style="font-size:10pt">Iowa City, Iowa 52242</span></font></div>
<div style="font-family:Calibri;font-size:15px"><font face="Arial"><span style="font-size:10pt">Phone: <a href="tel:319-353-8587" value="+13193538587" target="_blank">319-353-8587</a></span></font></div>
</div>
</div>
</div>
</div>
</div>
</div>
<div><br>
</div>
<span>
<div style="border-right:medium none;padding-right:0in;padding-left:0in;padding-top:3pt;text-align:left;font-size:11pt;border-bottom:medium none;font-family:Calibri;border-top:#b5c4df 1pt solid;padding-bottom:0in;border-left:medium none">
<span style="font-weight:bold">From: </span>Bradley Lowekamp <<a href="mailto:blowekamp@mail.nih.gov" target="_blank">blowekamp@mail.nih.gov</a>><br>
<span style="font-weight:bold">Date: </span>Tuesday, July 3, 2012 9:26 AM<br>
<span style="font-weight:bold">To: </span>ITK <<a href="mailto:insight-developers@itk.org" target="_blank">insight-developers@itk.org</a>><br>
<span style="font-weight:bold">Cc: </span>"<a href="mailto:Bill@public.kitware.com" target="_blank">Bill@public.kitware.com</a>" <<a href="mailto:Bill@public.kitware.com" target="_blank">Bill@public.kitware.com</a>>, Hans Johnson <<a href="mailto:hans.j.johnson@gmail.com" target="_blank">hans.j.johnson@gmail.com</a>>,
Luis Ibanez <<a href="mailto:luis.ibanez@kitware.com" target="_blank">luis.ibanez@kitware.com</a>><br>
<span style="font-weight:bold">Subject: </span>[Insight-developers] Performance Impact of using GetInput<br>
</div><div><div class="h5">
<div><br>
</div>
<div>
<div style="word-wrap:break-word">Hello,
<div><br>
</div>
<div>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:</div>
<div><br>
</div>
<div><a href="http://review.source.kitware.com/#/c/6367/" target="_blank">http://review.source.kitware.com/#/c/6367/</a></div>
<div><br>
</div>
<div>I find that difference to be quite significant difference, and is on the level of a bug.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div><a href="http://review.source.kitware.com/#/c/6369/" target="_blank">http://review.source.kitware.com/#/c/6369/</a></div>
<div><br>
</div>
<div><br>
</div>
<div>I get the following test failure (where previously there was none):</div>
<div><br>
</div>
<div>
<div><br>
</div>
<div>97% tests passed, 71 tests failed out of 2382</div>
</div>
<div><br>
</div>
<div>
<div>The following tests FAILED:</div>
<div><span style="white-space:pre-wrap"></span>160 - itkN4BiasFieldCorrectionImageFilterTest1 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>161 - itkN4BiasFieldCorrectionImageFilterTest2 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>311 - itkMultiThreaderEnvTest88 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>313 - itkMultiThreaderEnvTest123 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>398 - itkFFTConvolutionImageFilterTest4x4Mean (Failed)</div>
<div><span style="white-space:pre-wrap"></span>399 - itkFFTConvolutionImageFilterTest4x5Mean (Failed)</div>
<div><span style="white-space:pre-wrap"></span>400 - itkFFTConvolutionImageFilterTest5x5Mean (Failed)</div>
<div><span style="white-space:pre-wrap"></span>401 - itkFFTConvolutionImageFilterTest4x4MeanValidRegion (Failed)</div>
<div><span style="white-space:pre-wrap"></span>402 - itkFFTConvolutionImageFilterTest4x5MeanValidRegion (Failed)</div>
<div><span style="white-space:pre-wrap"></span>403 - itkFFTConvolutionImageFilterTest5x5MeanValidRegion (Failed)</div>
<div><span style="white-space:pre-wrap"></span>420 - itkRichardsonLucyDeconvolutionImageFilterGaussianKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>421 - itkRichardsonLucyDeconvolutionImageFilterIrregularKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>422 - itkLandweberDeconvolutionImageFilterGaussianKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>423 - itkLandweberDeconvolutionImageFilterIrregularKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>425 - itkProjectedLandweberDeconvolutionImageFilterGaussianKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>426 - itkProjectedLandweberDeconvolutionImageFilterIrregularKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>427 - itkInverseDeconvolutionImageFilterGaussianKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>428 - itkInverseDeconvolutionImageFilterIrregularKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>429 - itkTikhonovDeconvolutionImageFilterGaussianKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>430 - itkTikhonovDeconvolutionImageFilterIrregularKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>431 - itkWienerDeconvolutionImageFilterGaussianKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>432 - itkWienerDeconvolutionImageFilterIrregularKernelTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>433 - itkParametricBlindLeastSquaresDeconvolutionImageFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>436 - itkDeformableSimplexMesh3DBalloonForceFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>440 - itkPatchBasedDenoisingImageFilterTest0 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>441 - itkPatchBasedDenoisingImageFilterTestGaussian (Failed)</div>
<div><span style="white-space:pre-wrap"></span>442 - itkPatchBasedDenoisingImageFilterTestRician (Failed)</div>
<div><span style="white-space:pre-wrap"></span>443 - itkPatchBasedDenoisingImageFilterTestPoisson (Failed)</div>
<div><span style="white-space:pre-wrap"></span>521 - itkDisplacementFieldToBSplineImageFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>524 - itkContourMeanDistanceImageFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>525 - itkContourDirectedMeanDistanceImageFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>530 - itkHausdorffDistanceImageFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>532 - itkSignedMaurerDistanceMapImageFilterTest1 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>533 - itkSignedMaurerDistanceMapImageFilterTest2 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>656 - itkFastMarchingImageFilterTest_torus_multipleSeeds_NoTopo (Failed)</div>
<div><span style="white-space:pre-wrap"></span>657 - itkFastMarchingImageFilterTest_torus_multipleSeeds_StrictTopo (Failed)</div>
<div><span style="white-space:pre-wrap"></span>658 - itkFastMarchingImageFilterTest_torus_multipleSeeds_NoHandlesTopo (Failed)</div>
<div><span style="white-space:pre-wrap"></span>659 - itkFastMarchingImageFilterTest_wm_multipleSeeds_NoTopo (Failed)</div>
<div><span style="white-space:pre-wrap"></span>660 - itkFastMarchingImageFilterTest_wm_multipleSeeds_StrictTopo (Failed)</div>
<div><span style="white-space:pre-wrap"></span>661 - itkFastMarchingImageFilterTest_wm_multipleSeeds_NoHandlesTopo (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1072 - itkBSplineControlPointImageFilterTest2 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1079 - itkCyclicShiftImageFilterTest0 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1080 - itkCyclicShiftImageFilterTest1 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1081 - itkCyclicShiftImageFilterTest2 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1082 - itkCyclicShiftImageFilterTest3 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1083 - itkCyclicShiftImageFilterTest4 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1084 - itkCyclicShiftImageFilterTest5 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1085 - itkCyclicShiftImageFilterTest6 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1195 - itkModulusImageFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1377 - itkExtensionVelocitiesImageFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1378 - itkCannySegmentationLevelSetImageFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1412 - itkTwoLevelSetsv4DenseImage2DTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1471 - itkSimplexMeshVolumeCalculatorTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1659 - itkBinaryMask3DQuadEdgeMeshSourceTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1747 - itkPointSetToPointSetRegistrationTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1774 - itkDiffeomorphicDemonsRegistrationFilterTest01 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1775 - itkDiffeomorphicDemonsRegistrationFilterTest02 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1776 - itkDiffeomorphicDemonsRegistrationFilterTest03 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1777 - itkDiffeomorphicDemonsRegistrationFilterTest04 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1778 - itkDiffeomorphicDemonsRegistrationFilterTest05 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1779 - itkDiffeomorphicDemonsRegistrationFilterTest06 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1780 - itkDiffeomorphicDemonsRegistrationFilterTest07 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1781 - itkDiffeomorphicDemonsRegistrationFilterTest08 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1782 - itkDiffeomorphicDemonsRegistrationFilterTest09 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1783 - itkDiffeomorphicDemonsRegistrationFilterTest10 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1784 - itkDiffeomorphicDemonsRegistrationFilterTest11 (Failed)</div>
<div><span style="white-space:pre-wrap"></span>1802 - itkFastSymmetricForcesDemonsRegistrationFilterTest (Failed)</div>
<div><span style="white-space:pre-wrap"></span>2166 - itkVoronoiSegmentationImageFilterTest (Failed)</div>
</div>
<div><br>
</div>
<div><br>
</div>
<div>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?</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Thoughts on what to do?</div>
<div><br>
</div>
<div>Brad</div>
<div><br>
<div>
<div style="word-wrap:break-word;font-size:12px">
<p style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font style="font:normal normal normal 12px/normal Helvetica" face="Helvetica" size="3">========================================================</font></p>
<p style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font style="font:normal normal normal 12px/normal Helvetica" face="Helvetica" size="3">Bradley Lowekamp<span> </span><span> </span></font></p>
<p style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font style="font:normal normal normal 12px/normal Helvetica" face="Helvetica" size="3">Medical Science and Computing for</font></p>
<p style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font style="font:normal normal normal 12px/normal Helvetica" face="Helvetica" size="3">Office of High Performance Computing and Communications</font></p>
<p style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font style="font:normal normal normal 12px/normal Helvetica" face="Helvetica" size="3">National Library of Medicine<span> </span></font></p>
<p style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font style="font:normal normal normal 12px/normal Helvetica" face="Helvetica" size="3"><a href="mailto:blowekamp@mail.nih.gov" target="_blank">blowekamp@mail.nih.gov</a></font></p>
<br>
</div>
<br>
</div>
<br>
</div>
</div>
</div>
</div></div></span><br>
<br>
<hr>
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.
<hr>
</div>
<br>_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at<br>
<a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Kitware offers ITK Training Courses, for more information visit:<br>
<a href="http://kitware.com/products/protraining.php" target="_blank">http://kitware.com/products/protraining.php</a><br>
<br>
Please keep messages on-topic and check the ITK FAQ at:<br>
<a href="http://www.itk.org/Wiki/ITK_FAQ" target="_blank">http://www.itk.org/Wiki/ITK_FAQ</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.itk.org/mailman/listinfo/insight-developers" target="_blank">http://www.itk.org/mailman/listinfo/insight-developers</a><br>
<br></blockquote></div><br><br clear="all"><br>-- <br>Unpaid intern in BillsBasement at noware dot com<br><br>