SUGGESTED FIX
Here is the #ifdef removal which makes the previous fixes work:
--- /export/jrose/ws/tiger-6461827/webrevs/6461827c/src/share/vm/opto/graphKit.cpp- Mon Jan 29 13:15:14 2007
+++ graphKit.cpp Fri Jan 26 23:26:21 2007
@@ -1865,17 +1865,13 @@
r_ok_subtype->add_req( _gvn.transform( new (1) IfTrueNode ( iff3 ) ) );
set_control( _gvn.transform( new (1) IfFalseNode( iff3 ) ) );
// Now do a linear scan of the secondary super-klass array. Again, no real
// performance impact (too rare) but it's gotta be done.
-#ifdef IA64
Node *psc = new (3) PartialSubtypeCheckNode( subklass, superklass );
- psc->set_req(0, control());
+ psc->set_req(0, control()); // only call it when we must
Node *idx = _gvn.transform( psc );
-#else
- Node *idx = _gvn.transform( new (3) PartialSubtypeCheckNode( subklass, superklass ) );
-#endif
Node *cmp4 = _gvn.transform( new (3) CmpINode( idx, _gvn.intcon(0) ) );
Node *bol4 = _gvn.transform( new (2) BoolNode( cmp4, BoolTest::ne ) );
IfNode *iff4 = create_and_xform_if( control(), bol4, PROB_FAIR, COUNT_UNKNOWN );
r_not_subtype->add_req( _gvn.transform( new (1) IfTrueNode ( iff4 ) ) );
|
|
|
EVALUATION
It's a badly designed benchmark, so the numbers must not be taken quantitatively; they provide an order-of-magnitude indication of code quality only.
As is the case with many microbenchmarks, the warm-up phase OSRs, and the timing phase runs non-OSR code. In this case, the timing numbers are disturbed by the non-OSR compilation that occurs.
A quick fix to this (though probably not a stable one) is as follows:
--- Main.java~ 2006-08-14 16:06:57.000000000 -0700
+++ Main.java 2007-01-16 16:50:44.033302000 -0800
@@ -103,9 +103,11 @@
private void run(){
runIsAssignableFrom("isAssignable warm-up time");
+ runIsAssignableFrom("isAssignable warm-up time #2");
runIsAssignableFrom("isAssignable run time");
runSetField("setField warm-up time");
+ runSetField("setField warm-up time #2");
runSetField("setField run time");
}
When creating a benchmark, always try it out with -XX:+PrintCompilation and recode until there are no compilations occurring during the timing phase. Also run -Xbatch, of course. The #2 warm-up calls run into the -Xbatch execution barrier and wait for the non-OSR compilation of each test to finish before starting the timing run.
Also, result of the call to isAssignableFrom is not tested or otherwise used. That potentially allows more optimization (or cheating!) than the benchmark attempts to test. In this case, the open-coded call to isAssignableFrom can disappear completely. If this happens, the loop containing it also disappears, except for a single iteration to perform null checks on the operands. So any time reported has nothing to do with the speed of isAssignableFrom.
|
|
|
SUGGESTED FIX
Final fix webrevs for 1.6 and 1.5:
http://prt-web.sfbay.sun.com/net/prt-archiver.sfbay/data/archived_workspaces/1.6/update2/baseline/2007/20070104202509.jrose.mustang-6461827/workspace/webrevs/webrev-2007.01.04/index.html
http://analemma.sfbay.sun.com/net/prt-archiver.sfbay/data/archived_workspaces/1.5/tiger_update12_baseline/2006/20061218164652.jrose.tiger-6461827/workspace/webrevs/webrev-2006.12.18/index.html
|
|
|
SUGGESTED FIX
This should give a simple speed improvement on XML or Beans benchmarks.
It makes all sorts of reflection operations go much faster, by open-coding the JNI call which performs reflective type checking. The call can be completed in less than ten memory references if open-coded.
There is test version here, if you want to try it:
/net/prt-web.sfbay/prt-workspaces/20070102171145.jrose.mustang-6461827/workspace (temporary)
/net/foundation.sfbay/export/jrose/builds/mustang-6461827/prt-build (not so temporary)
|
|
|
EVALUATION
This is a point fix which can be backported from Dolphin.
I am targeting it for 5.0u12 (on the sub-CR) and 6u2.
See suggested fix for a webrev.
It is co-packaged with another point fix in the same code, for 6297094.
|
|
|
SUGGESTED FIX
Here is a suggested point-fix for 1.5:
http://javaweb.sfbay/~jrose/webrev/tiger-6461827/
Fixes in later releases are almost identical.
|
|
|
EVALUATION
I have the bug reproduced and it is not pretty, we need to consider
intrinsifying the IsAssignableFrom method, needs further evaluation to backport fix from 7.0.Commiting to mustang update, as this fix is not appropriate for 6 RC.
|
|
|
|