You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Matt Sicker <bo...@gmail.com> on 2016/02/25 21:07:45 UTC
Would it be worth it to migrate other CopyOnWriteArrayList usages to
an AppenderControlArraySet-style class?
So far I've found a couple places we use a CopyOnWriteArrayList for
listeners in StatusLogger and core.LoggerContext. I could refactor these
usages to use a custom set-like class like we use for the AppenderControl
set, but I don't know if it's worth it.
--
Matt Sicker <bo...@gmail.com>
Re: Would it be worth it to migrate other CopyOnWriteArrayList usages
to an AppenderControlArraySet-style class?
Posted by Matt Sicker <bo...@gmail.com>.
I started taking a look at the AppenderControlArraySet implementation and
it has a few things pretty specific to AppenderControl objects. Now this
would be way easier with some functional interfaces, but that's a shitty
option in Java 1.7 in a performance-sensitive class I'd imagine. I'm not
sure how to go about extracting some sort of generic class here (and
they're in two separate modules completely as it is) and don't really want
to have two very similar classes like that.
On 26 February 2016 at 16:59, Remko Popma <re...@gmail.com> wrote:
> Ah. I didn't think that through. You are correct.
>
> Sent from my iPhone
>
> On 2016/02/27, at 6:23, Matt Sicker <bo...@gmail.com> wrote:
>
> If you iterate by index instead of through an iterator, I don't think you
> get the copy-on-write semantics.
>
> On 26 February 2016 at 09:02, Matt Sicker <bo...@gmail.com> wrote:
>
>> In that case, the only thing that sounds like it might be useful would be
>> updating StatusLogger (unless the GC-free path requires disabling log4j's
>> logging).
>>
>> On 25 February 2016 at 16:56, Remko Popma <re...@gmail.com> wrote:
>>
>>> If it's a CopyOnWriteArray*List* we can iterate by index, instead of the
>>> for-each loop style. However, it may be good to focus on steady state
>>> application logging. For things only used during initialization there may
>>> not be much benefit. The test to detect memory allocation would be really
>>> useful, but I haven't had time to look at this. (If anyone wants to work on
>>> that that'd be great.)
>>>
>>> One thing: currently StatusLogger creates special ParameterizedMessages
>>> without a reference to the original parameters to prevent memory leaks. Now
>>> that the formatting logic is separated out to ParameterFormatter, that can
>>> be optimized to an ObjectMessage (holding a StringBuilder?). The
>>> specialized ParameterizedMessage subclass can then be removed which is a
>>> nice simplification.
>>>
>>> Sent from my iPhone
>>>
>>> On 2016/02/26, at 5:46, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>> * StatusLogger (StatusListener list)
>>> * LoggerContext (PropertyChangeListener list)
>>> * AbstractConfiguration (ConfigurationListener list)
>>> * PluginManager (String list of packages to scan)
>>> * DefaultShutdownCallbackRegistry (Cancellable list)
>>> * And some irrelevant usages in log4j-perf (they're intentionally using
>>> CopyOnWriteArrayList/Set)
>>>
>>> On 25 February 2016 at 14:18, Ralph Goers <ra...@dslextreme.com>
>>> wrote:
>>>
>>>> Do you have a list?
>>>>
>>>> Ralph
>>>>
>>>> On Feb 25, 2016, at 1:07 PM, Matt Sicker <bo...@gmail.com> wrote:
>>>>
>>>> So far I've found a couple places we use a CopyOnWriteArrayList for
>>>> listeners in StatusLogger and core.LoggerContext. I could refactor these
>>>> usages to use a custom set-like class like we use for the AppenderControl
>>>> set, but I don't know if it's worth it.
>>>>
>>>> --
>>>> Matt Sicker <bo...@gmail.com>
>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: Would it be worth it to migrate other CopyOnWriteArrayList usages to an AppenderControlArraySet-style class?
Posted by Remko Popma <re...@gmail.com>.
Ah. I didn't think that through. You are correct.
Sent from my iPhone
> On 2016/02/27, at 6:23, Matt Sicker <bo...@gmail.com> wrote:
>
> If you iterate by index instead of through an iterator, I don't think you get the copy-on-write semantics.
>
>> On 26 February 2016 at 09:02, Matt Sicker <bo...@gmail.com> wrote:
>> In that case, the only thing that sounds like it might be useful would be updating StatusLogger (unless the GC-free path requires disabling log4j's logging).
>>
>>> On 25 February 2016 at 16:56, Remko Popma <re...@gmail.com> wrote:
>>> If it's a CopyOnWriteArray*List* we can iterate by index, instead of the for-each loop style. However, it may be good to focus on steady state application logging. For things only used during initialization there may not be much benefit. The test to detect memory allocation would be really useful, but I haven't had time to look at this. (If anyone wants to work on that that'd be great.)
>>>
>>> One thing: currently StatusLogger creates special ParameterizedMessages without a reference to the original parameters to prevent memory leaks. Now that the formatting logic is separated out to ParameterFormatter, that can be optimized to an ObjectMessage (holding a StringBuilder?). The specialized ParameterizedMessage subclass can then be removed which is a nice simplification.
>>>
>>> Sent from my iPhone
>>>
>>>> On 2016/02/26, at 5:46, Matt Sicker <bo...@gmail.com> wrote:
>>>>
>>>> * StatusLogger (StatusListener list)
>>>> * LoggerContext (PropertyChangeListener list)
>>>> * AbstractConfiguration (ConfigurationListener list)
>>>> * PluginManager (String list of packages to scan)
>>>> * DefaultShutdownCallbackRegistry (Cancellable list)
>>>> * And some irrelevant usages in log4j-perf (they're intentionally using CopyOnWriteArrayList/Set)
>>>>
>>>>> On 25 February 2016 at 14:18, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>> Do you have a list?
>>>>>
>>>>> Ralph
>>>>>
>>>>>> On Feb 25, 2016, at 1:07 PM, Matt Sicker <bo...@gmail.com> wrote:
>>>>>>
>>>>>> So far I've found a couple places we use a CopyOnWriteArrayList for listeners in StatusLogger and core.LoggerContext. I could refactor these usages to use a custom set-like class like we use for the AppenderControl set, but I don't know if it's worth it.
>>>>>>
>>>>>> --
>>>>>> Matt Sicker <bo...@gmail.com>
>>>>
>>>>
>>>>
>>>> --
>>>> Matt Sicker <bo...@gmail.com>
>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
Re: Would it be worth it to migrate other CopyOnWriteArrayList usages
to an AppenderControlArraySet-style class?
Posted by Matt Sicker <bo...@gmail.com>.
If you iterate by index instead of through an iterator, I don't think you
get the copy-on-write semantics.
On 26 February 2016 at 09:02, Matt Sicker <bo...@gmail.com> wrote:
> In that case, the only thing that sounds like it might be useful would be
> updating StatusLogger (unless the GC-free path requires disabling log4j's
> logging).
>
> On 25 February 2016 at 16:56, Remko Popma <re...@gmail.com> wrote:
>
>> If it's a CopyOnWriteArray*List* we can iterate by index, instead of the
>> for-each loop style. However, it may be good to focus on steady state
>> application logging. For things only used during initialization there may
>> not be much benefit. The test to detect memory allocation would be really
>> useful, but I haven't had time to look at this. (If anyone wants to work on
>> that that'd be great.)
>>
>> One thing: currently StatusLogger creates special ParameterizedMessages
>> without a reference to the original parameters to prevent memory leaks. Now
>> that the formatting logic is separated out to ParameterFormatter, that can
>> be optimized to an ObjectMessage (holding a StringBuilder?). The
>> specialized ParameterizedMessage subclass can then be removed which is a
>> nice simplification.
>>
>> Sent from my iPhone
>>
>> On 2016/02/26, at 5:46, Matt Sicker <bo...@gmail.com> wrote:
>>
>> * StatusLogger (StatusListener list)
>> * LoggerContext (PropertyChangeListener list)
>> * AbstractConfiguration (ConfigurationListener list)
>> * PluginManager (String list of packages to scan)
>> * DefaultShutdownCallbackRegistry (Cancellable list)
>> * And some irrelevant usages in log4j-perf (they're intentionally using
>> CopyOnWriteArrayList/Set)
>>
>> On 25 February 2016 at 14:18, Ralph Goers <ra...@dslextreme.com>
>> wrote:
>>
>>> Do you have a list?
>>>
>>> Ralph
>>>
>>> On Feb 25, 2016, at 1:07 PM, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>> So far I've found a couple places we use a CopyOnWriteArrayList for
>>> listeners in StatusLogger and core.LoggerContext. I could refactor these
>>> usages to use a custom set-like class like we use for the AppenderControl
>>> set, but I don't know if it's worth it.
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>>
>>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
--
Matt Sicker <bo...@gmail.com>
Re: Would it be worth it to migrate other CopyOnWriteArrayList usages
to an AppenderControlArraySet-style class?
Posted by Matt Sicker <bo...@gmail.com>.
In that case, the only thing that sounds like it might be useful would be
updating StatusLogger (unless the GC-free path requires disabling log4j's
logging).
On 25 February 2016 at 16:56, Remko Popma <re...@gmail.com> wrote:
> If it's a CopyOnWriteArray*List* we can iterate by index, instead of the
> for-each loop style. However, it may be good to focus on steady state
> application logging. For things only used during initialization there may
> not be much benefit. The test to detect memory allocation would be really
> useful, but I haven't had time to look at this. (If anyone wants to work on
> that that'd be great.)
>
> One thing: currently StatusLogger creates special ParameterizedMessages
> without a reference to the original parameters to prevent memory leaks. Now
> that the formatting logic is separated out to ParameterFormatter, that can
> be optimized to an ObjectMessage (holding a StringBuilder?). The
> specialized ParameterizedMessage subclass can then be removed which is a
> nice simplification.
>
> Sent from my iPhone
>
> On 2016/02/26, at 5:46, Matt Sicker <bo...@gmail.com> wrote:
>
> * StatusLogger (StatusListener list)
> * LoggerContext (PropertyChangeListener list)
> * AbstractConfiguration (ConfigurationListener list)
> * PluginManager (String list of packages to scan)
> * DefaultShutdownCallbackRegistry (Cancellable list)
> * And some irrelevant usages in log4j-perf (they're intentionally using
> CopyOnWriteArrayList/Set)
>
> On 25 February 2016 at 14:18, Ralph Goers <ra...@dslextreme.com>
> wrote:
>
>> Do you have a list?
>>
>> Ralph
>>
>> On Feb 25, 2016, at 1:07 PM, Matt Sicker <bo...@gmail.com> wrote:
>>
>> So far I've found a couple places we use a CopyOnWriteArrayList for
>> listeners in StatusLogger and core.LoggerContext. I could refactor these
>> usages to use a custom set-like class like we use for the AppenderControl
>> set, but I don't know if it's worth it.
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: Would it be worth it to migrate other CopyOnWriteArrayList usages to an AppenderControlArraySet-style class?
Posted by Remko Popma <re...@gmail.com>.
If it's a CopyOnWriteArray*List* we can iterate by index, instead of the for-each loop style. However, it may be good to focus on steady state application logging. For things only used during initialization there may not be much benefit. The test to detect memory allocation would be really useful, but I haven't had time to look at this. (If anyone wants to work on that that'd be great.)
One thing: currently StatusLogger creates special ParameterizedMessages without a reference to the original parameters to prevent memory leaks. Now that the formatting logic is separated out to ParameterFormatter, that can be optimized to an ObjectMessage (holding a StringBuilder?). The specialized ParameterizedMessage subclass can then be removed which is a nice simplification.
Sent from my iPhone
> On 2016/02/26, at 5:46, Matt Sicker <bo...@gmail.com> wrote:
>
> * StatusLogger (StatusListener list)
> * LoggerContext (PropertyChangeListener list)
> * AbstractConfiguration (ConfigurationListener list)
> * PluginManager (String list of packages to scan)
> * DefaultShutdownCallbackRegistry (Cancellable list)
> * And some irrelevant usages in log4j-perf (they're intentionally using CopyOnWriteArrayList/Set)
>
>> On 25 February 2016 at 14:18, Ralph Goers <ra...@dslextreme.com> wrote:
>> Do you have a list?
>>
>> Ralph
>>
>>> On Feb 25, 2016, at 1:07 PM, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>> So far I've found a couple places we use a CopyOnWriteArrayList for listeners in StatusLogger and core.LoggerContext. I could refactor these usages to use a custom set-like class like we use for the AppenderControl set, but I don't know if it's worth it.
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
Re: Would it be worth it to migrate other CopyOnWriteArrayList usages
to an AppenderControlArraySet-style class?
Posted by Matt Sicker <bo...@gmail.com>.
* StatusLogger (StatusListener list)
* LoggerContext (PropertyChangeListener list)
* AbstractConfiguration (ConfigurationListener list)
* PluginManager (String list of packages to scan)
* DefaultShutdownCallbackRegistry (Cancellable list)
* And some irrelevant usages in log4j-perf (they're intentionally using
CopyOnWriteArrayList/Set)
On 25 February 2016 at 14:18, Ralph Goers <ra...@dslextreme.com>
wrote:
> Do you have a list?
>
> Ralph
>
> On Feb 25, 2016, at 1:07 PM, Matt Sicker <bo...@gmail.com> wrote:
>
> So far I've found a couple places we use a CopyOnWriteArrayList for
> listeners in StatusLogger and core.LoggerContext. I could refactor these
> usages to use a custom set-like class like we use for the AppenderControl
> set, but I don't know if it's worth it.
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: Would it be worth it to migrate other CopyOnWriteArrayList usages to an AppenderControlArraySet-style class?
Posted by Ralph Goers <ra...@dslextreme.com>.
Do you have a list?
Ralph
> On Feb 25, 2016, at 1:07 PM, Matt Sicker <bo...@gmail.com> wrote:
>
> So far I've found a couple places we use a CopyOnWriteArrayList for listeners in StatusLogger and core.LoggerContext. I could refactor these usages to use a custom set-like class like we use for the AppenderControl set, but I don't know if it's worth it.
>
> --
> Matt Sicker <boards@gmail.com <ma...@gmail.com>>