RFR: 8149996: TraceLoaderConstraints has been converted to Unified Logging.

[Max Ockner]

Comments below.

On 3/16/2016 2:11 AM, David Holmes wrote:
> Hi Max,
>
> tl;dr: you can push this as-is (modulo fixing copyright years) but I 
> have some further queries/discussion points below. :)
>
> Meta question: should we consider using "classload, constraints" 
> instead of "loaderconstraints" ?
>
I think this would be appropriate.
> On 16/03/2016 1:00 AM, Max Ockner wrote:
>> Hello again,
>>
>> new webrev: http://cr.openjdk.java.net/~mockner/8149996.05/
>>   - added resource marks back in
>>   - reverted to using outputStreams instead of log_info macro
>>
>> I ran into an issue with the outputStream eating its output when I
>> removed " ]\n" from the end of some of the messages, solved by changing
>> "out->print" to "out->print_cr".
>
> Aside: it is actually more efficient to have the \n in the string than 
> to use the print_cr form. :)
>
Thanks for pointing this out. I can change it.
> Nit: is there a reason to remove the initial capitals from the log 
> messages? I find the capitals are a visual aid when trying to find the 
> end of the decorators.
>
> Aside 2: I notice we have now removed the only example of a product_rw 
> flag. I wonder if we should rip out product_rw support? (Separate RFE 
> of course). I'm also wondering if our logging flags are 
> visible/modifiable via the management tools like manageable/product_rw 
> flags are?
>
 From an email discussion with Marcus:
"There is a DCMD to reconfigure logging during runtime. It's all baked 
into a single command called VM.log and it works similar to the -Xlog 
command, but all of the parameters are named, so -Xlog:gc:gclog.txt 
would be VM.log output=gclog.txt what=gc."

> With regard to the test I'm not sure those "loaders" are going to 
> continue to be valid once Jake has integrated. So you might want to 
> try this on a Jake build before finalising it.
>

Do you think this is important to test before submitting?

> Thanks,
> David
>
>> I thought about writing a log_info_rm which evaluates to a code block
>> instead of a function, but I think one of the benefits of the current
>> macros is that they allow two variable argument lists in the same
>> expression. Returning a write function is important for handling the
>> format string and its variable length argument list of format
>> %substitutions.
>>
>> Thanks,
>> Max
>>
>>
>> On 3/9/2016 11:29 PM, David Holmes wrote:
>>> On 10/03/2016 8:03 AM, Coleen Phillimore wrote:
>>>>
>>>> http://cr.openjdk.java.net/~mockner/8149996.02/src/share/vm/classfile/loaderConstraints.cpp.frames.html 
>>>>
>>>>
>>>>
>>>>
>>>> Why did you take out the ResourceMarks at line 130, 160, 231 and 244,
>>>> 297, 308 and 357?
>>>
>>> Yes these should all be of the form:
>>>
>>> if (log_is_enabled(...)) {
>>>   ResourceMark rm;
>>>   // log stuff
>>> }
>>>
>>>> Unfortunately, we really need log_info_rm(tag)("String") call because
>>>> all those places need a ResourceMark.
>>>>
>>>> Anytime we call name()->as_C_string() there needs to be a ResourceMark
>>>> in the scope of the call.
>>>
>>> Which means that a log_info_rm() call wouldn't help because we'd still
>>> be calling as_C_string in the caller. :(
>>>
>>> David
>>>
>>>>
>>>> Coleen
>>>>
>>>> On 3/9/16 3:47 PM, Max Ockner wrote:
>>>>> Hello again,
>>>>>
>>>>> Please review this change. TraceLoaderConstraints has been converted
>>>>> to Unified Logging and can be accessed using
>>>>> '-Xlog:loaderconstraints=info'. TraceLoaderConstraints has been added
>>>>> to the logging alias table.
>>>>>
>>>>> There are 0 lines of output for java -version, and ~10 lines from
>>>>> LoaderConstraintsTest.java
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8149996
>>>>> webrev: http://cr.openjdk.java.net/~mockner/8149996.02/
>>>>>
>>>>> Testing: jtreg runtime. Added new test which triggers logging for
>>>>> loaderconstraints by forcing class unloading. No other references to
>>>>> TraceLoaderConstraints found in hotspot/test, jdk/test , or in the
>>>>> noncolo tests.
>>>>>
>>>>> Thanks,
>>>>> Max
>>>>
>>