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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> 
> 
> 
> -- 
> Matt Sicker <[email protected]>

Reply via email to