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

[Rachel Protacio]

Looks good to me!
Rachel

On 3/28/2016 1:11 PM, Max Ockner wrote:
> 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
>>>>>>>
>>>>>
>>>
>