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

[Coleen Phillimore]

This still looks good to me except I missed the unhandled oop problem 
the first time.  Good find, Keith.
Thanks,
coleen

On 6/19/2012 9:08 PM, Jiangli Zhou wrote:
> 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
>>>>
>>
>