CRR: 7034139: G1: assert(Thread::current()->is_ConcurrentGC_thread()) failed: only a conc GC thread can call this (S)
Bengt Rutisson
bengt.rutisson at oracle.com
Fri Apr 29 06:05:36 UTC 2011
Ramki,
I totally agree with you. Most important is to have some kind of general
guideline so that we can start working towards a common way of
implementing non-product code.
I see your (and Tony's) point about PRODUCT_RETURN having been the way
to do this. However, I don't fully appreciate the "clutter argument"
against using a macro at the call site.
To me the line "force_overflow()->update()" below is clutter if I am
debugging this code from a product build since it does not do anything.
But from just going through this code it is not clear that it is
clutter. I actually have to go and understand what force_overflow()
does. So, changing the line to "NOT_PRODUCT(force_overflow()->update())"
is actually, IMHO, adding information - not adding clutter. The clutter
is already there. It just doesn't look like clutter. (Sorry for getting
all philosophical here :-) )
if (task_num == 0) {
clear_marking_state(concurrent() /* clear_overflow */);
force_overflow()->update();
if (PrintGC) {
gclog_or_tty->date_stamp(PrintGCDateStamps);
gclog_or_tty->stamp(PrintGCTimeStamps);
gclog_or_tty->print_cr("[GC concurrent-mark-reset-for-overflow]");
}
}
Anyway, Tony, since we don't have any common rule for this, or maybe the
rule is even to use PRODUCT_RETURN, I don't think you have to re-write
your code for this change. I am more interested in the general direction
that we will go in.
Bengt
On 2011-04-29 01:08, Y. S. Ramakrishna wrote:
> 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
More information about the hotspot-gc-dev
mailing list