Request for Review (xs) - 8159073: Error handling incomplete when creating GC threads lazily

Jon Masamitsu jon.masamitsu at oracle.com
Thu Jul 21 20:17:51 UTC 2016


Thanks.

Jon

On 7/21/2016 1:04 PM, Derek White wrote:
> Hi Jon,
>
> Looks good!
>
>  - Derek
>
> On 7/21/16 9:57 AM, Thomas Schatzl wrote:
>> Hi Jon,
>>
>> On Thu, 2016-07-21 at 06:48 -0700, Jon Masamitsu wrote:
>>> On 7/21/2016 1:20 AM, Thomas Schatzl wrote:
>>>> Hi Jon,
>>>>
>>>> On Wed, 2016-07-20 at 20:43 -0700, Jon Masamitsu wrote:
>>>>> It's ready again.
>>>>>
>>>>> Delta: http://cr.openjdk.java.net/~jmasa/8159073/webrev_delta_01_
>>>>> 02/
>>>>> Full: http://cr.openjdk.java.net/~jmasa/8159073/webrev.02/
>>>>>
>>>>     this looks good, except for the two log messages. They seem to
>>>> be
>>>> superfluous because first the callers of run_task(, uint) already
>>>> print
>>>> mostly the same message, and the given code guarantee's that
>>>> whatever
>>>> is printed anyway. Further, a single invocation of run_task will
>>>> now
>>>> trigger three messages basically telling the same.
>>>>
>>>> So I would prefer if these messages were either moved to trace
>>>> level,
>>>> or just removed.
>>>>
>>>> Thanks a lot for your patience with me, :)
>>>>     Thomas
>>> I deleted the log messages.
>>>
>>> --- a/src/share/vm/gc/shared/workgroup.cpp
>>> +++ b/src/share/vm/gc/shared/workgroup.cpp
>>> @@ -272,11 +272,9 @@
>>>                task->name(), num_workers, total_workers());
>>>      guarantee(num_workers > 0, "Trying to execute task %s with zero
>>> workers", task->name());
>>>      uint old_num_workers = _active_workers;
>>> -  log_debug(gc)("run_task: updating active workers for %s from %u
>>> to
>>> %u", task->name(), old_num_workers, num_workers);
>>>      update_active_workers(num_workers);
>>>      guarantee(_active_workers == num_workers, "active workers %u
>>> num_workers %u", _active_workers, num_workers);
>>>      _dispatcher->coordinator_execute_on_workers(task, num_workers);
>>> -  log_debug(gc)("run_task: restoring active workers from %u to %u",
>>> num_workers, old_num_workers);
>>>      update_active_workers(old_num_workers);
>>>    }
>>>
>>> Delta: http://cr.openjdk.java.net/~jmasa/8159073/webrev_delta_02_03/
>>> Full: http://cr.openjdk.java.net/~jmasa/8159073/webrev.03/
>>>
>>> Thanks.
>>    looks good, ship it :)
>>
>> Thanks,
>>    Thomas
>>
>



More information about the hotspot-gc-dev mailing list