Review for CR 6728865 : Improved heuristics for Collections.disjoint() [updated]
David Holmes
David.Holmes at oracle.com
Tue Dec 21 02:24:49 UTC 2010
Functionality looks okay to me.
I think those spec/doc clarifications may need to go through CCC though.
David
Mike Duigou said the following on 12/21/10 11:48:
> I've updated the webrev with Ulf's feedback. http://cr.openjdk.java.net/~mduigou/6728865.2/webrev/
>
> The old heuristics:
>
> 1. If c1 is a Set then iterate over c2.
>
> 2. Iterate over the smaller Collection.
>
>
> I believe that the || in the original should have been a &&
>
> I've rearranged it as branches in my revision.
>
>
> The new heuristics:
>
> 1. If c1 is a Set then iterate over c2.
>
> 2. If c2 is a Set then iterate over c1.
>
> 3. If either collection is empty then result is always true.
>
> 4. Iterate over the smaller Collection.
>
> Mike
>
>
>
> On Dec 19 2010, at 16:42 , David Holmes wrote:
>
>> Hi Mike,
>>
>> Mike Duigou said the following on 12/20/10 10:29:
>>> I have updated the webrev for CR 6728865 with Rémi's feedback. The new webrev is at:
>>> http://cr.openjdk.java.net/~mduigou/6728865.1/webrev/
>>> The size() comparisons are now done only when both c1 and c2 are not sets and I have removed the isEmpty() micro-optimization.
>> So to summarise this change:
>>
>> 1. The original code checked for c1 being a set and not c2, but not vice-versa - this fixes that
>>
>> 2. This code adds an optimization when they are both not sets but at least one is empty
>>
>> Did I miss anything?
>>
>> Seems functionally sound to me, but I can't attest to any performance benefits.
>>
>> Cheers,
>> David Holmes
>
More information about the core-libs-dev
mailing list