[jbosscache-dev] New collections

[Jason T. Greene]

Manik Surtani wrote:
> Guys,
> 
> I have created 3 new collection impls in trunk - ImmutableListCopy, 
> ImmutableSetCopy and ImmutableMapCopy.  I have also ported the first 2 
> to branch 2.2.x (the reason why I didnt port the third is because the 
> MapCopy which it replaces can get serialized and I don't want to break 
> binary compat).
> 

I took a look at adjusting these to be more efficient with the 
FastCopyHashMap. After doing some analysis and sanity benchmarks, I have 
determined that we should switch to unmodifiableXXX(obj.clone() or copy 
constructor) in all cases, for the following reasons:

1. There is no performance advantage with lists.
2. While the construction time in the Map and Set variants is faster, 
since it doesn't use hashing, it sacrifices way too much (O(n) lookup 
times). I did verify that the performance difference will be the same 
with just about every map implementation, since it's mostly allocation 
that is being measured here.
3. It prevents the copy optimization in FastCopyHashMap, so removing 
usage of ImmutableMapCopy will improve copy performance of anything 
using FastCopyHashMap.
4. The construction of the immutable wrapper is barely noticeable, and 
it's done per call, so the value is likely temporary.

Also I noticed that we have some duplicate copying going on. For example 
, CacheInvocationDelegate copies a set that is constructed by the 
command, per call.

What I would like to do, provided everyone is ok with this, is switch 
this code to static factory methods like so:

Immutables.immutableCopy(Map map);
Immutables.immutableCopy(List list);
Immutables.immutableCopy(Set set);

These will then just call clone and Collections.unmodifiableXXX as 
appropriate. We can then add optimizations later if they become available.


-- 
Jason T. Greene
JBoss, a division of Red Hat