harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Catherine Hope <catherine.v.h...@googlemail.com>
Subject Re: [classlib][luni] Collections classes - code reviews and optimisations
Date Wed, 04 Aug 2010 12:07:50 GMT
I did some analysis on what API methods access others between the RI and
Harmony (by subclassing ArrayList and adding some trace) to answer a couple
of the review points.  The differences I found were:
 - RI contains(Object) calls indexOf(Object), so we could also do this to
reduce the code duplication
 - RI add(Object), add(int, Object), addAll(Collection), addAll(int,
Collection) use ensureCapacity(int)
 - RI remove(Object) doesn't reference indexOf(Object) and remove(int) -
though I don't think this would save anything if we changed it

Cath

On Tue, Aug 3, 2010 at 11:45 PM, Mark Hindess
<mark.hindess@googlemail.com>wrote:

>
> In message <4C56E4F8.2040105@googlemail.com>, Oliver Deakin writes:
> >
> >   Hi all,
> >
> > A little while ago Mark suggested (not sure if it was in person or
> > on the list) that it would be a good idea to take a look at our
> > Collections classes and do a kind of code review, thinking about
> > improvements and possible optimisations.
> >
> > To that end Mark Hindess, Cath Hope and myself sat down and took a
> > close look at ArrayList earlier today. We got up to the growAtEnd()
> > method, annotating the code with questions and comments as we went.
> > I've opened HARMONY-6607 [1] and attached the diff with our comments
> > in so everyone can see what we've come up with so far.
> >
> > If you have any further comments or ideas for the code (including
> > patches for improvements) please add to the discussion here - it would
> > be great to have some extra input!
>
> We had another chat today and got a bit further.  I've updated the
> review patch on JIRA.
>
> I think there was general consensus that we should move all the
> exceptions to the beginning of methods.  (And in the case of removeRange
> fix the exception to be meaningful.)  I'll probably look at doing this
> shortly unless someone beats me too it as it was really annoying when
> reading the code.
>
> We should also consider tracking firstIndex and size rather than firstIndex
> and lastIndex because:
>
>  * Nearly every method uses size - most calculate it but many of them
>   "forget" to use it in all cases.
>
>  * Fewer methods use lastIndex.
>
>  * size() is the most commonly called method making it simpler would be
>   a useful side-effect
>
>  * the resulting serialization code might be simpler
>
> I'm still puzzled as to whether the fact that the call to growForInsert
> in addAll always grows by the size of the entire collection (rather than
> size - free space) is intentional or a bug.  The increment calculation
> should mean we always have sufficient free space even if we only ask for
> the smaller (strictly necessary) amount.  So I suspect it is a bug?
>
> Regards,
>  Mark.
>
> > [1] https://issues.apache.org/jira/browse/HARMONY-6607
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message