[infinispan-dev] Collections, immutables and copies

[Manik Surtani]

Yes, I understand your reasons for not wanting to push this patch in now.  I agree that this isn't the time for such large-scale, if simple, refactoring.

I was suggesting more for 5.2.0.


On 19 Jan 2012, at 17:06, Sanne Grinovero wrote:

> On 19 January 2012 10:59, Manik Surtani <manik at jboss.org> wrote:
>> Agreed.  But lets do this scientifically.  Sanne, have you got a list of the Collections used and where, in order of their impact on overall performance?
> 
> A list of changes is in the patch [1]: every changed line matches an
> affected collection usage or API. Furthermore it's not an exhaustive
> list, since the patch is not complete.
> 
> Why is the patch incomplete:
> 
> I can try and look again if there is a specific point which could be
> tuned, but from what we see in the Profiler there is no specific
> hotspot, just a very low overhead spread around in all usecases, so it
> would be lot of work for (I guess) no measurable benefit.
> 
> The main optimization potential I see is that in many cases we need to
> make a defensive copy because a change might be needed, but this is
> always a corner case - like for example when a new view is installed
> in race condition with other events.
> They are corner cases, but they add up.
> 
> Another source of copying, is that some interfaces require
> List<Address>, some other code points want to make sure the collection
> contains unique elements and changes to HashSet, and this
> transformation might happen several times for the lifecycle of a
> single command.
> 
> I'm not seeing any specific code path for which our time (and risk
> changing) is worth currently, unless the whole lifecycle is
> addressed.. as mentioned I've aborted the patch as it was getting too
> invasive;
> 
> To give an idea of the impact, this is containing I guess half of the
> total work needed [1]
> It's not the work which scares me off, but introducing such a change
> now especially all the public APIs involved.. if you think it's worth
> a try, I'll get this patch to compile and pass tests, but APIs will be
> broken.
> 
> [1] - https://github.com/Sanne/infinispan/commit/c94cfe3af2e58b1562d080c9e6a47f523053ccb6
> 
> Sanne
> 
>> On 19 Jan 2012, at 12:47, Bela Ban wrote:
>> 
>>> This makes a lot of sense IMO, as I've done similar things in JGroups
>>> (e.g. my own optimized hashmap "Headers" in Message).
>>> 
>>> On 1/18/12 7:19 PM, Sanne Grinovero wrote:
>>>> Hi All,
>>>> after some profiling I've seen that even to manage a single command,
>>>> there is a huge amount of List<Address>, Map<Address,[something]>,
>>>> HashSet constructions involved; often they are defensively copied as
>>>> many methods might potentially remove a couple of elements, but this
>>>> event is very unlikely.
>>>> 
>>>> So I think the standard collections are unfit for this purpose, as
>>>> this is pretty much a constant, and also HashSets are not fit for the
>>>> purpose as most commonly they will contain a very small amount of
>>>> addresses.
>>>> 
>>>> I started as a draft to change this to a custom collection which would
>>>> be immutable by default and provide efficient implementations for the
>>>> specific use cases Infinispan has, and while proceeding in making more
>>>> and more changes to make it compile I convinced myself that it was the
>>>> way to go as the amount of copies and operations performed on these is
>>>> impressive, and this was going to avoid most of the work.
>>>> 
>>>> BUT I'm changing way more code than expected, including lots of
>>>> interfaces, and keeping deprecated copies makes it too complex.
>>>> 
>>>> So even if this was meant to be a draft and I didn't even finish it,
>>>> I'm aborting the task already and proposing to do something similar
>>>> for 6.0.
>>>> I can say now that it's a big work, and while I don't have performance
>>>> tests backing this I expect it to be worth it.
>>> 
>>> 
>>> --
>>> Bela Ban
>>> Lead JGroups (http://www.jgroups.org)
>>> JBoss / Red Hat
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> infinispan-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> 
>> --
>> Manik Surtani
>> manik at jboss.org
>> twitter.com/maniksurtani
>> 
>> Lead, Infinispan
>> http://www.infinispan.org
>> 
>> 
>> 
>> 
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> 
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
manik at jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org