RFR (XS) JDK-8160055 Misplaced call to ClassLoaderDataGraph::clear_claimed_marks during initial mark

[Joseph Provino]

Hi Misha, I'll fix those.

Thanks.

joe


On 10/21/2016 11:24 AM, Michail Chernov wrote:
> Hi Joe,
>
> There are some extra spaces in new and existing code:
>
> public static void main(String [] args)
> should be
> public static void main(String[] args)
>
> If you will not create new webrev with other changes, I don't need 
> separate review for this minor fix.
>
> Thanks,
> Michail
>
> On 21.10.2016 18:09, Joseph Provino wrote:
>> Hi Thomas, I think I've made the changes you requested.
>>
>> Webrev is http://cr.openjdk.java.net/~jprovino/8160055/webrev.04
>>
>> joe
>>
>> On 10/5/2016 7:00 AM, Thomas Schatzl wrote:
>>> Hi Joe,
>>>
>>> On Wed, 2016-09-28 at 12:39 -0400, Joseph Provino wrote:
>>>> Please review the latest webrev:
>>>> http://cr.openjdk.java.net/~jprovino/8160055/webrev.01
>>>>
>>>> There is a WhiteBox test now.
>>>>
>>>> thanks.
>>>>
>>>> joe
>>>>
>>> - the log message itself should be consistent with other messages, i.e.
>>> no "Time" at the end. (This is all timing output anyway).
>>>
>>> - I would prefer to have it listed within "Other" times, at a lower log
>>> level. Maybe only print it if its value is > 0.0 too at this time.
>>> However I think the categorization will be fixed anyway soon, but at
>>> least this phase does not seem to warrant a top level category. (yes,
>>> "Merge PSS" is wrong too, but as mentioned, it will be fixed).
>>>
>>> - since the message is printed every time, but updated only during
>>> initial mark, this will cause confusing output after the first initial
>>> mark. I.e. you will need to initialize its value in
>>> G1GCPhaseTimes::note_gc_start().
>>>
>>> - the member of g1GCPhaseTimes should be consistent with other
>>> messages, i.e. have a "_recorded" prefix and a "_ms" suffix. Same with
>>> the method name and method parameters to record the time.
>>>
>>> - is there a reason there is a standalone test for this and not adding
>>> checking of this message to test/gc/g1/TestGCLogMessages.java as
>>> suggested in the previous reply at the bottom?
>>>
>>> - if there is a reason:
>>>    - there is no need to run the test in "othervm" mode.
>>>    - the test should have a @requires vm.gc.G1 tag.
>>>
>>> Thanks,
>>>    Thomas
>>>
>>>
>>>> On 8/29/2016 7:47 AM, Thomas Schatzl wrote:
>>>>> Hi,
>>>>>
>>>>> On Tue, 2016-08-16 at 14:15 -0400, Joseph Provino wrote:
>>>>>> Please review this very small change.
>>>>>> CR: JDK-8160055 Misplaced call to
>>>>>> ClassLoaderDataGraph::clear_claimed_marks during initial mark
>>>>>> Webrev: http://cr.openjdk.java.net/~jprovino/8160055/webrev.00
>>>>>> Thanks.
>>>>>> joe
>>>>>>
>>>>>     - the recorded time also needs to be printed, in
>>>>> G1GCPhaseTimes::print(), somewhere in the "Other" category. Please
>>>>> only print it if that phase has been executed.
>>>>>
>>>>>     - please try to expand tests/gc/g1/TestGCLogMessages.java to
>>>>> trigger an initial mark (triggering a marking round via whitebox
>>>>> should be sufficient) and check whether this log message appears.
>>>>>
>>>>> Thanks,
>>>>>     Thomas
>>>>>
>>
>