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

[Marcus Larsson]

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