RFR: 8142979: Add programmatic enable/disable methods to Unified Logging

Wed Dec 23 22:41:25 UTC 2015

Marcus,
I have downloaded your patch and tested it out. I am convinced that it 
works, and it looks good to me.
Thanks,
Max

On 12/18/2015 9:41 AM, Max Ockner wrote:
>  I would like to see a test. The code looks good to me, but I am 
> afraid to sign off on something without knowing that it does what it 
> is supposed to.
>
> On 12/14/2015 10:56 AM, Marcus Larsson wrote:
>>
>> On 2015-12-11 18:44, Max Ockner wrote:
>>> Marcus,
>>> Did you do any testing for this change? I couldn't tell from your 
>>> webrev or email.
>>
>> I did a JPRT run to verify that it compiles everywhere, but didn't 
>> test it further than that. It would be great to have a unit test for 
>> this. Although I'm not sure if I should write it as an InternalVMTest 
>> now, or wait for proper unit test support and write one that will be 
>> integrated together with the other unit tests I have lying around for 
>> UL.
>>
>> Regards,
>> Marcus
>>
>>> Thanks,
>>> Max
>>>
>>> On 12/10/2015 8:15 AM, Marcus Larsson wrote:
>>>>
>>>> On 2015-12-10 11:28, David Holmes wrote:
>>>>> On 10/12/2015 7:12 PM, Marcus Larsson wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 2015-12-10 06:08, David Holmes wrote:
>>>>>>> On 10/12/2015 4:52 AM, Marcus Larsson wrote:
>>>>>>>> Hey Max,
>>>>>>>>
>>>>>>>> On 2015-12-09 18:11, Max Ockner wrote:
>>>>>>>>> Hello Marcus,
>>>>>>>>>
>>>>>>>>> I looked over your code in logConfiguration.cpp
>>>>>>>>>
>>>>>>>>> What is the purpose of limiting the number of tags that can be
>>>>>>>>> configured through a given call to configure_stdout? I 
>>>>>>>>> understand that
>>>>>>>>> previously any change made to the configuration would have 
>>>>>>>>> reset it
>>>>>>>>> entirely, and as a result you could restrict the number of 
>>>>>>>>> tags in the
>>>>>>>>> configuration.  I can't tell if you want to restrict the 
>>>>>>>>> number of
>>>>>>>>> tags in the configuration or the number of tags processed by
>>>>>>>>> configure_stdout, because those restrictions are no longer 
>>>>>>>>> equivalent
>>>>>>>>> now that you can configure_stdout can add to the configuration 
>>>>>>>>> without
>>>>>>>>> clearing it.
>>>>>>>>
>>>>>>>> A call to configure_stdout will configure logging for a particular
>>>>>>>> tagset. It doesn't configure a bunch of tags separately. What I 
>>>>>>>> mean by
>>>>>>>> this is that LogConfigure::configure_stdout(LogLevel::Info, true,
>>>>>>>> LOG_TAGS(tag1, tag2)) is equivalent to the vm argument
>>>>>>>> -Xlog:tag1+tag2=info. It is not the same as 
>>>>>>>> -Xlog:tag1=info,tag2=info
>>>>>>>> (or alternatively -Xlog:tag1=info -Xlog:tag2=info).
>>>>>>>
>>>>>>> Sorry have to ask: what is the difference between these three 
>>>>>>> variants:
>>>>>>>
>>>>>>> -Xlog:tag1+tag2=info
>>>>>>
>>>>>> This will enable messages tagged with tag1 *and* tag2, like for 
>>>>>> example:
>>>>>>
>>>>>> log_info(tag1, tag2)("msg");
>>>>>>
>>>>>> Messages tagged with only one of these tags will not be configured.
>>>>>
>>>>> Ah I see. Where does the end user find out about this?
>>>>
>>>> Currently I think it's only described in the JEP.
>>>>
>>>> It should probably be explained better in the -Xlog:help output.
>>>>
>>>> Regards,
>>>> Marcus
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> -----
>>>>>
>>>>>
>>>>>>> -Xlog:tag1=info,tag2=info
>>>>>>> -Xlog:tag1=info -Xlog:tag2=info
>>>>>>>
>>>>>>> I can't see a description of the + version, but I assume the bototm
>>>>>>> two are equivalent?
>>>>>>
>>>>>> Yes, these two are equivalent now. They enable messages tagged 
>>>>>> either
>>>>>> with tag1 *or* tag2. It doesn't enable the combination of the two.
>>>>>>
>>>>>> Regards,
>>>>>> Marcus
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Marcus
>>>>>>>>
>>>>>>>>>
>>>>>>>>> This question also applies to the handling of multiple Xlog 
>>>>>>>>> options.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Max
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 12/1/2015 3:03 AM, Marcus Larsson wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2015-11-30 19:40, Coleen Phillimore wrote:
>>>>>>>>>>>
>>>>>>>>>>> I'm confused how this API works.
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~mockner/classunload/src/share/vm/prims/jvmtiEnv.cpp.sdiff.html 
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I imagine we'd want to do:
>>>>>>>>>>>
>>>>>>>>>>> if (value != 0) {
>>>>>>>>>>>   LogConfiguration::configure_stdout(info, true, classunload);
>>>>>>>>>>> } else {
>>>>>>>>>>>   LogConfiguration::configure_stdout(off, true, classunload);
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> ???
>>>>>>>>>>
>>>>>>>>>> Yes, except you would need to pass the classunload tag with the
>>>>>>>>>> LOG_TAGS macro, so it'd be:
>>>>>>>>>>
>>>>>>>>>> if (value != 0) {
>>>>>>>>>> LogConfiguration::configure_stdout(LogLevel::Info, true,
>>>>>>>>>> LOG_TAGS(classunload));
>>>>>>>>>> } else {
>>>>>>>>>> LogConfiguration::configure_stdout(LogLevel::Off, true,
>>>>>>>>>> LOG_TAGS(classunload));
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> and that would be the same as passing -Xlog:classunload=info and
>>>>>>>>>> -Xlog:classunload=off
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Marcus
>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Coleen
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 11/25/15 7:13 AM, Marcus Larsson wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> Please review the following patch adding support for 
>>>>>>>>>>>> configuration
>>>>>>>>>>>> of the stdout unified logging output from within the VM 
>>>>>>>>>>>> itself.
>>>>>>>>>>>>
>>>>>>>>>>>> The patch adds the configure_stdout() function, allowing a 
>>>>>>>>>>>> set of
>>>>>>>>>>>> tags to be set to the given level on stdout.
>>>>>>>>>>>> To disable logging the level is simply specified as 
>>>>>>>>>>>> LogLevel::Off.
>>>>>>>>>>>>
>>>>>>>>>>>> The patch depends on the fix for 8142952 [0] that's 
>>>>>>>>>>>> currently out
>>>>>>>>>>>> on review, so the configuration is correctly aggregated 
>>>>>>>>>>>> rather than
>>>>>>>>>>>> reset.
>>>>>>>>>>>>
>>>>>>>>>>>> Webrev:
>>>>>>>>>>>> http://cr.openjdk.java.net/~mlarsson/8142979/webrev.00/
>>>>>>>>>>>>
>>>>>>>>>>>> Issue:
>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8142979
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Marcus
>>>>>>>>>>>>
>>>>>>>>>>>> [0]: https://bugs.openjdk.java.net/browse/JDK-8142952
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>
>>
>