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

[Thomas Schatzl]

Hi,

On Thu, 2013-09-12 at 15:09 +0200, Bengt Rutisson wrote:
> 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.
> 

As mentioned, if you are okay with that, it's fine with me.

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

Thanks.

> 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.

Thomas