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

[Luis Ibanez]

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
>>
>
>