RFR(s): 8078463: optimize java/util/Map/Collisions.java

[Martin Buchholz]

Your changes look good, but:

 204             check(map.size() == i, "insertion: map expected size
m%d != i%d", map.size(), i);

many of those detail messages look like leftovers from a long debugging
session.  Here I would consider converting to a testng test (I ended up
doing this a few times myself) and writing very simply, standardly,
efficiently and readably

assertEquals(map.size(), i);

only adding more breadcrumbs if it's non-obvious, which is generally not
the case in this test.

testMap already prints out keys_desc, so the test output is unambiguous.

On Wed, May 13, 2015 at 6:44 PM, Stuart Marks <stuart.marks at oracle.com>
wrote:

> Hi all,
>
> Please review this change to optimize a test. Basically the test did
> string formatting for every assertion, but the string was thrown away if
> the assertion passed -- the most common case. The change is to do the
> string formatting only when an assertion fails and information needs to be
> printed out.
>
> Thanks to Andrey Zakharov for discovering and investigating this.
>
> Bug report:
>
>         https://bugs.openjdk.java.net/browse/JDK-8078463
>
> Webrev:
>
>         http://cr.openjdk.java.net/~smarks/reviews/8078463/webrev.0/
>
> On my (new, fast) laptop, with JVM options -Xcomp -XX:+DeoptimizeALot
> -client, the unmodified test takes about 21.4 seconds to run. The modified
> test takes only 12.3 seconds.
>
> Note that I have added several overloads of check() with different
> arguments. I tried an alternative, which is a varargs version of check():
>
>     static void check(boolean cond, String fmt, Object... args) {
>         if (cond) {
>             pass();
>         } else {
>             fail(String.format(fmt, args));
>         }
>     }
>
> This of course is much simpler code, but it took 14.2 seconds, about 15%
> slower than the proposed version. Is the simpler code worth the slowdown? I
> could go either way.
>
> Thanks.
>
> s'marks
>
>
>