The only reason to have more than the single vararg version is for
performance. I don't think the marker .add method is going to be called
often so ultra-performance is not necessary. I say don't bother with the
one or two param variant. I just want the varargs version.
On Tue, Apr 22, 2014 at 8:
On Tuesday, 22 April 2014, Bruce Brouwer wrote:
> I don't think performance is a real concern on this method. Style is what
> I am concerned about. My vote would be to change the add method to accept
> varargs.
>
We could include both. Or have one, two, and variable versions.
> On Apr 22, 2014
I don't think performance is a real concern on this method. Style is what I
am concerned about. My vote would be to change the add method to accept
varargs.
On Apr 22, 2014 11:45 AM, "Ralph Goers" wrote:
> I thought about the add method with multiple arguments but thought that it
> sort of goes a
I thought about the add method with multiple arguments but thought that it sort
of goes against the builder pattern. However, I am sure it would perform
better.
Ralph
On Apr 22, 2014, at 5:18 AM, Bruce Brouwer wrote:
> Don't worry about the part of my comment that you were confused about. I
Don't worry about the part of my comment that you were confused about. I
see now that varargs are not slower than array parameters, so long as you
pass in an array.
1) I did find some other things to talk about. This comment:
// It is not strictly necessary to copy the variable here bu
Actually, now I'm confused about that. Bruce, could you take a look at the
current trunk version of MarkerManager?
On 21 April 2014 21:48, Ralph Goers wrote:
> I'm not sure what you are referring to - the parameters? Using a copy of
> a volatile reference is usually faster than using the volat
I'm not sure what you are referring to - the parameters? Using a copy of a
volatile reference is usually faster than using the volatile reference itself.
I think that is what Bruce is referring to.
Ralph
> On Apr 21, 2014, at 7:20 PM, Matt Sicker wrote:
>
> Using Marker... or Marker[] compil
OK, that's what I thought. Thanks for confirming!
On 21 April 2014 21:46, Ralph Goers wrote:
> 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 wrote:
>
> Which brings up ano
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 wrote:
>
> Which brings up another issue with markers. In MarkerManager, we have a
> volatile array of Markers. Here's the message f
Yes, my benchmark showed the same thing, which is why I used the index.
Ralph
> On Apr 21, 2014, at 5:46 PM, Bruce Brouwer 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.
Using Marker... or Marker[] compile to the same bytecode. It should only
affect compile-time stuff unless I'm mistaken.
On 21 April 2014 20:17, Bruce Brouwer wrote:
> Yes, I did. The arrays are faster in 90% of cases. The only time I got the
> HashSet to be faster was when I was caching the ent
Yes, I did. The arrays are faster in 90% of cases. The only time I got the
HashSet to be faster was when I was caching the entire marker hierarchy and
the hierarchy was more than 3 levels deep. That is definitely not the most
common case.
Also, I think the Marker... localParents parameter might be
Also, did you compare the performance of using an array versus a HashSet?
On 21 April 2014 19:44, Matt Sicker wrote:
> Here's what I'm changing that contains method to:
>
> private static boolean contains(final Marker parent, final
> Marker... localParents) {
> //noinspectio
Here's what I'm changing that contains method to:
private static boolean contains(final Marker parent, final
Marker... localParents) {
//noinspection ForLoopReplaceableByForEach
for (int i = 0, localParentsLength = localParents.length; i <
localParentsLength; i++) {
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 access
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 wrote:
> I saw that some small changes were being mad
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 fi
17 matches
Mail list logo
