logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ralph Goers <rgo...@apache.org>
Subject Re: Some things I noticed with the Marker implementation
Date Tue, 22 Apr 2014 03:46:12 GMT
We only modify the array reference. A new array is created whenever it is modified so volatile
is correct here.

Ralph

> On Apr 21, 2014, at 6:42 PM, Matt Sicker <boards@gmail.com> wrote:
> 
> Which brings up another issue with markers. In MarkerManager, we have a volatile array
of Markers. Here's the message from IntelliJ:
> 
> Reports array fields which are declared as volatile. Such fields may be confusing, as
accessing the array itself follows the rules for volatile fields, but accessing the array's
contents does not. If such volatile access is needed to array contents, the JDK5.0 java.util.concurrent.atomic
classes should be used instead.
> 
> Is this relevant here?
> 
> 
>> On 21 April 2014 19:37, Matt Sicker <boards@gmail.com> wrote:
>> 1) that would be my bad. I usually replace those with foreach loops where possible.
It's usually good to comment in those cases. I'll revert that and comment.
>> 
>> 2) that makes more sense than the exception
>> 
>> 
>>> On 21 April 2014 18:46, Bruce Brouwer <bruce.brouwer@gmail.com> wrote:
>>> I saw that some small changes were being made to the Markers. I had a few thoughts
regarding them:
>>> 
>>> 1) Use of array iterator instead of indexed for loop. 
>>> for (Marker marker : localParents) 
>>> instead of 
>>> for (int i = 0; i < localParents.length; i++) 
>>> 
>>> When I was doing my performance benchmarks, I was finding the latter to be faster.
I'm guessing this is simply because a new Iterable object needs to be created to iterate over
the array.
>>> 
>>> For most methods, such as add, remove, this was not a big deal. But for the isInstanceOf
and checkParent methods, we want those to be as fast as possible. 
>>> 
>>> 2) isInstanceOf(String markerName)
>>> Instead of throwing an IllegalArgumentException when a marker of name markerName
doesn't exist, why don't we simply return false? I don't want an IllegalArgumentException
to happen because I'm testing a markerName. 
>>> 
>>> 
>>> 
>>> -- 
>>> 
>>> Bruce Brouwer
>> 
>> 
>> 
>> -- 
>> Matt Sicker <boards@gmail.com>
> 
> 
> 
> -- 
> Matt Sicker <boards@gmail.com>

Mime
View raw message