RFR: 8024671: G1 generates assert error messages in product builds

[Bengt Rutisson]

Hi Aleksey and Thomas,

On 9/12/13 2:40 PM, Thomas Schatzl wrote:
> Hi,
>
> On Thu, 2013-09-12 at 16:37 +0400, Aleksey Shipilev wrote:
>> On 09/12/2013 04:30 PM, Thomas Schatzl wrote:
>>>   thanks for finding and fixing this. Also for catching the typo.
>> Thanks for the review, Thomas!
>>
>>> On Thu, 2013-09-12 at 15:59 +0400, Aleksey Shipilev wrote:
>>>> Hi,
>>>>
>>>> Please review and push this simple change:
>>>>    http://cr.openjdk.java.net/~shade/8024671/webrev.00/
>>>>
>>>> TL;DR: err_msg() is not getting erased in product builds, because
>>>> check_card_num is not a macro, causing the performance regression. Turns
>>>> out it is simpler to inline the check_card_num all along, because it is
>>>> more consistent to other asserts.
>>> I really do not like the inlining of the check_card_num() method,
>>> replicating the code. I would prefer an approach that avoids that.
>> I thought that too before starting the patch, targeting to declare the
>> proper macro for check_card_num. But, inline asserts seem be coherent
>> will other asserts in the affected code blocks. Hence, I changed my
>> mind, and produced the current patch.
>    we can always improve the code :)
>
> Also the use of check_card_num() has probably been an attempt to factor
> out the common checking code, although it had the performance side
> effects you found.

I think I prefer the inlined versions as suggested in this patch. It is 
only one line and this way it is very fast to see what is actually being 
checked. Defining a macro or something will be less readable in my opinion.

Thomas, if you are ok with the current patch I can sponsor the push.

Aleksey, I'll wait with doing the actual push until tomorrow to allow 
people in other time zones to have a look before this gets pushed. If 
there are no objections until tomorrow morning I'll start the push job.

Bengt

>
> Thomas
>