On Tue, Sep 25, 2012 at 8:01 AM, <[email protected]> wrote: > Hi Matt, > > Thanks for the feedback. The two suggestions don't help in my case. Calling > Update() one or more times has the same result, nothing happens. Also, I am > using the function CreateElementAt() which really modifies the container, so > no circumvention of Modified() there. But the feedback is well appreciated. > > --- > > Regarding the place of the ModifiedTimeType typedef, to put it in itkIntTypes > or somewhere else: The function GetMTime is re-defined at many places > throughout the toolkit. This means I would have to propagate the typedef to > all these classes (typedef typename Superclass::ModifiedTimeType > ModifiedTimeType).' > > In addition, the situation is actually worse. DataObject has a public function > void SetPipelineMTime(ModifiedTimeType time) > so it also needs this typedef. > Derived classes of ProcessObject also use it. > Object has virtual ModifiedTimeType GetMTime() const; > also public. > This means that all derived classes from Object have the public function > GetMTime, so people can call > someType mtime = derivedClass->GetMTime(); > So, they need to know that someType == ModifiedTimeType, which they should be > able to get through > typedef DerivedClassType:: ModifiedTimeType ModifiedTimeType; > which means that we would have to copy the typedef to all classes in the > toolkit. > > It is easier to add one typedef to itkIntTypes. > > What are your thoughts?
Hi Marius, In this light, a typedef in itkIntTypes.h sounds reasonable to me. Thanks, Matt > > Regards, Marius > > -----Original Message----- > From: Matt McCormick [mailto:[email protected]] > Sent: maandag 24 september 2012 22:56 > To: Sean McBride > Cc: Bradley Lowekamp; [email protected]; Staring, M. (LKEB) > Subject: Re: [Insight-developers] type of modification time > > Hi Marius, > > I encountered a similar issue when helping with the FARSIGHT project. > In that case, the modified time would rollover in the context of ITK filters > used in OpenMP threads. When the rollover happened, the pipeline did not > update correctly, which predictably caused an "region not available" type of > exception. The workaround in that case was simply call Update() again. The > code to do this: > > > https://farsight-svn.ee.uh.edu/repos/farsight/trunk/ftkCommon/ftkTimeStampOverflowSafeUpdate.h > > https://farsight-svn.ee.uh.edu/repos/farsight/trunk/ftkCommon/ftkTimeStampOverflowSafeUpdate.hxx > > How the fix was tested: > > > https://farsight-svn.ee.uh.edu/repos/farsight/trunk/ftkCommon/itkRolloverOpenMPTest.cpp > > That may or may not help in your case. > > > On a related tangent: yesterday I merged a patch submitted by Ho related to > VectorContainer excessive calls to Modified(). There are const and non-const > versions of methods in VectorContainer, and the compiler was not smart enough > to pick the const version. This resulted in excessive calls to Modified(), > an associated performance hit, and progression towards the modified time > rollover. The problem was detected by examining valgrind/callgrind output, > and the solution was to force the compiler to use the const version with a > cast. More information here: > > http://review.source.kitware.com/#/c/7603/ > > > Whether or not those two approaches help with your case, the patch you are > proposing is a good step forward. More typedefs instead of hard-coded types > are helpful. As Brad L. remarked, it would be better if the typedef was a > member of the itk::TimeStamp class since it is specific to this class. As > Sean remarked, the type of this typedef is going to depend on the platform, > and we may get 64 bit support for at least some platforms and open the > possibility for 64 bit modified time in the future. > > Thanks, > Matt > > On Mon, Sep 24, 2012 at 5:42 PM, Sean McBride <[email protected]> wrote: >> On Mon, 24 Sep 2012 14:43:03 +0000, Matt McCormick said: >> >>>The type of the modified time is limited by the platform-specific >>>functions to perform the atomic operation that increments it. We are >>>limited to the type used in the platform specific functions, and I do >>>not think what you are proposing will work (although I would be happy >>>to be proved wrong :-) ). >> >> Indeed, it may be tricky for that reason. >> >> Also, on OS X, the OSAtomicIncrement64() function is available on 64 bit >> PowerPC, but not 32 bit PowerPC... OSAtomicIncrement32 is available on both, >> which is what is used now. We could always fall back to the slow mutex on >> PPC32 I guess. >> >> One portable thing that could be done is to use C++11's new atomic stuff, >> and fall back to the platform-specific code only if the compiler does not >> support C++11. Few compilers support it yet, but it's more future-proof. >> >> Cheers, >> >> -- >> ____________________________________________________________ >> Sean McBride, B. Eng [email protected] >> Rogue Research www.rogue-research.com >> Mac Software Developer Montréal, Québec, Canada >> >> _______________________________________________ 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
