Kent, You seem to bring up issues that get me thinking :)
Just use std::copy! Many version of STL should already do the type of optimization I suggested! And if very OOP, portable, and it's standard code we don't have to maintain! Brad On Apr 8, 2013, at 3:24 PM, Bradley Lowekamp <[email protected]> wrote: > Kent, > > Using memcpy to move a array of C++ object is a terrible idea. Previously I > have don't work to replace ImageIterator loops to do copy with memcpy. I saw > a 10-50X performance increase certainly for basic loop it won't be nearly a a > large of a difference. > > > Take a look at what I did here: > > http://www.itk.org/Doxygen/html/itkImageAlgorithm_8h_source.html > > I used TR1 type traits to determine if the pixel type was POD, if it was POD, > then it uses the memcpy implementation of the method. If you don't have TR1 > you get the slow method. I don't believe is_pod was every type where memcpy > could have been used, but is was an easy conservative traits. I think some of > the basic Array, classes in ITK it would work for but aren't detected as POD. > > I suggest you use the same approach for you de-optimization to suppress the > warnings. Perhaps you can find a place to make this method re-usable for > other situations as well. > > Brad > > > > On Apr 8, 2013, at 3:08 PM, "Williams, Norman K" > <[email protected]> wrote: > >> [Note, after writing this up I logged it as a bug. >> https://issues.itk.org/jira/browse/ITK-3021] >> >> One of our developers has encountered a compiler warning using ITK that >> seems to me to be >> >> A) a real problem and >> B) fixable >> >> In file included from >> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/inclu >> de/ITK-4.4/itkImportImageContainer.h:182: >> .../ITKv4-install/include/ITK-4.4/itkImportImageContainer.hxx:73:15: >> warning: destination for this 'memcpy' call is a pointer to dynamic class >> 'itk::HammerTissueAttributeVector'; >> vtable pointer will be >> overwritten [-Wdynamic-class-memaccess] >> memcpy( temp, m_ImportPointer, m_Size * sizeof( TElement ) ); >> ~~~~~~ ^ >> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/inclu >> de/ITK-4.4/itkImage.hxx:57:13: note: in instantiation of member function >> 'itk::ImportImageContainer<unsigned long, >> itk::HammerTissueAttributeVector>::Reserve' >> requested here >> m_Buffer->Reserve(num); >> >> Use of memcpy to move a C++ array is a terrible idea. It might work OK if >> A) the object class has no virtual methods B) All class members are plain >> ordinary data (e.g. scalars, arrays of scalars) - OR - C) the object >> class' member variables are either POD or class objects satisfying >> conditions A&B. >> >> I think the memcpy should be replaced by a 'for' loop that copies array >> elements one at a time. This will cause a compiler error if there is no >> operator= defined for the Pixel type, but if that is the case, using >> memcpy is double plus bad. >> >> This will silence the warnings and preserve C++ object semantics, and the >> perfomance penalty is not terrible. Particularly since the >> ImportImageContainer only uses memcpy in the case of growing or shrinking >> the container, which I can't imagine happens in the normal course of ITK >> usage. >> >> -- >> Kent Williams [email protected] >> >> >> >> >> >> >> On 4/8/13 12:08 PM, "Kim, Eun Young" <[email protected]> wrote: >> >>> Kent, >>> >>> >>> Hans told me to remove the warning, but not sure if that is even >>> possible. >>> Well, if there seems no simple solution, we could talk to Hans later. :) >>> >>> >>> Code: >>> BRAINSTools/BRAINSCut/BRAINSFeatureCreators/HammerAttributeCreator >>> >>> >>> Warning: >>> [eunyokim@wundt HammerAttributeCreator (myMaster +)]$ make -C >>> ../../../../releaseBuild/BRAINSTools-build/BRAINSCut/BRAINSFeatureCreators >>> /HammerAttributeCreator/ >>> Scanning dependencies of target HammerAttributeCreatorLib >>> Building CXX object >>> BRAINSCut/BRAINSFeatureCreators/HammerAttributeCreator/CMakeFiles/HammerAt >>> tributeCreatorLib.dir/HammerAttributeCreator.cxx.o >>> In file included from >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/BRAINSTools/BRAINSCut/BRAINSFea >>> tureCreators/HammerAttributeCreator/HammerAttributeCreator.cxx:11: >>> In file included from >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/incl >>> ude/ITK-4.4/itkImage.h:22: >>> In file included from >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/incl >>> ude/ITK-4.4/itkImportImageContainer.h:182: >>> .../ITKv4-install/include/ITK-4.4/itkImportImageContainer.hxx:73:15: >>> warning: destination for this 'memcpy' call is a pointer to dynamic class >>> 'itk::HammerTissueAttributeVector'; >>> vtable pointer will be >>> overwritten [-Wdynamic-class-memaccess] >>> memcpy( temp, m_ImportPointer, m_Size * sizeof( TElement ) ); >>> ~~~~~~ ^ >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/incl >>> ude/ITK-4.4/itkImage.hxx:57:13: note: in instantiation of member function >>> 'itk::ImportImageContainer<unsigned long, >>> itk::HammerTissueAttributeVector>::Reserve' >>> requested here >>> m_Buffer->Reserve(num); >>> ^ >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/incl >>> ude/ITK-4.4/itkImage.h:91:15: note: in instantiation of member function >>> 'itk::Image<itk::HammerTissueAttributeVector, 3>::Allocate' >>> requested here >>> itkNewMacro(Self); >>> ^ >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/incl >>> ude/ITK-4.4/itkMacro.h:165:21: note: expanded from macro 'itkNewMacro' >>> itkSimpleNewMacro(x) \ >>> ^ >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/incl >>> ude/ITK-4.4/itkMacro.h:175:22: note: expanded from macro >>> 'itkSimpleNewMacro' >>> smartPtr = new x; \ >>> ^ >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/incl >>> ude/ITK-4.4/itkImageSource.hxx:67:24: note: in instantiation of member >>> function 'itk::Image<itk::HammerTissueAttributeVector, 3>::New' >>> requested here >>> return TOutputImage::New().GetPointer(); >>> ^ >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/BRAINSTools/BRAINSCut/BRAINSFea >>> tureCreators/HammerAttributeCreator/HammerAttributeCreator.cxx:113:25: >>> note: in instantiation of member function >>> 'itk::ImageSource<itk::Image<itk::HammerTissueAttributeVector, 3> >>>> ::MakeOutput' requested here >>> modelAttributeFilter->SetStrength(Strength); >>> ^ >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/ITKv4-install/incl >>> ude/ITK-4.4/itkImportImageContainer.hxx:73:15: note: explicitly cast the >>> pointer to silence this warning >>> memcpy( temp, m_ImportPointer, m_Size * sizeof( TElement ) ); >>> ^ >>> (void*) >>> 1 warning generated. >>> Linking CXX shared library >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/bin/libHammerAttri >>> buteCreatorLib.dylib >>> Built target HammerAttributeCreatorLib >>> Linking CXX executable >>> /ipldev/scratch/eunyokim/src/BRAINS2013Apr/releaseBuild/bin/HammerAttribut >>> eCreator >>> Built target HammerAttributeCreator >>> >>> >>> >>> Just please take a look at the code when you have time. >>> >>> >>> Thank you! >>> >>> >>> Regina >> >> >> >> ________________________________ >> 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 > > _______________________________________________ > 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
