Re: Forcing initialization of string concat INDY expressions
The worst thing here is that conceptually, for an invokedynamic, the implementation do not use the fact that MethodTypes are interned. Only a direct call to a method handle (with invokeExact/invoke) needs MethodTypes to be interned to speedup the test that verifies that the parameter types are compatible. One way to solve that is to not use the cache when creating a MethodType but only when invoking a method handle directly. A MethodHandle can have two fields, one storing the non-interned method type (which is final) and one storing the corresponding interned method type (which is @Stable) and computed it if the pointer check of an invokeExact check fail because mh.invokeExact(desc) if (mh.internedMethodType == desc.methodType) { // fastpath } if (mh.internedMethodType == null) { mh.internedMethodType = intern(mh.type); if (mh.internedMethodType == desc.methodType) { // fastpath } } ... Rémi - Mail original - > De: "David Holmes" <david.hol...@oracle.com> > À: "Aleksey Shipilev" <sh...@redhat.com>, "core-libs-dev Libs" > <core-libs-dev@openjdk.java.net> > Envoyé: Mercredi 1 Mars 2017 13:36:49 > Objet: Re: Forcing initialization of string concat INDY expressions > Hi Aleksey, > > On 1/03/2017 7:46 PM, Aleksey Shipilev wrote: >> Hi, >> >> On 03/01/2017 07:36 AM, David Holmes wrote: >>> The INDY-fication of string concatenation has triggered a problem where a >>> JVM TI >>> agent's monitor-wait/ed callback hits an error path that uses string concat >>> which triggers a mass of indy related initialization, which in turn hits >>> monitor >>> use in MethodType$ConcurrentWeakInternSet.get, which causes the VM monitor >>> subsystem to be re-entered (and it is not reentrant!) so we crash. (log >>> extract >>> below - the amount of code to process this is truly scary!) >> >> Ouch. This is an unexpected circularity. It is unusual to see Java thread to >> do >> Java stuff when doing Object.wait. > > That's the fatal flaw in JVM TI. It pretends you can invoke arbitrary > Java code via an agent callback. You can invoke simple Java code - as > most real callbacks do - but not arbitrary Java code. The VM is not > reentrant in some subsystems - like monitors. > >>> I assume I can cover the exact case above by replicating it? But can I >>> generalize it to cover arbitrary string concat expressions that might arise >>> on >>> other error paths? >> >> Yes, the StringConcatFactory code is deliberately lazy. If you want to link >> eagerly, you would need to match the concat shape exactly (number and type of >> arguments) -- probably by using the same concat code the test failed on. We >> can >> probably eagerly link some popular concat shapes early during system init, >> but >> that requires JDK changes. > > So I can cover the current failing case easily enough but can't > generalize except by covering each case individually. > >> Looking at stack trace, it seems to be generic failure when adding new >> MethodType to the MethodType cache. This is not the first time that cache >> participates in circularities (I wonder if it *always* does). But I am >> puzzled >> how does this happen: >> >> ... >> V [jvm.dll+0x2b5412] Runtime1::monitorenter+0x1e2;; >> ?monitorenter@Runtime1@@CAXPAVJavaThread@@PAVoopDesc@@PAVBasicObjectLock@@@Z+0x1e2 >> v ~RuntimeStub::monitorenter_nofpu Runtime1 stub >> J 192 c1 >> java.lang.invoke.MethodType$ConcurrentWeakInternSet.get(Ljava/lang/Object;)Ljava/lang/Object; >> java.base@9-internal (54 bytes) @ 0x0242d7da [0x0242d4c0+0x031a] >> ... >> >> It calls RuntimeStub::monitorenter_nofpu destructor? Why it ends up calling >> into >> monitorenter? There are no locks in j.l.i.MT$CWIS.get (I checked the bytecode >> too). > > get() is compiled so I'm assuming it has inlined something from CHM that > does the locking. > >> If you want a nuclear option for your test, you may want to pass >> -XDstringConcat:inline to javac to disable indy string concat to bypass that >> circularity completely. > > H ... that is worth thinking about. > > Thanks, > David > >> Thanks, >> -Aleksey
Re: Forcing initialization of string concat INDY expressions
Hi Aleksey, On 1/03/2017 7:46 PM, Aleksey Shipilev wrote: Hi, On 03/01/2017 07:36 AM, David Holmes wrote: The INDY-fication of string concatenation has triggered a problem where a JVM TI agent's monitor-wait/ed callback hits an error path that uses string concat which triggers a mass of indy related initialization, which in turn hits monitor use in MethodType$ConcurrentWeakInternSet.get, which causes the VM monitor subsystem to be re-entered (and it is not reentrant!) so we crash. (log extract below - the amount of code to process this is truly scary!) Ouch. This is an unexpected circularity. It is unusual to see Java thread to do Java stuff when doing Object.wait. That's the fatal flaw in JVM TI. It pretends you can invoke arbitrary Java code via an agent callback. You can invoke simple Java code - as most real callbacks do - but not arbitrary Java code. The VM is not reentrant in some subsystems - like monitors. I assume I can cover the exact case above by replicating it? But can I generalize it to cover arbitrary string concat expressions that might arise on other error paths? Yes, the StringConcatFactory code is deliberately lazy. If you want to link eagerly, you would need to match the concat shape exactly (number and type of arguments) -- probably by using the same concat code the test failed on. We can probably eagerly link some popular concat shapes early during system init, but that requires JDK changes. So I can cover the current failing case easily enough but can't generalize except by covering each case individually. Looking at stack trace, it seems to be generic failure when adding new MethodType to the MethodType cache. This is not the first time that cache participates in circularities (I wonder if it *always* does). But I am puzzled how does this happen: ... V [jvm.dll+0x2b5412] Runtime1::monitorenter+0x1e2;; ?monitorenter@Runtime1@@CAXPAVJavaThread@@PAVoopDesc@@PAVBasicObjectLock@@@Z+0x1e2 v ~RuntimeStub::monitorenter_nofpu Runtime1 stub J 192 c1 java.lang.invoke.MethodType$ConcurrentWeakInternSet.get(Ljava/lang/Object;)Ljava/lang/Object; java.base@9-internal (54 bytes) @ 0x0242d7da [0x0242d4c0+0x031a] ... It calls RuntimeStub::monitorenter_nofpu destructor? Why it ends up calling into monitorenter? There are no locks in j.l.i.MT$CWIS.get (I checked the bytecode too). get() is compiled so I'm assuming it has inlined something from CHM that does the locking. If you want a nuclear option for your test, you may want to pass -XDstringConcat:inline to javac to disable indy string concat to bypass that circularity completely. H ... that is worth thinking about. Thanks, David Thanks, -Aleksey
Re: Forcing initialization of string concat INDY expressions
Hi, On 03/01/2017 07:36 AM, David Holmes wrote: > The INDY-fication of string concatenation has triggered a problem where a JVM > TI > agent's monitor-wait/ed callback hits an error path that uses string concat > which triggers a mass of indy related initialization, which in turn hits > monitor > use in MethodType$ConcurrentWeakInternSet.get, which causes the VM monitor > subsystem to be re-entered (and it is not reentrant!) so we crash. (log > extract > below - the amount of code to process this is truly scary!) Ouch. This is an unexpected circularity. It is unusual to see Java thread to do Java stuff when doing Object.wait. > I assume I can cover the exact case above by replicating it? But can I > generalize it to cover arbitrary string concat expressions that might arise on > other error paths? Yes, the StringConcatFactory code is deliberately lazy. If you want to link eagerly, you would need to match the concat shape exactly (number and type of arguments) -- probably by using the same concat code the test failed on. We can probably eagerly link some popular concat shapes early during system init, but that requires JDK changes. Looking at stack trace, it seems to be generic failure when adding new MethodType to the MethodType cache. This is not the first time that cache participates in circularities (I wonder if it *always* does). But I am puzzled how does this happen: ... V [jvm.dll+0x2b5412] Runtime1::monitorenter+0x1e2;; ?monitorenter@Runtime1@@CAXPAVJavaThread@@PAVoopDesc@@PAVBasicObjectLock@@@Z+0x1e2 v ~RuntimeStub::monitorenter_nofpu Runtime1 stub J 192 c1 java.lang.invoke.MethodType$ConcurrentWeakInternSet.get(Ljava/lang/Object;)Ljava/lang/Object; java.base@9-internal (54 bytes) @ 0x0242d7da [0x0242d4c0+0x031a] ... It calls RuntimeStub::monitorenter_nofpu destructor? Why it ends up calling into monitorenter? There are no locks in j.l.i.MT$CWIS.get (I checked the bytecode too). If you want a nuclear option for your test, you may want to pass -XDstringConcat:inline to javac to disable indy string concat to bypass that circularity completely. Thanks, -Aleksey
Forcing initialization of string concat INDY expressions
The INDY-fication of string concatenation has triggered a problem where a JVM TI agent's monitor-wait/ed callback hits an error path that uses string concat which triggers a mass of indy related initialization, which in turn hits monitor use in MethodType$ConcurrentWeakInternSet.get, which causes the VM monitor subsystem to be re-entered (and it is not reentrant!) so we crash. (log extract below - the amount of code to process this is truly scary!) I'm trying to workaround the problem by forcing some earlier string concat in the thread in which the callback will execute. From what I can see the test already does: log(Thread.currentThread() + "some text"); but the error code is doing: log("Agent '" + agentName + "' finished execution (finishedSuccessfully: " + finishedSuccessfully + ")"); where finishedSuccessfully is a boolean, and so we have a different form of makeConcatWithConstants call site. I assume I can cover the exact case above by replicating it? But can I generalize it to cover arbitrary string concat expressions that might arise on other error paths? And yes I realize this error code could hit monitor use in numerous other places, but so far this is the only case that we have seen trigger. (Though I can't reproduce it - even if I force the error path to be taken.) Thanks, David - V [jvm.dll+0x3ba688] report_vm_error+0x48;; ?report_vm_error@@YAXPBDH00ZZ+0x48 V [jvm.dll+0x7608f0] ObjectMonitor::enter+0x170;; ?enter@ObjectMonitor@@QAEXPAVThread@@@Z+0x170 V [jvm.dll+0x862035] ObjectSynchronizer::slow_enter+0x1d5;; ?slow_enter@ObjectSynchronizer@@SAXVHandle@@PAVBasicLock@@PAVThread@@@Z+0x1d5 V [jvm.dll+0x85fe8f] ObjectSynchronizer::fast_enter+0xff;; ?fast_enter@ObjectSynchronizer@@SAXVHandle@@PAVBasicLock@@_NPAVThread@@@Z+0xff V [jvm.dll+0x2b5412] Runtime1::monitorenter+0x1e2;; ?monitorenter@Runtime1@@CAXPAVJavaThread@@PAVoopDesc@@PAVBasicObjectLock@@@Z+0x1e2 v ~RuntimeStub::monitorenter_nofpu Runtime1 stub J 192 c1 java.lang.invoke.MethodType$ConcurrentWeakInternSet.get(Ljava/lang/Object;)Ljava/lang/Object; java.base@9-internal (54 bytes) @ 0x0242d7da [0x0242d4c0+0x031a] J 190 c1 java.lang.invoke.MethodType.makeImpl(Ljava/lang/Class;[Ljava/lang/Class;Z)Ljava/lang/invoke/MethodType; java.base@9-internal (66 bytes) @ 0x0242ca20 [0x0242c9a0+0x0080] J 255 c1 java.lang.invoke.MemberName.getMethodOrFieldType()Ljava/lang/invoke/MethodType; java.base@9-internal (72 bytes) @ 0x02445238 [0x02444f20+0x0318] J 239 c1 java.lang.invoke.InvokerBytecodeGenerator.isStaticallyInvocable(Ljava/lang/invoke/MemberName;)Z java.base@9-internal (169 bytes) @ 0x0243dc28 [0x0243d820+0x0408] j java.lang.invoke.InvokerBytecodeGenerator.addMethod()V+648 java.base@9-internal j java.lang.invoke.InvokerBytecodeGenerator.generateCustomizedCodeBytes()[B+6 java.base@9-internal j java.lang.invoke.InvokerBytecodeGenerator.generateCustomizedCode(Ljava/lang/invoke/LambdaForm;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/MemberName;+25 java.base@9-internal j java.lang.invoke.LambdaForm.compileToBytecode()V+69 java.base@9-internal j java.lang.invoke.LambdaForm.prepare()V+21 java.base@9-internal j java.lang.invoke.MethodHandle.(Ljava/lang/invoke/MethodType;Ljava/lang/invoke/LambdaForm;)V+33 java.base@9-internal j java.lang.invoke.BoundMethodHandle.(Ljava/lang/invoke/MethodType;Ljava/lang/invoke/LambdaForm;)V+3 java.base@9-internal j java.lang.invoke.BoundMethodHandle$Species_L9.(Ljava/lang/invoke/MethodType;Ljava/lang/invoke/LambdaForm;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V+3 java.base@9-internal j java.lang.invoke.BoundMethodHandle$Species_L9.copyWith(Ljava/lang/invoke/MethodType;Ljava/lang/invoke/LambdaForm;)Ljava/lang/invoke/BoundMethodHandle;+42 java.base@9-internal j java.lang.invoke.MethodHandles.dropArguments0(Ljava/lang/invoke/MethodHandle;ILjava/util/List;)Ljava/lang/invoke/MethodHandle;+105 java.base@9-internal j java.lang.invoke.MethodHandles.dropArguments(Ljava/lang/invoke/MethodHandle;I[Ljava/lang/Class;)Ljava/lang/invoke/MethodHandle;+6 java.base@9-internal j java.lang.invoke.StringConcatFactory$MethodHandleInlineCopyStrategy.generate(Ljava/lang/invoke/MethodType;Ljava/lang/invoke/StringConcatFactory$Recipe;)Ljava/lang/invoke/MethodHandle;+479 java.base@9-internal j java.lang.invoke.StringConcatFactory.generate(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/StringConcatFactory$Recipe;)Ljava/lang/invoke/MethodHandle;+101 java.base@9-internal j java.lang.invoke.StringConcatFactory.doStringConcat(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;ZLjava/lang/String;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;+507 java.base@9-internal j