harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Deakin <oliver.dea...@googlemail.com>
Subject Re: [classlib][luni] Collections classes - code reviews and optimisations
Date Wed, 04 Aug 2010 15:55:57 GMT
  On 04/08/2010 15:56, Mark Hindess wrote:
> I've now refactored the exception code to the start of the methods and
> shuffled some of the if else cases around a little.
>
> Next I plan to look at:
>
> 1) remove the following from addAll(...):
>
>           if (this == collection) {
>               collection = (ArrayList)clone();
>           }
>
>     since it is immediately followed by a call to collection.toArray() so
>     it should be unnecessary.
>
> 2) Change the fields to firstIndex and size.
>
> In message<AANLkTikT1+REkJr8H5nUK6E-UD-nHxh5WbwH2Gj3+MX5@mail.gmail.com>,
> Catherine Hope writes:
>> 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.
> Thanks Cath.  I wonder if we can use serialization/deserialization
> to figure out how the RI grows the capacity of the ArrayList.  Our
> implementation seems to grow it quite quickly.
>
>> The differences I found were:
>>   - RI contains(Object) calls indexOf(Object), so we could also do this
>> to reduce the code duplication
> Sounds good.

+1

>>   - RI add(Object), add(int, Object), addAll(Collection), addAll(int,
>> Collection) use ensureCapacity(int)
> Interesting.  Makes our three grow* methods seems a little excessive.

I was just thinking the same thing :)

It occurs to me that the grow methods for the start and end of the list 
are just special cases of growForInsert(). I think we could probably 
roll them all into one neater method covering all three cases. This 
should remove the necessity to check if we are at the start/end of the 
array in the add methods too - when we return from growForInsert() we 
will know that there will be the required number of empty slots at 
location and that first/lastIndex have been updated, so we can then just 
write our data straight into that position.


>>   - RI remove(Object) doesn't reference indexOf(Object) and remove(int) -
>> though I don't think this would save anything if we changed it
> Agreed.
+1

Regards,
Oliver

> -Mark.
>
>
>

-- 
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


Mime
View raw message