RFR (7xS): 8178148: Log more detailed information about scan rs phase

[sangheon]

Hi Thomas,

On 05/15/2017 04:50 AM, Thomas Schatzl wrote:
> Hi Sangheon,
>
>    thanks for your review and sorry for the late reply; 8180048 got in
> the way :)
I totally understand you were busy with 8180048. :)

>
> On Tue, 2017-05-09 at 11:46 -0700, sangheon wrote:
>> Hi Thomas,
>>
>> On 05/05/2017 05:13 AM, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>     recent reviews have made changes necessary to parts of the
>>> changeset chain.
>>>
>>>
> [...]
>>> JDK-8178148: Log more detailed information about scan rs phase
>>> http://cr.openjdk.java.net/~tschatzl/8178148/webrev/
>> Looks good.
>> I have a few minor comments below:
>>
>> ------------------------------------------
>> src/share/vm/gc/g1/g1GCPhaseTimes.cpp
>> copyright update.
>>
>> -------------------------------------------
>> src/share/vm/gc/g1/g1GCPhaseTimes.hpp
>> copyright update.
>>
>>     83   };
>>     84  private:
>> line separator between 83 and 84.
>>
>> -------------------------------------------
>> src/share/vm/gc/g1/g1ParScanThreadState.hpp
>> copyright update.
>>
>> -------------------------------------------
>> src/share/vm/gc/g1/g1RemSet.hpp
>>    215   size_t cards_scanned() { return _cards_scanned; }
>>    216   size_t cards_claimed() { return _cards_claimed; }
>>    217   size_t cards_skipped() { return _cards_skipped; }
>> Adding const ?
> All fixed.
>
>> -------------------------------------------
>> src/share/vm/gc/g1/workerDataArray.inline.hpp
>>     65   assert(index < MaxThreadWorkItems, "Tried to access thread
>> work
>> item %u max %u", index, MaxThreadWorkItems);
>> Assert message can be improved with describing more explicitly like
>> which one is maximum value etc..
>> e.g. "... work item %u (max=%u)
>>
>>     71   assert(index < MaxThreadWorkItems, "Tried to access thread
>> work
>> item %u max %u", index, MaxThreadWorkItems);
>> Same as above.
>>
> The max value is printed as your example actually shows. But I added
> the parentheses.
>
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8178148/webrev.0_to_1 (diff)
> http://cr.openjdk.java.net/~tschatzl/8178148/webrev.1 (full)
Thanks for the updates. Webrev.1 looks good to me.

Thanks,
Sangheon


>
> Thanks,
>    Thomas
>