[Insight-developers] Trying to avoid an extra copy reading files using itkImageSeriesReader
Roger Bramon Feixas
rogerbramon at gmail.com
Mon Mar 28 08:31:15 EDT 2011
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
> >>
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20110328/1e6b2a19/attachment.htm>
More information about the Insight-developers
mailing list