On Mar 26 2011, at 01:10 , Neil Richards wrote: > Hi Mike - thanks for posting up the webrev, and for your review. > > On Fri, 2011-03-25 at 19:33 -0700, Mike Duigou wrote: >> I have run the jtreg regression suite against your webrev. I had to >> make one change to IdentityHashMap to satisfy the Collection/MOAT >> test. The failure condition involved calling EntrySet.remove() when >> there was no current entry due to a prior remove() or because next() >> had never been called. > > With the assignment line suitably corrected, I believe there is no need > for the added check. > Please find below a corrected changeset 'hg export' with the relevant > correction made.
I will apply this patch and retest. >> If you have any metrics that you can contribute which show this change >> to have negligible impact upon performance across common benchmarks it >> would definitely help to allay any fears. > > Our versions of Java 6 (which have been "out in the field" for more than > 3 years now) are not susceptible to this particular problem, so our > performance folk (yes, we have them too!) have obviously found its > resolution to be unconcerning. > > Whilst developing the suggested change, I also reviewed EnumMap and > IdentityHashMap for opportunities to avoid using entry sets (and their > iterators) for the current object altogether. > This resulted in the expanded methods I highlighted in my previous post. > (In these cases, the performance should actually improve slightly, as > using the expansion avoids at least two object creations). > > Do you have any particular "common benchmarks" in mind? > I'm currently out of office for a few days (visiting family in Concord), > but can look to work on provide supporting data once I'm back "in situ", > as necessary. My concern was/is just a general anxiety in that I reasonably expect that the improved version will be slightly slower but I can't make a definitive assertion that the slowdown is insignificant. I did two very quick tests - One exercises the iterator but completes before there is a need to GC. There was essentially no difference in performance. - The other text starts a timer task that does a full GC every half second. Again there was no difference in performance. Mike
