RFR (S): 8227671: G1: assert_used_and_recalculate_used_equal performs work in product builds

[Kim Barrett]

> On Jul 15, 2019, at 11:38 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi all,
> 
>  can I have reviews for this small change that avoids doing
> unneccessary work in product builds?
> 
> In particular, the assert_used_and_recalculate_used_equal macro
> executes code (the recalculate_used() call) that is potentially
> lengthy.
> 
> I measured a few ms on a large heap (50GB) with (forced) 50k regions. I
> am not sure why this call is so expensive in my tests, but it is
> completely unnecessary.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8227671
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8227671/webrev/
> Testing:
> Currently running hs-tier1-3, local gc/g1 jtreg run, local benchmarks
> 
> Thanks,
>  Thomas

Looks good.

One minor nit issue, for which I don't need a new webrev.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1CollectedHeap.hpp
 366 #define assert_used_and_recalculate_used_equal(g1h) while(0)

That should probably be "do {} while (0)".  It probably doesn't
matter too much, since the case that can go wrong in a non-debug build
will probably not compile in a debug build.

------------------------------------------------------------------------------ 
src/hotspot/share/gc/g1/g1CollectedHeap.hpp
 356 #ifdef ASSERT
 357 #define assert_used_and_recalculate_used_equal(g1h)

I wonder why this is a macro and not a NOT_DEBUG_RETURN function.

The same goes for some of the others nearby.  I think macros were used
because we were getting poor stacktrace info in hs_err files, but I
thought that was pretty much fixed.

But this is all a separate issue from the performance bug being
addressed by this change.

------------------------------------------------------------------------------