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