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

[Jiangli Zhou]

Hi Coleen,

Thanks very much for re-looking at the change. I made a small adjustment 
in src/share/vm/classfile/verifier.cpp to obtain the exception table 
length before the loops based on Keith suggestion. Everything else are 
the same.

http://cr.openjdk.java.net/~jiangli/7178145/webrev.02/


Thanks!

Jiangli

On 06/20/2012 02:36 PM, Coleen Phillimore wrote:
>
> 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
>>>>>
>>>
>>