[Insight-developers] type of modification time

Mon Sep 24 10:56:51 EDT 2012

Hi Matt,

In my case I have the problem outside threads, it simply overflows.

But it may indeed still be a problem, giving a second look at itkTimeStamp.cxx. Mutex locks are indeed used there (I think regardless if you are using threads or not), and I see "optimized" code that uses special function that work with special types (but not ULL type).

The general case from that file should still work though: it declares a type (can thus be modified to be ULL), locks it, increments it, and unlocks it.

What is the purpose of the optimizations, performance?

I am interested in your work-around.

Regards, Marius

-----Original Message-----
From: Matt McCormick [mailto:matt.mccormick at kitware.com] 
Sent: maandag 24 september 2012 16:43
To: Bradley Lowekamp
Cc: Staring, M. (LKEB); insight-developers at itk.org; Ho Cheung
Subject: Re: [Insight-developers] type of modification time

Hi Marius,

I have also encountered this rollover problem.  If I recall correctly, it was only a problem when using ITK inside threads.  Is this your case?

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 :-) ).

I came up with alternative work-around to this problem that I can explain later today.

Thanks,
Matt

On Mon, Sep 24, 2012 at 1:51 PM, Bradley Lowekamp <blowekamp at mail.nih.gov> wrote:
> Marius,
>
> I think adding the ModifiedTimeType type is a very good idea. However, 
> I think I would prefer to put this type as a type def of the TimeStamp class.
>
> Adding this type def would make a great first step towards updating 
> this type. I have not looked into the detail of all the atomic 
> operations to know if what you proposed is plausible. But I think you  
> could make a patch to gerrit with this first step either way.
>
> I also found this interesting:
>
> $ git grep "GetMTime" | wc -l
>      292
>
> 300 seems like it may be do able to quickly check the usage of these 
> methods too.
>
> Brad
>
> On Sep 24, 2012, at 9:28 AM, "M.Staring at lumc.nl" <M.Staring at lumc.nl> wrote:
>
> Hi Brad,
>
> The std::vector may indeed be a good alternative. I made the filter 
> that uses the itk::VectorContainer a long time ago, and I forgot why I 
> chose one over the other. It is going to be some work for me to change 
> this. I will have a look to guess how much work that will be.
>
>
> Regarding the timestamp type. What I thought that could be done is to 
> define a type in itkIntTypes:
> typedef SizeValueType ModifiedTimeType; and then use it throughout the 
> toolkit. This way you avoid trickiness across platforms, right?
> That would mean replacing all these unsigned longs in itkTimeStamp.cxx 
> and also in many other classes with ModifiedTimeType. That is also 
> some work of
> course:
> grep "unsigned long GetMTime" -r * | wc -l gives me 21 which is not so 
> bad.
>
> What do you think of this approach?
>
> Regards, Marius
>
> From: Bradley Lowekamp [mailto:blowekamp at mail.nih.gov]
> Sent: maandag 24 september 2012 15:13
> To: Staring, M. (LKEB)
> Cc: insight-developers at itk.org
> Subject: Re: [Insight-developers] type of modification time
>
> Hello Marius,
>
> The modify is a moderately expensive operation, as it evolves a mutex 
> lock or an atomic operation. I think the simplest thing to do is not 
> it to use the VectorContainer for this situation why was this chosen 
> over the stl vector container? Looking at the implementation of the 
> itk::VectorContainer, it looks rather unfortunate that the Modified is 
> called for so many operations, as it really makes it not suitable for many situations.
>
> As for changing the type to a large integer type, that seems like a 
> good thing to me. The good thing now is that all compilers which ITK 
> supports have the unsigned long long type. The bad news is that if you 
> look at the itkTimeStamp.cxx file there are separate paths for win32, 
> apple and gcc. So there are quite a few paths of code which will need 
> to be implements. It's going to be very tricky to get all the types to 
> match in both 32 and 64 bit builds across the platforms, there there 
> are all the usage of the timestamp which may have a cast.
>
> What do you think will be best for you to do?
>
> Brad
>
>
> On Sep 24, 2012, at 7:45 AM, M.Staring at lumc.nl wrote:
>
>
> Hi all,
>
> The type of the modified time in ITK (m_MTime in Object) is 
> itk::TimeStamp which in turn has a member:
>
> unsigned long m_ModifiedTime;
>
> So this member can count in the range 0 till ulong max, which is about 
> 4 x 10^9.
>
> For my particular case this member overflows. I'm using the 
> itk::VectorContainer and every time an item is inserted Modified() is 
> called which increases the unsigned long value. In my case I am using 
> minimal spanning trees and a simple algorithm would have N^2 inserts 
> with N the number of points of the tree, which is about 5000 for my 
> application so
> 5000^2 = 25 x 10^6. Then I re-use the container in an iterative 
> algorithm, which makes the problem worse. I can do about 85 iterations 
> and then it overflows. (This causes my filter to never update anymore, 
> even if  I explicitly call modified on it, or on the output, or call 
> ResetPipeline().)
>
> A solution to this issue would be to use an unsigned long long for 
> this variable. From itkIntTypes.h there is the typedef SizeValueType 
> which may be appropriate.
>
> Before I start preparing a patch: Does this seem like a good approach 
> to you?
>
> Regards, Marius
>
> Marius Staring, PhD
> Division of Image Processing (LKEB)
> Department of Radiology
> Leiden University Medical Center
> PO Box 9600, 2300 RC Leiden, The Netherlands
> phone: +31 (0)71 526 2137, fax: +31 (0)71 524 8256 m.staring at lumc.nl
>
> _______________________________________________
> 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
>