[Insight-developers] Trying to avoid an extra copy reading files using itkImageSeriesReader

Bradley Lowekamp blowekamp at mail.nih.gov
Mon Mar 28 10:32:31 EDT 2011


Hello Roger,

Your benchmark program had a few more dependencies, the just ITK so I wrote my own and attached it. I used a series of tiff I have, so I hope it would be comparable. I have also arrived at a similar conclusion that the copy loop is expensive and should be avoided. However, my benchmark does indicate that the progress reporting is taking 50% of the additional execution time, which is rather different then your experiment.


Testing series reader with 349 files.
Image Size: [2048, 1536, 349]

# current ITK
Executed 10 times with mean 24.4403s

# progress commented out
Executed  10 times with mean 20.7206s

# copy loop commented out
Executed with 10 times with mean 16.5306s

# gerrit patch version
Executed 10 times with mean 16.9262s



On Mar 28, 2011, at 8:54 AM, Lowekamp, Bradley (NIH/NLM/LHC) [C] wrote:

> Roger,
> 
> I am going to be looking at this patch to day to see if I can make the enhancement that will work for all cases.
> 
> However, I have a few questions about this test case. How big is your image?
> 
> The difference between the two times is  8185ms or 8.2 seconds. Even for my large microscopy images copying the data takes much less time that this. So it makes me wonder if something else is going on. I read series of tifs a lot, so I'll do some experimentation to determine bottle necks as well.
> 
> Brad
> 
> 
> On Mar 28, 2011, at 8:31 AM, Roger Bramon Feixas wrote:
> 
>> You're welcome, Luis.
>> 
>> We're really interested on your patch to apply it to ITK 3.20. Do you think it's a good idea? We don't use streaming right now.
>> 
>> Thanks!
>> 
>> Roger
>> 
>> On Sat, Mar 26, 2011 at 5:09 PM, Luis Ibanez <luis.ibanez at kitware.com> wrote:
>> Ahhh,
>> 
>> The beauty of making decisions
>> based on actual experimental data  !
>> 
>> How refreshing      :-)
>> 
>> 
>> Thanks for running those tests Roger.
>> 
>> 
>>       Luis
>> 
>> 
>> 
>> -------------------------------------------------------
>> On Thu, Mar 24, 2011 at 12:27 PM, Roger Bramon Feixas
>> <rogerbramon at gmail.com> wrote:
>> > Hi,
>> > Here you have the timing numbers. I used a CT model with 882 files.
>> > ITK 3.20: 16854 ms
>> > ITK 3.20 without progress update: 16115 ms
>> > ITK 3.20 avoiding the copy: 8669 ms
>> > The penalty is not significant compared to the time needed to make the
>> > copy.
>> > Thanks,
>> > Roger
>> >
>> >
>> > On Thu, Mar 24, 2011 at 2:55 PM, Bradley Lowekamp <blowekamp at mail.nih.gov>
>> > wrote:
>> >>
>> >> Hello Roger,
>> >> I am curious as to how much of the performance penalty is from the copy
>> >> verses the progress update. Do you think you could run that experiment and
>> >> post the timing numbers?
>> >> Thanks,
>> >> Brad
>> >>
>> >> On Mar 24, 2011, at 9:08 AM, Roger Bramon Feixas wrote:
>> >>
>> >> Hi Luis,
>> >> Thank you! I've applied the patch to ITK 3.20 (few changes are needed) and
>> >> it works really well in the common scenario however it seems some of your
>> >> streaming tests don't pass. As you expected, itkImageSeriesReader is 2x
>> >> faster now.
>> >> However, ProgressReporter doesn't work now because:
>> >> progress.CompletedPixel();
>> >> has been deleted since it was in the copy loop.
>> >> A solution could be change:
>> >>   ProgressReporter progress( this, 0,
>> >>                             requestedRegion.GetNumberOfPixels(),
>> >>                             100 );
>> >> to:
>> >>   ProgressReporter progress( this, 0,
>> >>                             numberOfFiles,
>> >>                             100 );
>> >> and call progress.CompletedPixel(); for each new file read. Is it right or
>> >> is there a better solution?
>> >> Thanks,
>> >> Roger
>> >>
>> >>
>> >> On Thu, Mar 24, 2011 at 1:32 AM, Luis Ibanez <luis.ibanez at kitware.com>
>> >> wrote:
>> >>>
>> >>> Hi Roger,
>> >>>
>> >>> Thanks for pointing this out.
>> >>>
>> >>> The ImageFileReader could indeed be told to load the image
>> >>> buffer directly into a subset of the larger output image that
>> >>> is going to be produced by the ImageSeriesReader.
>> >>>
>> >>> This is done with code along the lines of:
>> >>>
>> >>> reader->GetOutput()->GetPixelContainer()->SetImportPointer(
>> >>>                    pixelData,
>> >>>                    totalNumberOfPixels,
>> >>>                    filterWillDeleteTheInputBuffer );
>> >>>
>> >>>
>> >>> However, this will only work if the block of data that is
>> >>> being loaded with the ImageFileReader corresponds to a
>> >>> contiguous block of memory in the output image of the
>> >>> ImageSeriesReader.
>> >>>
>> >>> This will be the case int the common scenario of a 3D
>> >>> image being loaded by reading a collection of 2D slices.
>> >>>
>> >>> But, will not work if the mapping from files in the series
>> >>> does not satisfy this requirement in the memory of the
>> >>> output image. I'm suspecting that some scenarios of
>> >>> streaming may break this assumption.
>> >>>
>> >>> One option that comes to mind is that we could
>> >>> analyze the relationship between:
>> >>>
>> >>>  ImageRegionType requestedRegion = output->GetRequestedRegion();
>> >>>
>> >>> in line 269 of itkImageSeriesReader.txx
>> >>>
>> >>> and the region:
>> >>>
>> >>>    sliceRegionToRequest
>> >>>
>> >>> as it is computed in line 285 of the same file.
>> >>>
>> >>> looking at the computation of lines 284 and 285
>> >>> it would seem that the output region will always
>> >>> be contiguous.
>> >>>
>> >>> An initial patch for removing the copying
>> >>> is now available in Gerrit:
>> >>>
>> >>> http://review.source.kitware.com/#change,1248
>> >>>
>> >>> It probably needs more testing before we merge it...
>> >>>
>> >>> If you have some time available, it will be great
>> >>> if you try that modified file in your test case, and
>> >>> let us know if it addressed the concern for speed.
>> >>>
>> >>>
>> >>>    Thanks
>> >>>
>> >>>
>> >>>         Luis
>> >>>
>> >>>
>> >>> -------------------------------------------------
>> >>> On Wed, Mar 23, 2011 at 8:27 AM, Roger Bramon Feixas
>> >>> <rogerbramon at gmail.com> wrote:
>> >>> > Hi,
>> >>> > I'm developing with ITK 3.20 + GDCM 2.0.17 + VTK 5.6 and I've noticed
>> >>> > itkImageSeriesReader is ~2x slower than vtkGDCMImageReader (from
>> >>> > GDCM2). I
>> >>> > compared both codes and I think the difference is the extra copy which
>> >>> > itkImageSeriesReader makes from ImageFileReader's output to its own
>> >>> > output
>> >>> > (ImageSeriesReader::GenerateData() line 393). I've commented it just to
>> >>> > test
>> >>> > the time needed without making the copy and now both readers take more
>> >>> > or
>> >>> > less the same time.
>> >>> > Is there a way to avoid the copy? I mean, could ImageFileReader write
>> >>> > into
>> >>> > the itkImageSeriesReader's output allocated memory directly? It's quite
>> >>> > dramatic for huge series.
>> >>> > I've also attached a test program if someone want to test it.
>> >>> > Thanks for your attention,
>> >>> > Roger
>> >>> >
>> >>> > _______________________________________________
>> >>> > 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.html
>> >>> >
>> >>> > 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
>> >>> >
>> >>> >
>> >>
>> >> <ATT00001..txt>
>> >>
>> >> ========================================================
>> >>
>> >> Bradley Lowekamp
>> >>
>> >> Lockheed Martin Contractor for
>> >>
>> >> Office of High Performance Computing and Communications
>> >>
>> >> National Library of Medicine
>> >>
>> >> blowekamp at mail.nih.gov
>> >>
>> >
>> >
>> 
> 
> ========================================================
> Bradley Lowekamp  
> Lockheed Martin Contractor for
> Office of High Performance Computing and Communications
> National Library of Medicine 
> blowekamp at mail.nih.gov
> 
> 
> <ATT00001..txt>

========================================================
Bradley Lowekamp  
Lockheed Martin Contractor for
Office of High Performance Computing and Communications
National Library of Medicine 
blowekamp at mail.nih.gov


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20110328/69962070/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: itkImageSeriesReaderPerformance.cxx
Type: application/octet-stream
Size: 1215 bytes
Desc: not available
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20110328/69962070/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20110328/69962070/attachment-0001.htm>


More information about the Insight-developers mailing list