CRR: 7034139: G1: assert(Thread::current()->is_ConcurrentGC_thread()) failed: only a conc GC thread can call this (S)

[John Coomes]

Tony Printezis (tony.printezis at oracle.com) wrote:
> ...
> > For example, I think I would prefer that concurrentMark.cpp had calls 
> > like:
> >
> > DEBUG_ONLY(force_overflow()->update());
> >
> > rather than having this in concurrentMark.hpp:
> >
> > void update() PRODUCT_RETURN;
> >
> > That way someone browsing the code in concurrentMark.cpp can quickly 
> > see the difference between test code and product code. I realize that 
> > this might be a matter of taste, so I'm ok with the way it is now as 
> > well.
> 
> I do see the point you and Jesper (he made in his subsequent e-mail) 
> and, of course :-), we're generally not consistent in taking one 
> approach over the other in HotSpot. Having said this, this is the sort 
> of situation for which the PRODUCT_RETURN macro was introduced in the 
> first place. I personally like using PRODUCT_RETURN since it cuts down 
> the clutter. Compare:
> 
> void foo() PRODUCT_RETURN;
> 
> #ifndef PRODUCT
> void foo() {
>   ...
> }
> #endif
> 
> foo();
> 
> with:
> 
> NOT_PRODUCT(void foo();)
> 
> #ifndef PRODUCT
> void foo() {
>   ...
> }
> #endif
> 
> NOT_PRODUCT(foo();)
> 
> 
> But I'd be OK with the latter for the reasons you and Jesper brought up. 
> Anyone else in group have a strong opinion one way or another?

Not a strong opinion, but I favor the latter a bit.  It also depends
on the code in question.  If the use point is something like
verify_card_table() then it's pretty obvious that it's non-product
code.  But if the use point is more generic like 'update()' or
'invalidate()' then you have to go look.

-John