On 20 Aug 2009, at 16:12, Brian Stansberry wrote: > This didn't seem to go through; apologies for spam if it did. > > -------- Original Message -------- > Message-ID: <4A8C79BF.3000103(a)redhat.com&gt; > Date: Wed, 19 Aug 2009 17:16:31 -0500 > From: Brian Stansberry <brian.stansberry(a)redhat.com&gt; > User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.1) > Gecko/20090814 Fedora/3.0-2.6.b3.fc11 Thunderbird/3.0b3 > MIME-Version: 1.0 > To: jbosscache-dev <jbosscache-dev(a)lists.jboss.org&gt; > Subject: Immutables.immutableSetCopy > Content-Type: text/plain; charset=ISO-8859-1; format=flowed > Content-Transfer-Encoding: 7bit > > This method looks quite inefficient for its actual usage. > > A look in the IDE shows its used by UnversionedNode.getChildrenNames() > and getKeysDirect(). Those pass either a ConcurrentHashMap$KeySet, a > FastCopyHashMap$KeySet, Collections$EmptySet or > Collections$SingletonSet. The attempted optimizations (some of which > involve reflection) in immutableSetCopy can handle non of those and > eventually the HashSet copy constructor gets called. > > A profiling run showed 22 invocations of UnversionedNode.getKeysDirect() > took 3,804 microseconds, of which 3,563 was in immutableSetCopy. 789 of > that was in the new HashSet(toCopy) call; the rest was basically wasted. > > Any reason I shouldn't just turn this into a new HashSet(toCopy) call? Good point, thanks for catching this. This approach works for most collections types where known clone methods can be faster than a new construction. IMO the sets you mentioned should be added to the known types, and handled accordingly. https://jira.jboss.org/jira/browse/JBCACHE-1535

-- Manik Surtani manik(a)jboss.org Lead, Infinispan Lead, JBoss Cache http://www.infinispan.org http://www.jbosscache.org