RFR: 8024671: G1 generates assert error messages in product builds
bengt.rutisson at oracle.com
Thu Sep 12 06:09:37 PDT 2013
Hi Aleksey and Thomas,
On 9/12/13 2:40 PM, Thomas Schatzl wrote:
> 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:
>>>> Please review and push this simple change:
>>>> 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.
More information about the hotspot-gc-dev