Improve performance of UniqueCoordinateFilter #422
Conversation
Signed-off-by: Martin Davis <[email protected]>
Signed-off-by: Martin Davis <[email protected]>
Signed-off-by: Martin Davis <[email protected]>
| TreeSet treeSet = new TreeSet(); | ||
| ArrayList list = new ArrayList(); | ||
| private Set<Coordinate> coordSet = new HashSet<Coordinate>(); | ||
| // Use an auxiliary list as well in order to preserve coordinate order |
There was a problem hiding this comment.
I wonder about LinkedHashSet here?
There was a problem hiding this comment.
Interesting suggestion. But I suspect that using an ArrayList is more efficient, since it is simpler than a doubly linked list?
LinkedHashSet is probably better for situations where the ordering needs to be maintained dymamically, whereas in this use case requires only a single access to the ordered points.
There was a problem hiding this comment.
Yeah, I can imagine the ArrayList coming out ahead too. Hard to say without measuring. Just thought I'd mention it in case it hadn't occurred to you.
There was a problem hiding this comment.
Thanks... good to be reminded of the ever-increasing plethora of useful Java data structures.
There was a problem hiding this comment.
You're right that the ArrayList is faster. Filtering a million-coordinate array ten times takes 12.1s in master, 2.5s with this PR, and 2.8s with the LinkedHashSet.
There was a problem hiding this comment.
Nice! The big win of course is the improvement over master. But the extra few cycles saved with ArrayLIst are nice too.
Use HashSet, and boolean return value from Set.add().
Javadoc improvements
Based on suggestions in #420.