Major performance issue TimeProbe, ResourceProbe constructor

[N-Dekker]

Two weeks ago, Theo van Walsum (@tvanwalsum, Erasmus MC, Rotterdam) reported a major performance issue at discourse.itk.org:

I recently upgraded my elastix registration software, and simultaneously upgrading itk from 4.8 to the 4.13. This greatly affected the registration timings (3 to 4-fold slower). After some testing, I found it this problem starts with itk 4.9.

While Theo encountered the issue with ITK 4.8, using VS2013, it is still there with ITK 5 beta 3, using VS2017.

It appears that this performance issue is caused by an enhancement of itk::ResourceProbe, introduced with ITK 4.9. This enhancement includes calling GetSystemInformation() in the default-constructor of ResourceProbe: https://github.com/InsightSoftwareConsortium/ITK/blob/v4.9.0/Modules/Core/Common/include/itkResourceProbe.hxx#L51 Which causes a major performance drop of its derived classes, including itk::TimeProbe.

It can be reproduced by profiling the following example:

  double meanValues[263];

  for (auto&& meanValue : meanValues)
  {
    itk::TimeProbe timeProbe;
    timeProbe.Start();
    timeProbe.Stop();
    meanValue = timeProbe.GetMean();
  }

On Visual Studio 2017 Version 15.9.4 64-bit Release, I observed that this code takes more than 25 seconds. When I manually remove the GetSystemInformation() call from the default-constructor of ResourceProbe (at https://github.com/InsightSoftwareConsortium/ITK/blob/v5.0b03/Modules/Core/Common/include/itkResourceProbe.hxx#L52), the very same code takes less than 0.001 seconds!

[N-Dekker]

@hyunjaeKang I think this issue is related to an ITK contribution of yours, right? Can you please have a look? Specifically, do you think it would be OK to remove the GetSystemInformation() call from the default-constructor of itk::ResourceProbe?

[N-Dekker]

Update: I'm preparing a fix for this issue... please wait!

[N-Dekker]

Thank you very much for approving and merging pull request #351 to the master branch, @hjmjohnson . I usually don't ask this but would it be possible to also apply this fix to ITK 4? @thewtex ? I feel this fix could also be of interest to those ITK users who cannot yet move forward to ITK5/C++11.

[dzenanz]

To do that, you should cherry-pick this patch on top of release-4.13 branch, resolve conflicts (if any), and make a PR to release-4.13 branch.

[N-Dekker]

Thanks @dzenanz Are there still plans for another ITK 4 release?

[dzenanz]

Yes, there are plans for at last one more 4.13.x release.

[N-Dekker]

@dzenanz Thanks, I'll ask around here at LUMC if people care about having this performance fix in ITK4.

[thewtex]

Thanks for diving into this @N-Dekker !

Backported to release-4.13 here: #377