Optimized quad tree and cluster algorithm with HashSet instead of ArrayList #368
Conversation
…yList ArrayList.remove() is a much more expensive operation than HashSet.remove(). Neither of these collections need to be an array, as neither order nor duplicate entries are required. A set collection is much more optimal with very noticeable speed improvements with large sets of cluster items.
jeffdgr8
changed the title
|
Huge +1 for this as long as it doesn't affect algorithm correctness. It changes time complexity from O(n) to O(1) for remove() operation (assuming I've run the tests locally and they all pass, and while it's been a while since I've done a deep dive on QuadTrees, I don't believe changing from a I'd like to get another set of eyes on this that's familiar with QuadTrees before merging though. |
|
This looks fine, but I can't recall if iteration order ever matters (use TreeSet instead, then). Does this actually improve performance noticeably, though? MAX_ELEMENTS=50, so you should only be iterating over 50 elements. This might not be true if MAX_DEPTH is reached. |
|
From a quick look through the code it doesn't appear that order matters - only operations used are add and remove with simple iterations though collection. |
|
There's a very noticeable speed improvement in our use case where we have potentially 10,000+ markers on the map at times. When toggling them on/off the ArrayList.remove() operation was the clear bottleneck. |
|
Sad, such a nice PR not merged because emulator couldn't start on travis |
|
@broady @barbeau @stephenmcd @markmcd any chance we can get this as well as my other PRs #337 #339 #378 #380 #381 merged? These along with this PR #217 have been functioning great in our production app for a long time now using my fork. https://github.com/jeffdgr8/android-maps-utils/tree/sales-rabbit It'd be great to have these fixes finally in the main repo. |
|
Thanks a lot |
A clustering performance improvement was made in #368 by switching from using an ArrayList to a HashSet. However, ArrayLists maintain insertion order of the elements when iterating over them later, while HashSets do not. Because the cluster has the center of the first element, that center will change if the iteration order of the elements change, which results in clusters changing position when updating an item model and reclustering. This changes to using a LinkedHashSet, which should give the same constant time performance benefits as the Set for add/remove over the List, but still maintains insertion order. A unit test for insertion order is also added. Fixes #615 Refs: 615
ArrayList.remove() is a much more expensive operation than HashSet.remove(). Neither of these collections need to be an array, as neither order nor duplicate entries are required. A set collection is much more optimal with very noticeable speed improvements with large sets of cluster items.