[9] RFR [XS] 8054492: Casting can result in redundant null checks in generated code
Paul Sandoz
paul.sandoz at oracle.com
Tue Oct 14 15:39:31 UTC 2014
Hi Vladimir,
Thanks for looking into this.
The patch works fine for the simple test case (NullCheckDroppingsTest on the issue) but if i write a test using a MethodHandle to set a field then i still observe generated code for the uncommon trap of obj == null case.
See below for test case. When i run it:
java -cp target/classes -XX:-TieredCompilation -XX:+UnlockDiagnosticVMOptions -Xbatch -XX:LoopUnrollLimit=0 -XX:+PrintAssembly NullCheckDroppingsMHTest
;; B4: # B6 <- B3 Freq: 0.999997
0x00000001116a4e27: mov %rbx,%r10
0x00000001116a4e2a: add $0x14,%r10 ;*invokevirtual putObject
; - java.lang.invoke.LambdaForm$MH/617901222::putObjectFieldCast at 28
; - java.lang.invoke.LambdaForm$MH/1072591677::invokeExact_MT at 11
; - NullCheckDroppingsMHTest::testLoopOne at 5 (line 47)
; - NullCheckDroppingsMHTest::testLoop at 13 (line 29)
0x00000001116a4e2e: mov 0xc(%rbx),%r8d
0x00000001116a4e32: mov $0x193f5c000,%r11
0x00000001116a4e3c: jmp 0x00000001116a4e44
0x00000001116a4e3e: nop
0x00000001116a4e3f: nop
;; B5: # B6 <- B7 top-of-loop Freq: 671086
0x00000001116a4e40: mov 0xc(%rbx),%r8d ;*getfield svalue
; - NullCheckDroppingsMHTest::testLoop at 10 (line 29)
;; B6: # B9 B7 <- B4 B5 Loop: B6-B5 inner Freq: 671087
0x00000001116a4e44: test %r8d,%r8d
0x00000001116a4e47: je 0x00000001116a4e75 ;*ifnull
; - java.lang.Class::cast at 1 (line 3368)
; - java.lang.invoke.DirectMethodHandle$Accessor::checkCast at 5 (line 437)
; - java.lang.invoke.DirectMethodHandle::checkCast at 5 (line 506)
; - java.lang.invoke.LambdaForm$MH/617901222::putObjectFieldCast at 13
; - java.lang.invoke.LambdaForm$MH/1072591677::invokeExact_MT at 11
; - NullCheckDroppingsMHTest::testLoopOne at 5 (line 47)
; - NullCheckDroppingsMHTest::testLoop at 13 (line 29)
;; B7: # B5 B8 <- B6 Freq: 671086
0x00000001116a4e49: mov %r8d,(%r10) ;*goto
; - NullCheckDroppingsMHTest::testLoop at 19 (line 28)
0x00000001116a4e4c: mov %r10,%r8
0x00000001116a4e4f: shr $0x9,%r8
0x00000001116a4e53: mov %r12b,(%r11,%r8,1) ;*invokevirtual putObject
; - java.lang.invoke.LambdaForm$MH/617901222::putObjectFieldCast at 28
; - java.lang.invoke.LambdaForm$MH/1072591677::invokeExact_MT at 11
; - NullCheckDroppingsMHTest::testLoopOne at 5 (line 47)
; - NullCheckDroppingsMHTest::testLoop at 13 (line 29)
0x00000001116a4e57: inc %ebp ;*iinc
; - NullCheckDroppingsMHTest::testLoop at 16 (line 28)
0x00000001116a4e59: cmp $0xf4240,%ebp
0x00000001116a4e5f: jl 0x00000001116a4e40 ;*if_icmpge
; - NullCheckDroppingsMHTest::testLoop at 5 (line 28)
;; B8: # N132 <- B3 B7 Freq: 0.319999
0x00000001116a4e61: mov $0xffffff5d,%esi
0x00000001116a4e66: mov %rbx,(%rsp)
0x00000001116a4e6a: nop
0x00000001116a4e6b: callq 0x00000001116088e0 ; OopMap{[0]=Oop off=144}
;*if_icmpge
; - NullCheckDroppingsMHTest::testLoop at 5 (line 28)
; {runtime_call}
Paul.
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
public class NullCheckDroppingsMHTest {
static final MethodHandle SET_SSINK;
static {
try {
SET_SSINK = MethodHandles.lookup().findSetter(NullCheckDroppingsMHTest.class, "ssink", String.class);
} catch (Exception e) {
throw new Error(e);
}
}
volatile String svalue = "A";
volatile String snull = null;
String ssink;
public static void main(String[] args) {
NullCheckDroppingsMHTest t = new NullCheckDroppingsMHTest();
t.testLoop();
}
void testLoop() {
// Ensure testLoopOne is compiled and does not
// see a null value
for (int i = 0; i < 1000000; i++) {
testLoopOne(svalue);
// Uncomment the following and the call outside
// the loop should not result in any deoptimization
// testLoopOne(snull);
}
// Sleep just to create delay in the compilation log
try {
Thread.currentThread().sleep(1000);
} catch (Exception e) {}
// This should cause a de-optimisation
// if testLoopOne is compiled with a null-check
testLoopOne(snull);
}
void testLoopOne(String s) {
try {
SET_SSINK.invokeExact(this, s);
} catch (Throwable t) {
throw new Error(t);
}
}
}
On Oct 11, 2014, at 1:41 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> https://bugs.openjdk.java.net/browse/JDK-8054492
> http://cr.openjdk.java.net/~kvn/8054492/webrev/
>
> Thanks to Roland for the evaluation of this problem.
>
> Class.cast has an explicit null check:
>
> public T cast(Object obj) {
> if (obj != null && !isInstance(obj))
> throw new ClassCastException(cannotCastMsg(obj));
> return (T) obj;
> }
>
> Compiler generates null check and uncommon trap when it did not see null values in profiled data. In most cases when Class.cast is inlined isInstance() is folded and as result compiled code is looks like:
>
> if (obj != null) {
> return obj;
> } else {
> uncommon_trap(); // which triggers deoptimization and Interpreter
> // will execute the same 'return obj;' code.
> }
>
> The same result you will get with next compiled code. So the null check is noop and will be removed from compiled code (it is the goal of this change):
>
> if (obj != null) {
> }
> return obj;
>
> Even if isInstance() is not folded we don't need to generate uncommon trap for obj == null case:
>
> if (obj != null) {
> if (!isInstance(obj)) {
> uncommon_trap(); // or throw code
> }
> }
> return obj;
>
> One benefit from having uncommon trap is it could be converted to implicit null check by folding it into a following memory instruction (load or store). In Class.cast() we don't have such memory instruction so we don't benefit from uncommon trap.
>
> The fix is rise probability of not-taken path to avoid generation of uncommon trap when processing ifnull bytecode in Class.cast method.
>
> Tested with JPRT, jtreg
>
> Thanks,
> Vladimir
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20141014/1ce23471/signature.asc>
More information about the hotspot-compiler-dev
mailing list