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
