RFR: 8240263: Assertion-only call in Method::link_method affecting product builds

Claes Redestad claes.redestad at oracle.com
Tue Mar 3 13:37:31 UTC 2020


Thanks Aleksey, Dan and Ioi for reviewing!

Hearing no strong opinions the other way I pushed the ASSERT version.

/Claes

On 2020-03-02 19:36, Ioi Lam wrote:
> Looks good to me too. Thanks for finding this performance problem!
> 
> Thanks
> - Ioi
> 
> On 3/2/20 8:33 AM, Daniel D. Daugherty wrote:
>> I have a preference for the DEBUG_ONLY style, but that's probably
>> because I just did some of those in the ObjectMonitor subsystem.
>>
>> Either way: Thumbs up!
>>
>> Dan
>>
>> On 3/2/20 5:08 AM, Claes Redestad wrote:
>>>
>>>
>>> On 2020-03-02 10:22, Aleksey Shipilev wrote:
>>>> On 3/2/20 10:22 AM, Claes Redestad wrote:
>>>>> Guarding the resolution by DEBUG_ONLY seem the right thing to do here,
>>>>> which bring a small improvement on startup tests when CDS is enabled.
>>>>>
>>>>> See attached patch[1].
>>>>
>>>> I believe we usually do this in such case:
>>>>
>>>> #ifdef ASSERT
>>>>    address entry = Interpreter::entry_for_cds_method(h_method);
>>>>    assert(entry != NULL && entry == _i2i_entry,
>>>>           "should be correctly set during dump time");
>>>> #endif
>>>
>>> I've seen it done both ways, and have no strong preference.
>>>
>>>>
>>>> It keeps the block easily editable, without looking back at whether 
>>>> it interferes with macros.
>>>>
>>>> Otherwise looks fine, assuming release builds still pass.
>>>
>>> Running your suggested variant through tier 1 to make sure.
>>>
>>> Thanks!
>>>
>>> /Claes
>>
> 


More information about the hotspot-runtime-dev mailing list