Re: RFR 8192004: InspectedFrame.materializeVirtualObjects only updates locals with new objects

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8192004: InspectedFrame.materializeVirtualObjects only updates locals with new objects

Vladimir Kozlov
Posting to hotspot-dev alias since our group does not have JVMTI experts.
I looked and it seems fine but someone from serviceability should look too.

Thanks,
Vladimir

On 12/22/17 11:08 AM, Tom Rodriguez wrote:

> Could I get some reviews?  This doesn't change the way the logic behaves
> for the core use of JVMTI.  It just extends the encoding so that numbers
> after the locals are interpreted as expression stack and monitor values.
>   I believe there are existing tests of the JVMTI set locals part and
> JVMCI is the only only consumer of the monitor and expression stack part.
>
> tom
>
> Tom Rodriguez wrote:
>> JVMCI adds the ability to introspect on deoptimized frames which
>> requires early materialization of escape analyzed objects. The
>> jvmtiDeferredLocalVariableSet machinery is reused to capture the local
>> updates required for this. The existing code only updates locals,
>> leaving the stack and monitor information with out of date values. This
>> can lead to deadlocks and incorrect execution. The fix is to slightly
>> generalize jvmtiDeferredLocalVariableSet to handle expression stack and
>> monitor slots. Tested with new graal regression test
>> https://github.com/graalvm/graal/blob/7fd37bde8955780a57049964d87a51aa2407d86b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotStackIntrospectionTest.java 
>>
>> and previously failing Truffle use cases. I assume the new test case
>> will come across with a normal graal update. The clean mach5 run is in
>> the bug report.
>>
>> http://cr.openjdk.java.net/~never/8192004/webrev
>> https://bugs.openjdk.java.net/browse/JDK-8192004
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8192004: InspectedFrame.materializeVirtualObjects only updates locals with new objects

Vladimir Kozlov
No response from JVMTI so I looked again through it. It seems fine to
me. Reviewed.

Thanks,
Vladimir

On 12/22/17 12:14 PM, Vladimir Kozlov wrote:

> Posting to hotspot-dev alias since our group does not have JVMTI experts.
> I looked and it seems fine but someone from serviceability should look too.
>
> Thanks,
> Vladimir
>
> On 12/22/17 11:08 AM, Tom Rodriguez wrote:
>> Could I get some reviews?  This doesn't change the way the logic
>> behaves for the core use of JVMTI.  It just extends the encoding so
>> that numbers after the locals are interpreted as expression stack and
>> monitor values.   I believe there are existing tests of the JVMTI set
>> locals part and JVMCI is the only only consumer of the monitor and
>> expression stack part.
>>
>> tom
>>
>> Tom Rodriguez wrote:
>>> JVMCI adds the ability to introspect on deoptimized frames which
>>> requires early materialization of escape analyzed objects. The
>>> jvmtiDeferredLocalVariableSet machinery is reused to capture the local
>>> updates required for this. The existing code only updates locals,
>>> leaving the stack and monitor information with out of date values. This
>>> can lead to deadlocks and incorrect execution. The fix is to slightly
>>> generalize jvmtiDeferredLocalVariableSet to handle expression stack and
>>> monitor slots. Tested with new graal regression test
>>> https://github.com/graalvm/graal/blob/7fd37bde8955780a57049964d87a51aa2407d86b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotStackIntrospectionTest.java 
>>>
>>> and previously failing Truffle use cases. I assume the new test case
>>> will come across with a normal graal update. The clean mach5 run is in
>>> the bug report.
>>>
>>> http://cr.openjdk.java.net/~never/8192004/webrev
>>> https://bugs.openjdk.java.net/browse/JDK-8192004
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8192004: InspectedFrame.materializeVirtualObjects only updates locals with new objects

serguei.spitsyn@oracle.com
Tom and Vladimir,

I'm looking at it.

Thanks,
Serguei

On 1/16/18 09:16, Vladimir Kozlov wrote:

> No response from JVMTI so I looked again through it. It seems fine to
> me. Reviewed.
>
> Thanks,
> Vladimir
>
> On 12/22/17 12:14 PM, Vladimir Kozlov wrote:
>> Posting to hotspot-dev alias since our group does not have JVMTI
>> experts.
>> I looked and it seems fine but someone from serviceability should
>> look too.
>>
>> Thanks,
>> Vladimir
>>
>> On 12/22/17 11:08 AM, Tom Rodriguez wrote:
>>> Could I get some reviews?  This doesn't change the way the logic
>>> behaves for the core use of JVMTI. It just extends the encoding so
>>> that numbers after the locals are interpreted as expression stack
>>> and monitor values.   I believe there are existing tests of the
>>> JVMTI set locals part and JVMCI is the only only consumer of the
>>> monitor and expression stack part.
>>>
>>> tom
>>>
>>> Tom Rodriguez wrote:
>>>> JVMCI adds the ability to introspect on deoptimized frames which
>>>> requires early materialization of escape analyzed objects. The
>>>> jvmtiDeferredLocalVariableSet machinery is reused to capture the local
>>>> updates required for this. The existing code only updates locals,
>>>> leaving the stack and monitor information with out of date values.
>>>> This
>>>> can lead to deadlocks and incorrect execution. The fix is to slightly
>>>> generalize jvmtiDeferredLocalVariableSet to handle expression stack
>>>> and
>>>> monitor slots. Tested with new graal regression test
>>>> https://github.com/graalvm/graal/blob/7fd37bde8955780a57049964d87a51aa2407d86b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotStackIntrospectionTest.java 
>>>>
>>>> and previously failing Truffle use cases. I assume the new test case
>>>> will come across with a normal graal update. The clean mach5 run is in
>>>> the bug report.
>>>>
>>>> http://cr.openjdk.java.net/~never/8192004/webrev
>>>> https://bugs.openjdk.java.net/browse/JDK-8192004
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8192004: InspectedFrame.materializeVirtualObjects only updates locals with new objects

