From fc696f82f02147f771351367c6ed2a33ea02e519 Mon Sep 17 00:00:00 2001 From: Niels Dekker Date: Thu, 5 May 2022 21:52:51 +0200 Subject: [PATCH] STYLE: Declared `m_ComputePerThreadVariables` as an `std::vector` (2 x) Declared m_ComputePerThreadVariables of both as `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` as an `std::vector`, instead of a raw pointer to the structs. Simplified zero-initialization of the structs. Removed manual `delete[]` statements. Based on the second commit of pull request https://github.com/SuperElastix/elastix/pull/132 "Improved style of m_ComputePerThreadVariable data members". Follows C++ Core Guidelines (April 10, 2022): "Avoid calling new and delete explicitly" http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-ptr --- .../itkAdvancedImageMomentsCalculator.h | 11 +-- .../itkAdvancedImageMomentsCalculator.hxx | 67 ++++++------------- Common/itkComputeDisplacementDistribution.h | 7 +- Common/itkComputeDisplacementDistribution.hxx | 64 +++++------------- 4 files changed, 46 insertions(+), 103 deletions(-) diff --git a/Common/Transforms/itkAdvancedImageMomentsCalculator.h b/Common/Transforms/itkAdvancedImageMomentsCalculator.h index 5a5b117d5..a9987163b 100644 --- a/Common/Transforms/itkAdvancedImageMomentsCalculator.h +++ b/Common/Transforms/itkAdvancedImageMomentsCalculator.h @@ -32,6 +32,8 @@ #include "itkPlatformMultiThreader.h" +#include + namespace itk { /** \class AdvancedImageMomentsCalculator @@ -250,7 +252,7 @@ class ITK_TEMPLATE_EXPORT AdvancedImageMomentsCalculator : public Object protected: AdvancedImageMomentsCalculator(); - ~AdvancedImageMomentsCalculator() override; + ~AdvancedImageMomentsCalculator() override = default; void PrintSelf(std::ostream & os, Indent indent) const override; @@ -310,10 +312,9 @@ class ITK_TEMPLATE_EXPORT AdvancedImageMomentsCalculator : public Object mutable MultiThreaderParameterType m_ThreaderParameters; - mutable AlignedComputePerThreadStruct * m_ComputePerThreadVariables; - mutable ThreadIdType m_ComputePerThreadVariablesSize; - bool m_UseMultiThread; - SizeValueType m_NumberOfPixelsCounted; + mutable std::vector m_ComputePerThreadVariables; + bool m_UseMultiThread; + SizeValueType m_NumberOfPixelsCounted; SizeValueType m_NumberOfSamplesForCenteredTransformInitialization; InputPixelType m_LowerThresholdForCenterGravity; diff --git a/Common/Transforms/itkAdvancedImageMomentsCalculator.hxx b/Common/Transforms/itkAdvancedImageMomentsCalculator.hxx index 546735a71..28ddbf582 100644 --- a/Common/Transforms/itkAdvancedImageMomentsCalculator.hxx +++ b/Common/Transforms/itkAdvancedImageMomentsCalculator.hxx @@ -51,21 +51,11 @@ AdvancedImageMomentsCalculator::AdvancedImageMomentsCalculator() this->m_ThreaderParameters.st_Self = this; // Multi-threading structs - this->m_ComputePerThreadVariables = nullptr; - this->m_ComputePerThreadVariablesSize = 0; this->m_CenterOfGravityUsesLowerThreshold = false; this->m_NumberOfSamplesForCenteredTransformInitialization = 10000; this->m_LowerThresholdForCenterGravity = 500; } -//---------------------------------------------------------------------- -// Destructor -template -AdvancedImageMomentsCalculator::~AdvancedImageMomentsCalculator() -{ - delete[] this->m_ComputePerThreadVariables; -} - /** * ************************* InitializeThreadingParameters ************************ */ @@ -85,24 +75,8 @@ AdvancedImageMomentsCalculator::InitializeThreadingParameters() */ const ThreadIdType numberOfThreads = this->m_Threader->GetNumberOfWorkUnits(); - /** Only resize the array of structs when needed. */ - if (this->m_ComputePerThreadVariablesSize != numberOfThreads) - { - delete[] this->m_ComputePerThreadVariables; - this->m_ComputePerThreadVariables = new AlignedComputePerThreadStruct[numberOfThreads]; - this->m_ComputePerThreadVariablesSize = numberOfThreads; - } - - /** Some initialization. */ - for (ThreadIdType i = 0; i < numberOfThreads; ++i) - { - this->m_ComputePerThreadVariables[i].st_M0 = NumericTraits::Zero; - this->m_ComputePerThreadVariables[i].st_M1 = NumericTraits::Zero; - this->m_ComputePerThreadVariables[i].st_M2.Fill(NumericTraits::ZeroValue()); - this->m_ComputePerThreadVariables[i].st_Cg = NumericTraits::Zero; - this->m_ComputePerThreadVariables[i].st_Cm.Fill(NumericTraits::ZeroValue()); - this->m_ComputePerThreadVariables[i].st_NumberOfPixelsCounted = NumericTraits::Zero; - } + // For each thread, assign a struct of zero-initialized values. + m_ComputePerThreadVariables.assign(numberOfThreads, AlignedComputePerThreadStruct()); } // end InitializeThreadingParameters() @@ -337,12 +311,14 @@ AdvancedImageMomentsCalculator::ThreadedCompute(ThreadIdType threadId) } } /** Update the thread struct once. */ - this->m_ComputePerThreadVariables[threadId].st_M0 = M0; - this->m_ComputePerThreadVariables[threadId].st_M1 = M1; - this->m_ComputePerThreadVariables[threadId].st_M2 = M2; - this->m_ComputePerThreadVariables[threadId].st_Cg = Cg; - this->m_ComputePerThreadVariables[threadId].st_Cm = Cm; - this->m_ComputePerThreadVariables[threadId].st_NumberOfPixelsCounted = numberOfPixelsCounted; + AlignedComputePerThreadStruct computePerThreadStruct; + computePerThreadStruct.st_M0 = M0; + computePerThreadStruct.st_M1 = M1; + computePerThreadStruct.st_M2 = M2; + computePerThreadStruct.st_Cg = Cg; + computePerThreadStruct.st_Cm = Cm; + computePerThreadStruct.st_NumberOfPixelsCounted = numberOfPixelsCounted; + m_ComputePerThreadVariables[threadId] = computePerThreadStruct; } // end ThreadedCompute() @@ -354,25 +330,24 @@ template void AdvancedImageMomentsCalculator::AfterThreadedCompute() { - const ThreadIdType numberOfThreads = this->m_Threader->GetNumberOfWorkUnits(); /** Accumulate thread results. */ - for (ThreadIdType k = 0; k < numberOfThreads; ++k) + for (auto & computePerThreadStruct : m_ComputePerThreadVariables) { - this->m_M0 += this->m_ComputePerThreadVariables[k].st_M0; + this->m_M0 += computePerThreadStruct.st_M0; for (unsigned int i = 0; i < ImageDimension; ++i) { - this->m_M1[i] += this->m_ComputePerThreadVariables[k].st_M1[i]; - this->m_Cg[i] += this->m_ComputePerThreadVariables[k].st_Cg[i]; - this->m_ComputePerThreadVariables[k].st_M1[i] = 0; - this->m_ComputePerThreadVariables[k].st_Cg[i] = 0; + this->m_M1[i] += computePerThreadStruct.st_M1[i]; + this->m_Cg[i] += computePerThreadStruct.st_Cg[i]; + computePerThreadStruct.st_M1[i] = 0; + computePerThreadStruct.st_Cg[i] = 0; for (unsigned int j = 0; j < ImageDimension; ++j) { - this->m_M2[i][j] += this->m_ComputePerThreadVariables[k].st_M2[i][j]; - this->m_Cm[i][j] += this->m_ComputePerThreadVariables[k].st_Cm[i][j]; - this->m_ComputePerThreadVariables[k].st_M2[i][j] = 0; - this->m_ComputePerThreadVariables[k].st_Cm[i][j] = 0; + this->m_M2[i][j] += computePerThreadStruct.st_M2[i][j]; + this->m_Cm[i][j] += computePerThreadStruct.st_Cm[i][j]; + computePerThreadStruct.st_M2[i][j] = 0; + computePerThreadStruct.st_Cm[i][j] = 0; } - this->m_ComputePerThreadVariables[k].st_M0 = 0; + computePerThreadStruct.st_M0 = 0; } } DoPostProcessing(); diff --git a/Common/itkComputeDisplacementDistribution.h b/Common/itkComputeDisplacementDistribution.h index 444026187..63136b62b 100644 --- a/Common/itkComputeDisplacementDistribution.h +++ b/Common/itkComputeDisplacementDistribution.h @@ -26,6 +26,8 @@ #include "itkImageFullSampler.h" #include "itkPlatformMultiThreader.h" +#include + namespace itk { /**\class ComputeDisplacementDistribution @@ -132,7 +134,7 @@ class ITK_TEMPLATE_EXPORT ComputeDisplacementDistribution : public ScaledSingleV protected: ComputeDisplacementDistribution(); - ~ComputeDisplacementDistribution() override; + ~ComputeDisplacementDistribution() override = default; /** Typedefs for multi-threading. */ using ThreaderType = itk::PlatformMultiThreader; @@ -214,8 +216,7 @@ class ITK_TEMPLATE_EXPORT ComputeDisplacementDistribution : public ScaledSingleV private: mutable MultiThreaderParameterType m_ThreaderParameters; - mutable AlignedComputePerThreadStruct * m_ComputePerThreadVariables; - mutable ThreadIdType m_ComputePerThreadVariablesSize; + mutable std::vector m_ComputePerThreadVariables; SizeValueType m_NumberOfPixelsCounted; bool m_UseMultiThread; diff --git a/Common/itkComputeDisplacementDistribution.hxx b/Common/itkComputeDisplacementDistribution.hxx index 7805b8130..33ff314c6 100644 --- a/Common/itkComputeDisplacementDistribution.hxx +++ b/Common/itkComputeDisplacementDistribution.hxx @@ -56,24 +56,9 @@ ComputeDisplacementDistribution::ComputeDisplacementDis /** Initialize the m_ThreaderParameters. */ this->m_ThreaderParameters.st_Self = this; - // Multi-threading structs - this->m_ComputePerThreadVariables = nullptr; - this->m_ComputePerThreadVariablesSize = 0; - } // end Constructor -/** - * ************************* Destructor ************************ - */ - -template -ComputeDisplacementDistribution::~ComputeDisplacementDistribution() -{ - delete[] this->m_ComputePerThreadVariables; -} // end Destructor - - /** * ************************* InitializeThreadingParameters ************************ */ @@ -93,22 +78,8 @@ ComputeDisplacementDistribution::InitializeThreadingPar */ const ThreadIdType numberOfThreads = this->m_Threader->GetNumberOfWorkUnits(); - /** Only resize the array of structs when needed. */ - if (this->m_ComputePerThreadVariablesSize != numberOfThreads) - { - delete[] this->m_ComputePerThreadVariables; - this->m_ComputePerThreadVariables = new AlignedComputePerThreadStruct[numberOfThreads]; - this->m_ComputePerThreadVariablesSize = numberOfThreads; - } - - /** Some initialization. */ - for (ThreadIdType i = 0; i < numberOfThreads; ++i) - { - this->m_ComputePerThreadVariables[i].st_MaxJJ = 0.0; - this->m_ComputePerThreadVariables[i].st_Displacement = 0.0; - this->m_ComputePerThreadVariables[i].st_DisplacementSquared = 0.0; - this->m_ComputePerThreadVariables[i].st_NumberOfPixelsCounted = NumericTraits::Zero; - } + // For each thread, assign a struct of zero-initialized values. + m_ComputePerThreadVariables.assign(numberOfThreads, AlignedComputePerThreadStruct()); } // end InitializeThreadingParameters() @@ -450,11 +421,12 @@ ComputeDisplacementDistribution::ThreadedCompute(Thread } /** Update the thread struct once. */ - this->m_ComputePerThreadVariables[threadId].st_MaxJJ = maxJJ; - this->m_ComputePerThreadVariables[threadId].st_Displacement = displacement; - this->m_ComputePerThreadVariables[threadId].st_DisplacementSquared = displacementSquared; - this->m_ComputePerThreadVariables[threadId].st_NumberOfPixelsCounted = numberOfPixelsCounted; - + AlignedComputePerThreadStruct computePerThreadStruct; + computePerThreadStruct.st_MaxJJ = maxJJ; + computePerThreadStruct.st_Displacement = displacement; + computePerThreadStruct.st_DisplacementSquared = displacementSquared; + computePerThreadStruct.st_NumberOfPixelsCounted = numberOfPixelsCounted; + m_ComputePerThreadVariables[threadId] = computePerThreadStruct; } // end ThreadedCompute() @@ -466,8 +438,6 @@ template void ComputeDisplacementDistribution::AfterThreadedCompute(double & jacg, double & maxJJ) { - const ThreadIdType numberOfThreads = this->m_Threader->GetNumberOfWorkUnits(); - /** Reset all variables. */ maxJJ = 0.0; double displacement = 0.0; @@ -475,19 +445,15 @@ ComputeDisplacementDistribution::AfterThreadedCompute(d this->m_NumberOfPixelsCounted = 0.0; /** Accumulate thread results. */ - for (ThreadIdType i = 0; i < numberOfThreads; ++i) + for (const auto & computePerThreadStruct : m_ComputePerThreadVariables) { - maxJJ = std::max(maxJJ, this->m_ComputePerThreadVariables[i].st_MaxJJ); - displacement += this->m_ComputePerThreadVariables[i].st_Displacement; - displacementSquared += this->m_ComputePerThreadVariables[i].st_DisplacementSquared; - this->m_NumberOfPixelsCounted += this->m_ComputePerThreadVariables[i].st_NumberOfPixelsCounted; - - /** Reset all variables for the next resolution. */ - this->m_ComputePerThreadVariables[i].st_MaxJJ = 0; - this->m_ComputePerThreadVariables[i].st_Displacement = 0; - this->m_ComputePerThreadVariables[i].st_DisplacementSquared = 0; - this->m_ComputePerThreadVariables[i].st_NumberOfPixelsCounted = 0; + maxJJ = std::max(maxJJ, computePerThreadStruct.st_MaxJJ); + displacement += computePerThreadStruct.st_Displacement; + displacementSquared += computePerThreadStruct.st_DisplacementSquared; + this->m_NumberOfPixelsCounted += computePerThreadStruct.st_NumberOfPixelsCounted; } + // Reset all variables for the next resolution. + std::fill_n(m_ComputePerThreadVariables.begin(), m_ComputePerThreadVariables.size(), AlignedComputePerThreadStruct()); /** Compute the sigma of the distribution of the displacements. */ const double meanDisplacement = displacement / this->m_NumberOfPixelsCounted;