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.ma...@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 > > >
