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.

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(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 19 January 2012 10:59, Manik Surtani <manik(a)jboss.org&gt; 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/c94cfe3af2e58b1562d080c9e6a47f... 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(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > -- > Manik Surtani > manik(a)jboss.org > twitter.com/maniksurtani > > Lead, Infinispan > http://www.infinispan.org > > > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev