Request for review 7178145: Change constMethodOop::_exception_table to optionally inlined u2 table

[Jiangli Zhou]

Hi Keith,

Here is the updated webrev: 
http://cr.openjdk.java.net/~jiangli/7178145/webrev.01/.

I fixed the loops in  methodOop::fast_exception_handler_bci_for(), 
ClassVerifier::verify_exception_handler_tableOn() and 
ClassVerifier::verify_exception_handler_targets() to reacquire the 
exception table inside the loop. I didn't apply the same change to 
GenerateOopMap::mark_bbheaders_and_count_gc_points(), since it was only 
using a typeArrayOop instead a handle to the table before. Please let me 
know if that looks okay to you.

Thanks,

Jiangli


06/19/2012 04:40 PM, Jiangli Zhou wrote:
> Hi Keith,
>
> Thanks for the review and comments!
>
> On 06/19/2012 04:01 PM, keith mcguigan wrote:
>>
>> Hi Jiangli,
>>
>> In methodOop.hpp, the class ExceptionTable holds a raw pointer into 
>> the heap (the pointer to the embedded table).  This is used in 
>> methodOop::fast_exception_handler_bci_for() in a loop when searching 
>> for the bci.  If class loading is initiated , a Full GC could occur 
>> which could invalidate the pointer in ExceptionTable on the next pass 
>> through the loop.  Need some sort of handle or recalculation in that 
>> loop. Perhaps a No_Safepoint_Verifier to catch any other places like 
>> this?
>
> Wow, so the embedded table could move along with the methodOop when 
> fast_exception_handler_bci_for() is looping through it if there is a 
> GC happening at the same time due to class loading. Thanks for 
> pointing that out! The original code was ok as it was using a 
> typeArrayHandle to the separate table, which is GC 'safe'.
>
> It seems No_Safepoint_Verifier asserts if it passes a safe point, 
> which might not be what fast_exception_handler_bci_for() should do. 
> I'll take your first suggestion and initialize the table inside the 
> loop. :)
>
>>
>> Also in ClassVerifier::verify_exception_handler_targets().
>>
>> I didn't look to see if this was possible in generateOopMaps code 
>> too, but you might want to take a look at that too -- it's definitely 
>> looping with that raw pointer (I didn't follow through to see if it 
>> could safepoint).
>
> Will fix those.
>
>>
>> Other than that one issue, I think this looks really good.
>
> Thanks a lot!
>
> Jiangli
>>
>> -- 
>> - Keith
>>
>> On 6/19/2012 5:24 PM, Jiangli Zhou wrote:
>>> Hi,
>>>
>>> Please review the following webrev for method exception handler table
>>> memory reduction:
>>>
>>> http://javaweb.sfbay.sun.com/~jianzhou/webrev.exceptiontable/
>>>
>>> Per VM spec the method exception handler table is tuples of u2's, 
>>> but in
>>> hotspot the data are stored as ints in constMethodOop::_exception_table
>>> (a typeArray). The change reduces the exception handler table memory
>>> usage by:
>>>
>>>   * Changing the exception handler table from int array to u2 array.
>>>   * Making the exception handler table as conditionally allocated
>>>     inlined table in constMethod, and eliminate the _exception_table
>>>     field. The inlined table is only allocated when the exception table
>>>     attribute exists for the method.
>>>   * Removing the overhead of the extra typeArray, the data is directly
>>>     stored in the inlined table when exception handler exists for a 
>>> method.
>>>
>>> Tested with runthese, jprt and vm.quick.testlist. No noticeable
>>> performance degradation from specjvm98 and specjbb2005.
>>>
>>> ============================================================================== 
>>>
>>> logs.exceptiontable_baseline.1:
>>> Benchmark Samples Mean Stdev Geomean Weight
>>> specjbb2005 8 56372.58 763.16
>>> specjvm98 8 534.50 19.50
>>> ============================================================================== 
>>>
>>> logs.exceptiontable.1:
>>> Benchmark Samples Mean Stdev %Diff P Significant
>>> specjbb2005 8 56734.05 880.76 0.64 0.395 *
>>> specjvm98 8 531.05 21.00 -0.65 0.738 *
>>> ============================================================================== 
>>>
>>>
>>> ============================================================================== 
>>>
>>> logs.exceptiontable_baseline.2:
>>> Benchmark Samples Mean Stdev Geomean Weight
>>> specjvm98 8 527.63 17.10
>>> ============================================================================== 
>>>
>>> logs.exceptiontable.2:
>>> Benchmark Samples Mean Stdev %Diff P Significant
>>> specjvm98 8 536.79 20.54 1.74 0.349 *
>>> ============================================================================== 
>>>
>>>
>>> Thanks,
>>>
>>> Jiangli
>>>
>