[ImageJ-devel] proposed changes in the ImgLib2 abstract class hierarchy

[Curtis Rueden]

Hi all,


> Surprising!  That means that the runtime check is somehow erased by the
> compiler---cool.  Probably a special optimization rule for the oh so
> common 'constructors with a boolean'.  Ok---if it is like that then I
> have no concerns going for your suggestion.
>

OK, I updated the code:
    https://github.com/imagej/imglib/commit/5239120f

Feel free to revert it if these changes cause any problems.

Regards,
Curtis


On Thu, Mar 22, 2012 at 4:52 PM, Stephan Saalfeld <saalfeld at mpi-cbg.de>wrote:

> Surprising!  That means that the runtime check is somehow erased by the
> compiler---cool.  Probably a special optimization rule for the oh so
> common 'constructors with a boolean'.  Ok---if it is like that then I
> have no concerns going for your suggestion.
>
> Best,
> Stephan
>
>
> On Wed, 2012-03-21 at 13:25 -0500, Curtis Rueden wrote:
> > Hi Stephan,
> >
> >
> >         I agree that your version is easier to understand and cleaner
> >         to read
> >         but it introduces a runtime check that although being tiny,
> >         will slow
> >         down the wrapping of Points and RealPoints slightly.
> >
> > Thanks for the explanation. But I don't I buy it. Here is a little
> > test:
> >
> >     https://gist.github.com/2150639
> >
> > On my system:
> >
> > Five trials of RealPointSS yields: 418, 361, 357, 359, 370
> > Five trials of RealPointCR yields: 365, 357, 357, 372, 360
> >
> > With each trial consisting of 10 million object creations.
> >
> > The difference looks negligible to me.
> >
> > Regards,
> > Curtis
> >
> >
> > On Wed, Mar 21, 2012 at 3:48 AM, Stephan Saalfeld
> > <saalfeld at mpi-cbg.de> wrote:
> >         Hi Curtis,
> >
> >         since I am responsible for that construct, I take the freedom
> >         to reply.
> >
> >         I agree that your version is easier to understand and cleaner
> >         to read
> >         but it introduces a runtime check that although being tiny,
> >         will slow
> >         down the wrapping of Points and RealPoints slightly.  In
> >         situations
> >         where this is happening millions of times, I do not want the
> >         additional
> >         cost.  So, I prefer your solution in business code but ImgLib2
> >         aims to
> >         be performance code :).  For clarity, the constructor is
> >         protected and
> >         therefore not exposed to the normal API user, a clear Javadoc
> >         should
> >         tell extenders about its expected use.  Normally, you will use
> >         the
> >         RealPoint.wrap(double[]) factory method instead.  We could
> >         make the
> >         constructor private but then we would lose the option to use
> >         it from the
> >         now extending classes.
> >
> >         If this is still too ugly for your gusto, we could make the
> >         current
> >         RealPoint and Point AbstractRealPoint and AbstractPoint and
> >         introduce
> >         both this constructor and the static wrap method in the
> >         extending
> >         RealPoint and Point classes.  I'd actually like that.
> >
> >         Best,
> >         Stephan
> >
> >
> >
> >
> >         On Tue, 2012-03-20 at 23:17 -0500, Curtis Rueden wrote:
> >         > Hi Tobias,
> >         >
> >         > First of all, thanks very much for all the API improvements!
> >         >
> >         > I just have one comment...
> >         >
> >         >                 Why is there the Object x in there?
> >         >
> >         >         If it wouldn't be there, then the compiler would be
> >         unable to
> >         >         distinguish whether you call
> >         >         protected Realpoint( final double[] position )
> >         >         or
> >         >         public RealPoint( final double... position )
> >         >
> >         > Sure, the dummy Object parameter differentiates the
> >         constructors.
> >         > However, I do not believe such drastic measures are
> >         necessary. Why not
> >         > do something like this instead:
> >         >
> >         >     https://gist.github.com/2144264
> >         >
> >         > I think it is easier to understand and accomplishes the same
> >         ends.
> >         >
> >         > Regards,
> >         > Curtis
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://imagej.net/pipermail/imagej-devel/attachments/20120419/926ebd21/attachment.html>