serguei.spitsyn@oracle.com
In reply to this post by Vladimir Kozlov
It looks good to me.
Sorry for the latency.

Thanks,
Serguei


On 1/16/18 09:16, Vladimir Kozlov wrote:

> No response from JVMTI so I looked again through it. It seems fine to
> me. Reviewed.
>
> Thanks,
> Vladimir
>
> On 12/22/17 12:14 PM, Vladimir Kozlov wrote:
>> Posting to hotspot-dev alias since our group does not have JVMTI
>> experts.
>> I looked and it seems fine but someone from serviceability should
>> look too.
>>
>> Thanks,
>> Vladimir
>>
>> On 12/22/17 11:08 AM, Tom Rodriguez wrote:
>>> Could I get some reviews?  This doesn't change the way the logic
>>> behaves for the core use of JVMTI. It just extends the encoding so
>>> that numbers after the locals are interpreted as expression stack
>>> and monitor values.   I believe there are existing tests of the
>>> JVMTI set locals part and JVMCI is the only only consumer of the
>>> monitor and expression stack part.
>>>
>>> tom
>>>
>>> Tom Rodriguez wrote:
>>>> JVMCI adds the ability to introspect on deoptimized frames which
>>>> requires early materialization of escape analyzed objects. The
>>>> jvmtiDeferredLocalVariableSet machinery is reused to capture the local
>>>> updates required for this. The existing code only updates locals,
>>>> leaving the stack and monitor information with out of date values.
>>>> This
>>>> can lead to deadlocks and incorrect execution. The fix is to slightly
>>>> generalize jvmtiDeferredLocalVariableSet to handle expression stack
>>>> and
>>>> monitor slots. Tested with new graal regression test
>>>> https://github.com/graalvm/graal/blob/7fd37bde8955780a57049964d87a51aa2407d86b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotStackIntrospectionTest.java 
>>>>
>>>> and previously failing Truffle use cases. I assume the new test case
>>>> will come across with a normal graal update. The clean mach5 run is in
>>>> the bug report.
>>>>
>>>> http://cr.openjdk.java.net/~never/8192004/webrev
>>>> https://bugs.openjdk.java.net/browse/JDK-8192004
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8192004: InspectedFrame.materializeVirtualObjects only updates locals with new objects

Tom Rodriguez-2
Thanks!

tom

[hidden email] wrote:

> It looks good to me.
> Sorry for the latency.
>
> Thanks,
> Serguei
>
>
> On 1/16/18 09:16, Vladimir Kozlov wrote:
>> No response from JVMTI so I looked again through it. It seems fine to
>> me. Reviewed.
>>
>> Thanks,
>> Vladimir
>>
>> On 12/22/17 12:14 PM, Vladimir Kozlov wrote:
>>> Posting to hotspot-dev alias since our group does not have JVMTI
>>> experts.
>>> I looked and it seems fine but someone from serviceability should
>>> look too.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 12/22/17 11:08 AM, Tom Rodriguez wrote:
>>>> Could I get some reviews?  This doesn't change the way the logic
>>>> behaves for the core use of JVMTI. It just extends the encoding so
>>>> that numbers after the locals are interpreted as expression stack
>>>> and monitor values.   I believe there are existing tests of the
>>>> JVMTI set locals part and JVMCI is the only only consumer of the
>>>> monitor and expression stack part.
>>>>
>>>> tom
>>>>
>>>> Tom Rodriguez wrote:
>>>>> JVMCI adds the ability to introspect on deoptimized frames which
>>>>> requires early materialization of escape analyzed objects. The
>>>>> jvmtiDeferredLocalVariableSet machinery is reused to capture the local
>>>>> updates required for this. The existing code only updates locals,
>>>>> leaving the stack and monitor information with out of date values.
>>>>> This
>>>>> can lead to deadlocks and incorrect execution. The fix is to slightly
>>>>> generalize jvmtiDeferredLocalVariableSet to handle expression stack
>>>>> and
>>>>> monitor slots. Tested with new graal regression test
>>>>> https://github.com/graalvm/graal/blob/7fd37bde8955780a57049964d87a51aa2407d86b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotStackIntrospectionTest.java
>>>>>
>>>>> and previously failing Truffle use cases. I assume the new test case
>>>>> will come across with a normal graal update. The clean mach5 run is in
>>>>> the bug report.
>>>>>
>>>>> http://cr.openjdk.java.net/~never/8192004/webrev
>>>>> https://bugs.openjdk.java.net/browse/JDK-8192004
>>>
>