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 <[email protected]> wrote: > > 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 <[email protected]> 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 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 more performant >> as Marker[] localParents >> >> >>> On Mon, Apr 21, 2014 at 10:07 PM, Matt Sicker <[email protected]> wrote: >>> Also, did you compare the performance of using an array versus a HashSet? >>> >>> >>>> On 21 April 2014 19:44, Matt Sicker <[email protected]> wrote: >>>> 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++) { >>>> final Marker marker = localParents[i]; >>>> if (marker == parent) { >>>> return true; >>>> } >>>> } >>>> return false; >>>> } >>>> >>>> >>>> >>>>> On 21 April 2014 19:42, 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]> >>>> >>>> >>>> >>>> -- >>>> Matt Sicker <[email protected]> >>> >>> >>> >>> -- >>> Matt Sicker <[email protected]> >> >> >> >> -- >> >> Bruce Brouwer > > > > -- > Matt Sicker <[email protected]>
