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

[David Holmes]

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" ?

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. :)

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?

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.

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
>>>
>