Request Review: 6902182: Starting with jdwp agent should not incur performance penalty

[Tom Rodriguez]

That looks good to me.  Dan said he'd take a look at the JVMTI parts.

tom

On Dec 15, 2009, at 12:59 PM, Deneau, Tom wrote:

> In http://cr.openjdk.java.net/~tdeneau/6902182/webrev.03
> the issues raised by Tom Rodriguez below have been addressed.
> 
> 
>   * there is now a common routine in graphKit.cpp called from both
>     GraphKit::builtin_throw and from parse2.cpp
> 
>   * in parse2.cpp, it falls thru, if the uncommon trap is not taken
> 
>   * uses local thread variable in the two runtime routines.
> 
> -- Tom D.
> 
> ======================================================================================
> 
> -----Original Message-----
> From: Thomas.Rodriguez at Sun.COM [mailto:Thomas.Rodriguez at Sun.COM] 
> Sent: Monday, December 14, 2009 6:51 PM
> To: Deneau, Tom
> Cc: hotspot-compiler-dev at openjdk.java.net
> Subject: Re: Request Review: 6902182: Starting with jdwp agent should not incur performance penalty
> 
> 
> On Dec 14, 2009, at 1:26 PM, Deneau, Tom wrote:
> 
>> New webrev is at  http://cr.openjdk.java.net/~tdeneau/6902182/webrev.02/
>> 
>> This rev changes
>> 
>>  * two places in the compiler where code for exception throws is
>>    being JITted (see parse2.cpp, graphKit.cpp).  I was thinking the
>>    common code in these two should be extracted to one place but I
>>    wasn't sure whether that belonged in graphKit.cpp or in
>>    macro.cpp.
> 
> GraphKit is base class containing most of the idiomatic code generation so you could put it there.
> 
>> 
>>  * trace_exception in opto/runtime.cpp
>> 
>>  * exception_handler_for_pc_helper in c1/c1_Runtime1.cpp
> 
> In the two, why don't you use the thread that's in a local instead of calling JavaThread::current()?
> 
> In parse2.cpp why don't you just fall through instead of duplicating that code?
> 
> otherwise this looks good to me.
> 
> tom
> 
>> 
>> 
>> Previously these places checked jvmti_can_post_exceptions() which only
>> looked at whether the jvmti capabilities for exceptions were enabled,
>> taking a slow path if true.
>> 
>> Now they check a new flag JavaThread::_must_post_exception_events.
>> This new flag is updated by calling jvmtiExport::must_post_exception_events
>> whenever the jvmti situation for a thread might have changed.
>> 
>> jvmtiExport::must_post_exception_events uses logic similar to that
>> used by jvmtiExport::post_exception_throw and returns false if
>> jvmtiExport::post_exception_throw wouldn't have done anything.
>> 
>> I would appreciate it if someone familiar with the jvmti codepaths
>> could review this to make sure that the must_post_exception_events
>> flag is being updated in all the necessary places.  Right now, it gets
>> updated in
>>  * JavaThread::set_jvmti_thread_state
>>  * JvmtiEventControllerPrivate::recompute_enabled
>> 
>> 
>> 
>> 
>> 
> 
> 
>