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

Joseph Provino joseph.provino at oracle.com
Fri Oct 21 15:09:34 UTC 2016


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