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

[Max Ockner]

Hey everyone. Patching this into a jake repo was difficult as there were 
other dependencies that had not yet been pulled into jake.  As a result, 
I tabled this until jigsaw integration.

New webrev: http://cr.openjdk.java.net/~mockner/8149996.06/
  - Updated the test to check for new paths.

Tested with jtreg runtime.

Thanks,
Max

On 3/16/2016 5:09 PM, David Holmes wrote:
> Follow ups below ...
>
> On 17/03/2016 1:00 AM, Max Ockner wrote:
>> 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.
>
> Ok.
>
>>> 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.
>
> It was just an observation - we don't pay this much attention elsewhere.
>
>>> 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."
>
> Thanks.
>
>>> 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?
>
> It might stop working next week so yes I think it worth checking this 
> now.
>
> Thanks,
> David
>
>>> 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
>>>>>>
>>>>
>>