Hi Martin, thanks for taking a look.
It probably would be a good idea to convert this test (and a whole bunch of
others) to Test-NG. However, that's more change than I want to bite off at this
point, so I'd prefer to stick with my change as it stands right now.
s'marks
On 5/13/15 7:24 PM, Martin Buchholz wrote:
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
<mailto: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/
<http://cr.openjdk.java.net/%7Esmarks/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
