Re: [jbosscache-dev] New collections

Manik Surtani wrote: 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