RFR(S): 8035493: JVMTI PopFrame capability must instruct compilers not to prune locals

Coleen Phillimore coleen.phillimore at oracle.com
Fri Feb 21 22:53:11 UTC 2014


New fix is better.  Sorry I missed the subtlety of the conditional.   I 
thought the difference was part of the bug fix.
Coleen

On 2/21/14 5:24 PM, serguei.spitsyn at oracle.com wrote:
> Hi Markus,
>
> The fix is good.
>
> On 2/21/14 1:11 PM, Vladimir Kozlov wrote:
>> Indent is off:
>>
>> +  if (!_jvmti_can_access_local_variables &&
>> +    JvmtiExport::can_access_local_variables()) {
>
> Also, the return statements there have incorrect indent 4.
>
> +bool ciEnv::jvmti_state_changed() const {
> +  if (!_jvmti_can_access_local_variables &&
> +    JvmtiExport::can_access_local_variables()) {
> +      return true;
> +  }
> +  if (!_jvmti_can_hotswap_or_post_breakpoint &&
> +    JvmtiExport::can_hotswap_or_post_breakpoint()) {
> +      return true;
> +  }
> +  if (!_jvmti_can_post_on_exceptions &&
> +    JvmtiExport::can_post_on_exceptions()) {
> +      return true;
> +  }
> +  if (!_jvmti_can_pop_frame &&
> +    JvmtiExport::can_pop_frame()) {
> +      return true;
> +  }
> +  return false;
>   }
>
> Thanks,
> Serguei
>
>>
>> otherwise it is good.
>>
>> Thanks,
>> Vladimir
>>
>> On 2/21/14 12:27 PM, Markus Gronlund wrote:
>>> Hi Vladimir,
>>>
>>> You are absolutely right, many thanks for pointing it out - I should 
>>> have inspected that more closely...
>>>
>>> Updated:
>>>
>>> http://cr.openjdk.java.net/~mgronlun/8035493/webrev02/
>>>
>>>
>>> Thanks again
>>> Best regards
>>>
>>> Markus
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Vladimir Kozlov
>>> Sent: den 21 februari 2014 19:06
>>> To: Markus Gronlund; serviceability-dev at openjdk.net; 
>>> hotspot-runtime-dev; hotspot-compiler-dev at openjdk.java.net
>>> Subject: Re: RFR(S): 8035493: JVMTI PopFrame capability must 
>>> instruct compilers not to prune locals
>>>
>>> Markus,
>>>
>>> jvmti_state_changed() produces different result than the code it 
>>> replaced. If original jvmti cached values were true we did not 
>>> bailout compilation regardless their current. Only with change from 
>>> false to true we do that.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 2/21/14 2:53 AM, Markus Gronlund wrote:
>>>> Greetings,
>>>>
>>>> Kindly asking for reviews for the following changeset:
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~mgronlun/8035493/webrev01/
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8035493
>>>>
>>>> Problem statement summary and details are outlined in bug 
>>>> JDK-8035493 <https://bugs.openjdk.java.net/browse/JDK-8035493> .
>>>>
>>>> Testing completed:
>>>>
>>>> nsk/jvmti/scenarios/* (this includes JVMTI Hotswap and PopFrame
>>>> testing)
>>>>
>>>> hotspot/test/compiler/*
>>>>
>>>> Additionals:
>>>>
>>>> I would like, if possible, if we could move away from intertwining
>>>> specific jvmti capabilities inside the different compilers.
>>>>
>>>> The existing code makes it difficult to evolve and extend with
>>>> additional instructions to the compilers, esp. if we would like to
>>>> pass results from higher level conditions, perhaps by combining
>>>> contextual data with/without JVMTI. If possible, a suggestion would be
>>>> the creation of a higher level interface which the ciEnv can query 
>>>> for compilation instructions- the implementations of this interface 
>>>> could then be shielded away and allow for any type of combinatorial 
>>>> logic - leaving the code in the compilers themselves untouched.
>>>>
>>>> Thanks in advance
>>>>
>>>> Markus
>>>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140221/980b18bf/attachment-0001.html 


More information about the hotspot-runtime-dev mailing list