[jbosscache-dev] New collections

[Manik Surtani]

Great - thanks dude!

On 7 Aug 2008, at 05:35, Jason T. Greene wrote:

> Manik Surtani wrote:
>> On 5 Aug 2008, at 14:42, Jason T. Greene wrote:
>>>>>
>>>>> 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.
>>>> Would these be more generic, to suit any collection  
>>>> implementation?  Remember that not all Maps, for example,  
>>>> implement Cloneable or have a copy constructor.  And wrapping any  
>>>> Map passed in as a FastCopyHashMap may break correctness, for  
>>>> example, if a SortedMap were passed in.
>>>
>>> Right, they would use instanceof to check all the known map types  
>>> as a fast-path optimization. After that it would check for  
>>> cloneable, and use reflection to invoke clone. It would then look  
>>> for a copy constructor, and failing that it would then create a  
>>> different collection type. We could for example, in the case of a  
>>> non-copyable SortedMap, use a TreeMap, and order is maintained.
>> Right yes, you would need the logic in place to do this.
>> -- 
>
> Ok, today I went ahead and made these changed (commit 6536). So in  
> case anything breaks, you know why :) I did run through the unit  
> tests locally though, with no failures.
>
> There is now a Immutables factory class that does copying,  
> conversion, and wrapping of list, set, collection, and map. So as  
> long as everything uses this, it should be easy to swap out  
> implementations.
>
> As suggested, I went ahead and left ImmutableListCopy, however the  
> other types use a wrapped clone/copy. Since the marshalling code  
> needs to detect these types, I ended up having to reimplement the  
> Collections.unmodifiableXXX types (not a lot of code though).
>
> These changes saved another copy in the unmarshalling code, since it  
> needs to construct a new HashMap anyway. In this scenario, the  
> Immutables.immutableMapWrap() is used, which does not copy.
>
> Also the duplicate copy in CacheInvocationDelegate.getChildrenNames  
> was also removed using the set variant of the wrap method.
>
> -- 
> Jason T. Greene
> JBoss, a division of Red Hat

--
Manik Surtani
Lead, JBoss Cache
manik at jboss.org