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

[Y. S. Ramakrishna]

I'm OK with either. The general question
(not the specific one in regards to this
specific changeset) would benefit from
input from the bigger hotspot-dev alias
(for reasons of global consistency, as was
pointed out).

I agree that having a clear discriminator at the
call-site definitely aids clarity. The clutter from
the macro perhaps offsets that a bit -- but the clutter
itself is the "signal" here, even if that may sound a
bit oxymoronic, so  may be there's an aesthetic balancing
act here to do, i suspect. So global style guidelines
will need to dictate behaviour, because as everyone noted
consistency (or in general, a monotonic move towards
consistency) would be good to keep the system from
going into an oscillatory state.

Historically, one has of course favoured the PRODUCT_RETURN*
form for historical reasons, but it's a good
idea to revisit historical norms every once in a
while -- as long as such revision is consistent and
global, so that the system as a whole advances towards the
desired goal.

I'll now get off the soap-box :-)
-- ramki

On 04/28/11 12:57, Tony Printezis wrote:
...
> 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?
> 
> Tony