[Insight-developers] FW: BRAINSCut Warning

Mon Apr 8 15:24:47 EDT 2013

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" <norman-k-williams at uiowa.edu> 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 norman-k-williams at uiowa.edu
> 
> 
> 
> 
> 
> 
> On 4/8/13 12:08 PM, "Kim, Eun Young" <eunyoung-kim at uiowa.edu> 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 at 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