review (S) for 6943485: JVMTI always on capabilities change code generation too much

[daniel.daugherty at oracle.com]

On 4/22/2010 4:43 PM, Tom Rodriguez wrote:
> On Apr 22, 2010, at 11:58 AM, daniel.daugherty at oracle.com wrote:
>
>   
>> On 4/20/2010 4:39 PM, Tom Rodriguez wrote:
>>     
>>> http://cr.openjdk.java.net/~never/6943485
>>>  
>>>       
>> Summary: thumbs up
>>
>> I updated the bug comments with some code history stuff.
>>     
>
> Thanks.
>
>   
>> src/share/vm/opto/c2compiler.cpp
>>   So it's okay to EscapeAnalysis when redefining classes?
>>     
>
> I believe so.  If the methods are redefined then the code should be invalidated and recompiled.  There might be one place where we need an evol dependency but I need to check into that.  We had previously been running all the JVMTI tests with EA enabled at various points and hadn't seen any issues.
>   

That's good to know!


>> src/share/vm/prims/jvmtiManageCapabilities.cpp
>>   In addition to can_generate_breakpoint_events, the
>>   can_generate_frame_pop_events should be queried also when
>>   setting can_access_local_variables.
>>     
>
> Why? 
>   

When the can_generate_frame_pop_events capability is enabled,
you can call NotifyFramePop() on a thread that is suspended or
on your own thread. You also specify a depth value that ids the
frame in which you are interested.

You either resume the target thread or continue execution in
your own thread. When the target frame is popped from the stack,
a FramePop event is generated. From the FramePop event handler,
you can access local variables and get the bci and...

However, you have to use the GetLocal APIs which already require
can_access_local_variables...

It looks like a similar argument can be made for not needing
can_generate_breakpoint_events... The Breakpoint event handler
gets similar parameters to the FramePop event handler so why
did you add can_generate_breakpoint_events?

What about GetFrameLocation()? Do you need access to locals in
order to get the current execution location (bci)? That API
doesn't require a capability so I'm guessing that it doesn't
need locals access to do its work...

Dan