RFR(S): 7147724: G1: hang in SurrogateLockerThread::manipulatePLL

[John Cuthbertson]

Hi Bengt,

Thanks for looking again. Replies inline....

On 03/09/12 02:25, Bengt Rutisson wrote:
>
> Hi John,
>
> I think I like this change, but either I am confused or you forgot to 
> remove the "bool no_safepoint_check" from:
>
> G1CollectedHeap::humongous_obj_allocate_find_first()
> G1CollectedHeap::humongous_obj_allocate()
>
> Did you mean to remove it?

Yes I did mean to remove it. Done.

>
> Just a style comment on this piece of code:
>
>     bool safepoint_check_flag = Mutex::_no_safepoint_check_flag;
>     if (Thread::current()->is_Java_thread()) {
>       safepoint_check_flag = !Mutex::_no_safepoint_check_flag;
>     }
>
> I understand that we want to use Mutex::_no_safepoint_check_flag since 
> it is defined. But since there is no Mutex::_safepoint_check_flag or 
> similar I think we break the abstraction by doing 
> "safepoint_check_flag = !Mutex::_no_safepoint_check_flag". This kind 
> of exposes that we are dealing with a bool. So, given this I think I 
> would prefer to change the code to just:
>
> safepoint_check_flag = Thread::current()->is_Java_thread();

OK. Done, but with a small difference. For Java threads we wish to pass 
false so that we don't skip the safepoint check so I used "bool 
no_safepoint_check_flag = !Thread::current()->is_Java_thread();" and can 
then just pass in the value.

New webrev: http://cr.openjdk.java.net/~johnc/7147724/webrev.2/

Thanks,

JohnC