[Insight-developers] Image memory model

Sun Jun 1 15:28:36 EDT 2008

Karthik,

Have you verified that running the NeighborhoodIterators through the
Functors/Accessors did not affect performance? If the extra level of
indirection is not properly optimized out for the default case, then we
could have introduced a performance overhead.  Did you running any timing
tests before/after this change?

Jim

On Sun, Jun 1, 2008 at 1:19 PM, Karthik Krishnan <
karthik.krishnan at kitware.com> wrote:

> On Sun, Jun 1, 2008 at 12:35 PM, Jim Miller <millerjv at gmail.com> wrote:
> > I think the issue with this design is that the NeighborhoodIterators do
> not
> > go through the PixelAccessors (for efficiency).  In fact, the
> > NeighborhoodIterators keep a "neighborhood" or pointers to pixels and
> know
> > how to update those pointers via the iteration.
>
> I had added these a while ago, to support the VectorImage.
> Please see itk::NeighborhoodIteratorFunctor and
> itkVectorImageNeighborhoodAccessorFunctor.
>
> All iterators in ITK go through accessors/accessor functors.
>
> The NeighborhoodAccessorFunctors work just like the PixelAccessors,
> however have a slightly different API, so as to gel with the
> neighborhoodAccessors.
>
>  inline PixelType Get( const InternalPixelType * ) const
>
>  inline void Set( InternalPixelType* &pixelPointer, const PixelType
> &p ) const
>
>  inline PixelType BoundaryCondition(
>      const OffsetType& point_index,
>      const OffsetType &boundary_offset,
>      const NeighborhoodType *data,
>      const ImageBoundaryConditionConstPointerType boundaryCondition) const
>
> Dan, you must be familiar with the Set/Get methods, having written the
> PixelAccessor for the SliceContiguousImage. The last method,
> BoundaryCondition method is called whenever the operator() method is
> accessed for the boundary conditions, so you can return the right
> pointer there. In the accessors, we delegate this back to the boundary
> condition (the slower alternative signature is used here). You will
> notice that there are two signatures for each of the boundary
> conditions, One of them takes the accessor itself as an argument.
>
> These accessors are obtained as traits of the image. All iterators are
> tested for itk::VectorImage and itk::Image.
>
> So, I think this should be possible for the SliceContiguousImage as
> well, by writing a itk::SliceContiguousNeighborhoodAccessorFunctor.
>
> Thanks
> --
> karthik
>
> > Introducing the GetBufferPointer() method is one of things I regret most
> in
> > the design of the ITK image. I never should have done that.  If all the
> > iterators worked of the concept of an "offset" from the first pixel (i.e.
> > the number of pixels since the beginning of the buffer), then they could
> > have indexed the pixels as PixelContainer[i] to get the ith pixel.  The
> > PixelContainer could then hide the memory layout from the Image class and
> > from the Iterators. This would even allow PixelContainers that internally
> > compressed the image data or store part of the image data on disk and
> part
> > in memory.
> >
> > ImageRegionIterator can support such a referencing scheme easily.
> > ImageRegionIteratorWithIndex would require a little bit of work to
> support
> > the memory model.  NeighborIterator would require a good deal of work to
> > support such a model. These are all doable.  It is just a matter of
> > resources and a matter of efficiency (eg. do the neighborhood iterators
> get
> > too slow in this new model).
> >
> > Outside of the Iterators, the IO mechanisms (including the Import/Export
> > facilities for interfacing to other systems), and few poorly written
> > filters, the use of GetBufferPointer() should be fairly limited and
> > isolated.
> >
> > In practice, when I interface into systems that have a slice contiguous
> > model, I either use ITK just on 2D slices and forgo 3D processing, or I
> make
> > a volume contiguous image that is a copy of the slice contiguous image.
> > These are not the best solutions but todays 64 bit world is currently
> much
> > larger than medical image data set sizes.
> >
> >
> >
> > On Sun, May 25, 2008 at 8:28 PM, Karthik Krishnan
> > <karthik.krishnan at kitware.com> wrote:
> >>
> >> Dan:
> >>
> >> On Sun, May 25, 2008 at 2:41 AM, Dan Mueller <dan.muel at gmail.com>
> wrote:
> >> >
> >> > Image::GetBufferPointer() is used extensively within the toolkit. Take
> >> > a look at the constructors for ImageConstIterator,
> >> > ConstNeighborhoodIterator, ImageReverseConstIterator,
> >> > ImageConstIteratorWithIndex, etc. I have added a list to
> >> >    http://www.itk.org/Wiki/Proposals:Slice_contiguous_images
> >> > however I realise now that some of these are
> >> > ImportImageContainer::GetBufferPointer() (I'll fix this up when time
> >> > permits).
> >>
> >> That should not be a cause of concern
> >>   image->GetBufferPointer() is just a pointer to the first pixel in
> >> the image. It is the dereferencing (m_BufferPointer + offset) that's
> >> of concern.
> >>
> >> >
> >> > Unfortunately, while your suggestion of using the PixelAccessor and
> >> > PixelAccessorFunctor would have been easy, I think it is too good to
> >> > be true.
> >>
> >> It is true.
> >>
> >> > The assumptions regarding contiguous memory allocation are
> >> > deeply entrenched.
> >>
> >> These classes were introduced a year or two ago, precisely because we
> >> had to support an alternate memory model image for NAMIC.
> >>
> >> >
> >> > Take for example the very first step you discussed, the act of passing
> >> > the input pixel value from the iterator.Get() function to the
> >> > PixelAccessorFunctor:
> >> >>   PixelType iterator.Get()   reads
> >> >>    { return pixelAccessorFunctor.Get(*(m_Buffer+m_Offset)); }
> >>
> >> The variable is passed by reference, however it is later dereferenced
> >> by the functor to the corrent pointer. The VectorPixelAccessor reads:
> >>
> >>  inline ExternalType Get( const InternalType & input, const unsigned
> >> long offset ) const
> >>    {
> >>    ExternalType output( (&input)+(offset*m_OffsetMultiplier) ,
> >> m_VectorLength );
> >>    return output;
> >>    }
> >>
> >> ie the offset is passed in and based on the vector length instead of
> >> returning *(m_Buffer+m_Offset) that an itk::Image would do, the
> >> itk::VectorImage returns *(m_Buffer + m_Offset * m_VectorLength ) .
> >>
> >> >
> >> > This deference *(m_Buffer+m_Offset) will access potentially invalid
> >> > memory in the slice-contiguous case (m_Buffer is the result of
> >> > m_Image->GetBufferPointer(), the start of the contiguous array, or the
> >> > start of the first slice for my case).
> >>
> >> I realize that it would have been better to pass in the offset and the
> >> buffer_start_pointer in, to avoid invalid de-references. (I think
> >> passing them by reference avoids the de-reference). So we could change
> >> the signature of the the accesssor to
> >>
> >>  inline ExternalType Get( const InternalType & begin, const unsigned
> >> long offset ) const
> >>
> >> and the iterator would read:
> >>
> >>  PixelType Get(void) const
> >>    { return m_PixelAccessorFunctor.Get(m_Buffer, m_Offset); }
> >>
> >> instead of
> >>
> >>  PixelType Get(void) const
> >>    { return m_PixelAccessorFunctor.Get(*(m_Buffer+m_Offset)); }
> >>
> >>
> >> > There are other places this assumption is also made: for example
> >> > ImageConstIteratorWithIndex::SetIndex() uses the buffer pointer to
> >> > compute the index.
> >>
> >> Again the starting pointer should not matter. All attempts to
> >> dereference the pointers, via Set or Get go through the accessors.
> >>
> >> >
> >> > Have I misinterpreted your meaning? I think either m_Buffer and/or
> >> > m_Offset would need to altered from their original meaning to work for
> >> > slice-contiguous memory...
> >>
> >> No. I don't think so. In your case,
> >>
> >> m_Buffer would be any arbitrary value say 0.
> >> m_Offset would be the Nth pixel into the image.
> >>
> >> Any attempt to access the Nth pixel would go through the accessor,
> >> which would have start pointers to all the slices in your image and
> >> would read something like:
> >>
> >>  inline ExternalType itk::SlicePixelAccessor::Get( const InternalType
> >> & begin, const unsigned long offset ) const
> >>    {
> >>    const unsigned long slice = offset/(x_size*y_size);
> >>    const unsigned long offset_into_slice = offset % (x_size*y_size);
> >>    return *(m_SlicePointer[slice] + offset_into_slice);
> >>    }
> >>
> >>
> >> --
> >> Hope this helps.
> >>
> >> Thanks
> >> --
> >> Karthik Krishnan
> >> R&D Engineer,
> >> Kitware Inc.
> >> _______________________________________________
> >> Insight-developers mailing list
> >> Insight-developers at itk.org
> >> http://www.itk.org/mailman/listinfo/insight-developers
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20080601/bcbe1118/attachment.htm>