[Insight-developers] Trying to avoid an extra copy reading files using itkImageSeriesReader
Roger Bramon Feixas
rogerbramon at gmail.com
Mon Mar 28 14:53:38 EDT 2011
Hi Brad,
I have done some tests using your test program.
On one hand, I generated a series of tiff images in order to compare it with
your results. The resolution isn't the same but I suppose isn't a problem. I
don't know if "current ITK" in your test means ITK 3.20. In this case, the
progress reporting is taking more time since it depends on the number of
pixels.
Testing series reader with 349 files.
Image Size: [1668, 847, 349]
Pixel Type: signed char
# ITK 3.20
Executed 10 times with mean 28.2091s
# ITK 3.20 progress commented out
Executed 10 times with mean 26.5379s
# gerrit patch version
Executed 10 times with mean 12.0135s
On the other hand, I've also test a series of DICOM files. I've uploaded it
if you want to test it: http://bit.ly/fyTAMs. I've changed the Pixel Type of
your benchmark to "signed short" which is the desirable one for this
series.
Testing series reader with 881 files.
Image Size: [512, 512, 881]
# ITK 3.20
Executed 10 times with mean 14.9983s
# ITK 3.20 progress commented out
Executed 10 times with mean 14.5739s
# gerrit patch version - Pixel Type: signed short
Executed 10 times with mean 7.2905s
Thanks for your work,
Roger
On Mon, Mar 28, 2011 at 4:32 PM, Bradley Lowekamp <blowekamp at mail.nih.gov>wrote:
> 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/6a4640e5/attachment-0001.htm>
More information about the Insight-developers
mailing list