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

[Tom Rodriguez]

On Apr 22, 2010, at 4:20 PM, daniel.daugherty at oracle.com wrote:

> 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!

I added some changes in bcEscapeAnalyzer to make sure that evol dependencies are added when it's used.  Thanks for asking that question.

> 
> 
>>> 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?

It's mainly because keeping extra locals live is really targetted at a true debugger used by an end user which I figured would be marked by the use of breakpoints.  You can always access locals in Java frames even without can_access_local_variables it's just that if they weren't live anymore they will have their default Java value instead of whatever value was actually computed.  I've added can_generate_frame_pop_events to the set as you suggested since it seems reasonable.

> 
> 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...

No.  We always have that information around.

tom

> 
> Dan
>