RFR(s): 8151264: Add a notification mechanism for UL configuration changes

[Robbin Ehn]

Thanks Marcus,

I'll update before push. (no webrev)

/Robbin

On 03/14/2016 09:43 AM, Marcus Larsson wrote:
> Hi,
>
> On 03/11/2016 03:27 PM, Robbin Ehn wrote:
>> Hi, please review this updated changeset.
>>
>> Webrev: http://cr.openjdk.java.net/~rehn/8151264/v2/webrev/
>
> Looks good. Just one comment: Right now listeners are referred to as
> both "config listeners" and "update listeners" in different places.
> Would be good to use only one name for them to prevent confusion.
>
> Thanks,
> Marcus
>
>>
>> Thanks!
>>
>> /Robbin
>>
>> On 03/10/2016 11:38 AM, Robbin Ehn wrote:
>>> Thanks David for looking at this.
>>>
>>> On 03/10/2016 11:19 AM, David Holmes wrote:
>>>> Hi Robbin,
>>>>
>>>> On 9/03/2016 7:02 PM, Robbin Ehn wrote:
>>>>>
>>>>> Hi, please review this minor change.
>>>>
>>>> A notification mechanism is not so minor - what are the programming
>>>> rules for the callbacks? which threads might execute them? Are there
>>>> any
>>>> obvious deadlock/livelock potentials? Do we need to unregister a
>>>> callback?
>>>>
>>>> At a minimum there should be some comments regarding the nature of the
>>>> callback and what it can and can't do.
>>>
>>> Yes, I will document it better.
>>>
>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8151264/
>>>>> Webrev: http://cr.openjdk.java.net/~rehn/8151264/webrev/
>>>>
>>>> Some minor nits:
>>>>
>>>> src/share/vm/logging/log.cpp
>>>>
>>>> Copyright needs 2016 added.
>>>>
>>>> ---
>>>>
>>>> src/share/vm/logging/logConfiguration.cpp
>>>>
>>>> +   assert(cb != NULL, "Should not initialize register NULL as
>>>> listener.");
>>>>
>>>> "initialize register" doesn't make grammatical sense - delete
>>>> "initialize"
>>>>
>>>> +   for (size_t i=0;i<_n_listener_callbacks;i++) {
>>>>
>>>> Style nits: spaces around operators and after ;
>>>>
>>>> ---
>>>>
>>>> src/share/vm/logging/logConfiguration.hpp
>>>>
>>>> +   // After any configuration change this method should be called
>>>> by in
>>>> scope of LogConfigurationLock
>>>>
>>>> Delete 'by' - or may I suggest:
>>>>
>>>> // This should be called after any configuration change while still
>>>> holding ConfigurationLock
>>>>
>>>
>>> Yes, thanks!
>>>
>>> /Robbin
>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> Tested with a new internal vm test (included).
>>>>>
>>>>> Thanks!
>>>>>
>>>>> /Robbin
>