RFR: 8142976: TraceClassInitialization has been reimplemented with Unified Logging.

Tue Dec 1 00:12:52 UTC 2015

A little bit more from the e-mail that announced the feature:

On 10/24/14 1:07 PM, Jonathan Gibbons wrote:

 > Subject: Re: jtreg 4.1 fcs b10 has been promoted

<snip>

 > * New tag @run driver
 >     Like @run main except that the class is not executed with any VM
 >     options specified on the command line. Recommended for use when
 >     the class is "like a shell script, but written in Java", meaning
 >     that the class does not contain the test: instead, it executes
 >     the class that is the test.

To me the suppression of VM options from the Java "driver" code is
the important part of the feature and that just doesn't come across
clearly in the tag-spec.html text (IMHO).

Dan

On 11/30/15 3:56 PM, George Triantafillou wrote:
> Hi Max,
>
> From http://openjdk.java.net/jtreg/tag-spec.html
>
> driver[/fail][/timeout=<seconds>]<class><arg>*
>
>    Invoke the main method of the specified class, passing any arguments
>    after the class name. Although superficially similar to|@run main|,
>    this is for use when it is desirable to perform additional setup or
>    configuration before running the class containing the actual test
>    code, possibly in a child VM.
>
>    The named<class>will be compiled on demand, just as though an
>    "|@run|build<class>" action had been inserted before this action. If
>    this action requires classes other than<class>to be up to date,
>    insert an appropriate build action before this action.
>
>    -George
>
>
> On 11/30/2015 5:46 PM, Max Ockner wrote:
>> I do not know what "driver" means in the test,  but I have found that 
>> it is important. The test does not work without it. Does anyone know 
>> what it means and why it is suddenly important?
>>
>> Max
>>
>> On 11/24/2015 7:17 PM, Coleen Phillimore wrote:
>>>
>>> Hi Max,  This looks mostly good:
>>>
>>> http://cr.openjdk.java.net/~mockner/ulclassinit02/src/share/vm/classfile/verifier.cpp.udiff.html 
>>>
>>>
>>> In function log_end_verification, the indentation is wrong. It 
>>> should be 2 and the end } should be in column 1.
>>>
>>> You didn't mention that we decided that if you specify 
>>> VerboseVerification *and* -Xlog:classinit that you'll get the same 
>>> message twice. This is because the logs can go to different places. 
>>> When VerboseVerification is converted to UL, the logging statements 
>>> will be more compact.
>>>
>>> Can you fix the indentation of this line too?
>>>
>>> *!tty->print_cr("Fail over class verification to old verifier for: 
>>> %s", klassName);*
>>>
>>>
>>> http://cr.openjdk.java.net/~mockner/ulclassinit02/test/logging/BadMap50.jasm.html 
>>>
>>>
>>> This has the wrong copyright header.
>>>
>>> http://cr.openjdk.java.net/~mockner/ulclassinit02/test/logging/ClassInitializationTest.java.html 
>>>
>>>
>>>   30  * @run driver ClassInitializationTest
>>>
>>>
>>> Is "driver" new?
>>>
>>> You don't need to include these:
>>>
>>>   34 import java.lang.ref.WeakReference;
>>>   35 import java.lang.reflect.Method;
>>>
>>>
>>> Otherwise, I think this looks good.
>>>
>>> Coleen
>>>
>>> On 11/24/15 4:27 PM, Max Ockner wrote:
>>>> New webrev @ http://cr.openjdk.java.net/~mockner/ulclassinit02/
>>>> Fixed everything that I said I would fix below.
>>>>
>>>> On 11/24/2015 3:40 PM, Rachel Protacio wrote:
>>>>> Hi,
>>>>>
>>>>> Looks mostly good, just a few comments:
>>>>>
>>>>> verifier.cpp
>>>>> - at line 118 (the first "Verification for" line), it should just 
>>>>> be "print", not "print_cr".
>>>> Thanks. Fixed, though I wonder how much it matters.
>>>>> - between lines 194 and 195, I think you need a ResourceMark for 
>>>>> the LogHandle stream.
>>>> The ResourceMark is defined already, it just isn't part of the 
>>>> diffs because it was already there.
>>>>> - in the sections starting at lines 179 and 608, I appreciate that 
>>>>> you were minimizing the number of lines, but I think it's a bad 
>>>>> idea to have duplicates of the logged strings. Do you think you 
>>>>> could define the strings outside of the logging and pass it to 
>>>>> both functions? Another possible solution would be to make a 
>>>>> function to do that with a signature like
>>>>>     void log_multiple(bool enabled1, outputStream* st1, bool 
>>>>> enabled2, outputStream* st2, char* msg);
>>>>> that could do this in a more formalized manner. A function like 
>>>>> this could be useful for other similar situations as well while 
>>>>> we're converting flags one by one. Or what are your thoughts on that?
>>>> Two reasons why I don't think we should do that.
>>>>     (1) We don't want to evaluate format strings unless something 
>>>> is being logged. I guess if you can find a way to avoid doing this 
>>>> while still making the code look nicer then that is OK. I think it 
>>>> would be just as bad to write extra lines of code just to ensure 
>>>> that a short string isn't duplicated.
>>>>     (2) We do not guarantee that these two messages will always be 
>>>> the same. A conversation I had with Coleen led me to believe we 
>>>> should keep the messages separate.
>>>>> - I think the reordered nesting makes sense.
>>>>>
>>>>> ClassInitializationTest.java
>>>>> - nit: can you move the ");"s from the process builder lines onto 
>>>>> the lines before them?
>>>> OK.
>>>>>
>>>>> Thanks,
>>>>> Rachel
>>>>>
>>>>> On 11/24/2015 3:09 PM, Max Ockner wrote:
>>>>>> Hello,
>>>>>> Please review my new unified logging code:
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8142976
>>>>>> http://cr.openjdk.java.net/~mockner/ulclassinit01/src/share/vm/classfile/verifier.cpp.cdiff.html 
>>>>>>
>>>>>> Summary: -XX:+TraceClassInitialization logging has been 
>>>>>> reimplemented using unified logging under the classinit tag.
>>>>>>
>>>>>> In the segment with recursive verification (see verifier.cpp) I 
>>>>>> reordered the nested if statement to check 
>>>>>> was_recursively_verified() first. I valued clean code over 
>>>>>> potentially avoiding a function call to was_recursively_verified.
>>>>>>
>>>>>> What do you think?
>>>>>>
>>>>>> Tested with:
>>>>>> jtreg hotspot tests
>>>>>> new jtreg test for classinit tag
>>>>>> performance testing with refworkload.
>>>>>>
>>>>>> Thanks, Max
>>>>>
>>>>
>>>
>>
>
>