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]>
