Request for review: 6312706: Map entrySet iterators should return different entries on each call to next()
mike.duigou at oracle.com
Mon Mar 28 21:01:34 UTC 2011
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.
More information about the core-libs-dev