On Feb 25 2013, at 01:51 , Alan Bateman wrote: > On 25/02/2013 04:06, Mike Duigou wrote: >> I have created an issue for fixing the introduced regression and created a >> regression test that would have caught the error (fix a bug, write a >> test...). >> >> 8008785: IdentityHashMap.values().toArray(V[]) broken by JDK-8008167 >> >> http://cr.openjdk.java.net/~mduigou/JDK-8008785/0/webrev >> >> Mike > The fix to IdentityHashMap looks good. It is surprising that it wasn't caught > by tests in the jdk repository so a reminder that we always need to check > that we have good test coverage when doing performance fixes. > > The webrev suggests you've replaced Map/Collisions.java and assume that is > not the intention. Otherwise the new test looks okay, an alternative would > have been to expand MOAT.
Ouch! The patch generated by webrev gets it wrong too: --- old/test/java/util/Map/Collisions.java 2013-02-24 19:47:40.491333191 -0800 +++ /dev/null 2013-02-24 18:12:51.832474922 -0800 --- /dev/null 2013-02-24 18:12:51.832474922 -0800 +++ new/test/java/util/Map/ToArray.java 2013-02-24 19:47:40.319333199 -0800 vs diff on source repo: diff --git a/test/java/util/Map/Collisions.java b/test/java/util/Map/ToArray.java copy from test/java/util/Map/Collisions.java copy to test/java/util/Map/ToArray.java --- a/test/java/util/Map/Collisions.java +++ b/test/java/util/Map/ToArray.java
