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

[Tom Rodriguez]

On Dec 10, 2009, at 8:38 AM, Deneau, Tom wrote:

> Tom --
> 
> I agree it makes sense to cache a flag in the thread state for this.
> Should the flag be in JavaThread or in jvmti_thread_state?
> I assume it's slightly faster to access it if it's in JavaThread.
> Should it be a new flag or an unused bit in some existing flag?
> 
> Is the following the general template for generating the nodes to read this flag?
> 
>  Node* thread = transform_later(new (C, 1) ThreadLocalNode());
>  Node* adr = basic_plus_adr(top(),thread, in_bytes(JavaThread::new_flag_offset()));
>  Node* new_flag = make_load(NULL, adr, TypeOopPtr::BOTTOM, T_OBJECT, NoAlias, false);

In your case it would be more like:

Node* thread = _gvn.transform(new (C, 1) ThreadLocalNode());
Node* adr = basic_plus_adr(top(), thread, in_bytes(JavaThread::new_flag_offset()));
Node* new_flag = make_load(control(), adr, TypeInt::INT, T_INT, NoAlias, Compile::AliasIdxRaw);

The base argument to basic_plus_adr is the base pointer of an object if the AddP refers to an oop.  It's required for GC.  For this case it's not an oop so pass top().  You need to set the control on the load so that it doesn't get commoned.  Normally the memory graph would keep that from happening but it doesn't for raw memory.  I assumed the flag is an int so I used T_INT.  If it's a bool you'll need to test and select the proper basic type based on sizeof(bool) kind of the way that MacroAssembler::movbool selects the proper load size.  TypeInt::INT can stay the same though you could make it TypeInt::BOOL to indicate that it's an int value that's either 0 or 1.

tom


> 
> I don't really understand the first argument to basic_plus_adr()
> 
> I'll fix the other two can_post_exceptions cases you mentioned.
> 
> I can easily remove the global flag, I had it mostly for testing.
> 
> -- Tom D.
> 
> 
> -----Original Message-----
> From: Thomas.Rodriguez at Sun.COM [mailto:Thomas.Rodriguez at Sun.COM] 
> Sent: Wednesday, December 09, 2009 4:04 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
> 
> Why can't you use a flag in the JavaThread instead of performing a runtime call every time?  That just seems too expensive and complicated for every exception.  I don't think we want this under a flag either.  It should be the default behaviour.
> 
> By the way, another thing I think you want to fix is this code in runtime.cpp:
> 
>    if (JvmtiExport::can_post_exceptions()) {
>      // "Full-speed catching" is not necessary here,                                                         
>      // since we're notifying the VM on every catch.                                                         
>      // Force deoptimization and the rest of the lookup                                                     
>      // will be fine.                                                                                       
>      deoptimize_caller_frame(thread, true);
>    }
> 
> It should also be checking that someone actually wants to see the exception.  There's similar code in c1_Runtime1.cpp.  I don't the client compiler needs any other changes to work properly with debug enabled.
> 
> tom
> 
> On Dec 9, 2009, at 1:31 PM, Deneau, Tom wrote:
> 
>> Webrev is at  http://cr.openjdk.java.net/~tdeneau/6902182/webrev.01/
>> 
>> This webrev changes two places in the compiler where code for exception
>> throws is being JITted.
>> 
>>  * previously these checked jvmti_can_post_exceptions() at compile
>>    time, compiling in a slow path if true
>> 
>>  * now they call a new jvmtiExport::must_post_exception_events at
>>    run time, and take a faster path if must_post_exception_events is
>>    false.
>> 
>> 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.
>> 
>> A similar change of calling must_post_exception_events instead of
>> jvmti_can_post_exceptions is made in the exception runtime code in
>> trace_exception in opto/runtime.cpp
>> 
>> Throughput Numbers from a JIRA-based web workload (JIRA makes fairly heavy use
>> of exception throws and catches).  Bigger is better.
>> 
>>  unmodified hotspot, no jdwp agent       69.7
>>  unmodified hotspot, with jdwp agent     11.8 
>>  modified hotspot, with jdwp agent       65.7
>> 	
>> All changes are platform-independent.
>> 
>> -- Tom Deneau
>>  tom.deneau at amd.com
>> 
>> 
>> 
> 
> 
>