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

Michail Chernov michail.chernov at oracle.com
Fri Oct 21 15:24:33 UTC 2016

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.


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

More information about the hotspot-gc-dev mailing list