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

[Max Ockner]

OK I will submit this now.  Thanks everybody.
On 3/28/2016 3:01 PM, Coleen Phillimore wrote:
> Max,
> This looks good!
> Thanks,
> Coleen
>
> On 3/28/16 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
>>>>>>>>
>>>>>>
>>>>
>>
>