RFR(s): 8151264: Add a notification mechanism for UL configuration changes
Robbin Ehn
robbin.ehn at oracle.com
Fri Mar 11 14:27:56 UTC 2016
Hi, please review this updated changeset.
Webrev: http://cr.openjdk.java.net/~rehn/8151264/v2/webrev/
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
More information about the hotspot-runtime-dev
mailing list