SegmentNodeList::getSplitCoordinates(): fix quadratic performance pat…

…tern getSplitCoordinates() calls addEdgeCoordinates() repeatdly, which itself calls std::unique() on the whole coordList, thus iterating from the start each time, which is a quadratic performance pattern. As addEdgeCoordinates() is a private method only called by getSplitCoordinates(), move the call to std::unique() to the end of getSplitCoordinates() itself. Issue was detected when observing long runtimes on safe-TestBufferJagged.xml Before this PR: $ /home/even/geos/build_cmake/bin/test_xmltester -v /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test1: buffer(a, 0.35): ok. (11 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test2: buffer(a, 0.75): ok. (155 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test3: buffer(a, 1.01): ok. (209 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test4: buffer(a, 1.1): ok. (210 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test5: buffer(a, 1.5): ok. (310 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test6: buffer(a, 2): ok. (429 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test7: buffer(a, 5): ok. (1326 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test8: buffer(a, 10): ok. (2497 ms) After: $ /home/even/geos/build_cmake/bin/test_xmltester -v /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test1: buffer(a, 0.35): ok. (11 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test2: buffer(a, 0.75): ok. (65 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test3: buffer(a, 1.01): ok. (79 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test4: buffer(a, 1.1): ok. (78 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test5: buffer(a, 1.5): ok. (105 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test6: buffer(a, 2): ok. (133 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test7: buffer(a, 5): ok. (302 ms) /home/even/geos/tests/xmltester/tests/misc/safe-TestBufferJagged.xml: case1: test8: buffer(a, 10): ok. (464 ms) Files: 1 Tests: 8 Failed: 0 Succeeded: 8
libgeos · Jun 16, 2021 · e615f6c · e615f6c
1 parent e98528f
commit e615f6c
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 2 deletions.
diff --git a/NEWS b/NEWS
@@ -8,6 +8,7 @@ Changes in 3.9.2
   - Fix IsValidOp to correctly report invalid nested MultiPolygons (#1112, Martin Davis)
   - Fix BufferOp to avoid artifacts in certain polygon buffers (#1101, Martin Davis)
   - Declare parameterless functions in geos_c.h as func(void) (Paul Ramsey)
+  - Performance improvement in SegmentNodeList (Even Rouault)
 
 
 Changes in 3.9.1

diff --git a/src/noding/SegmentNodeList.cpp b/src/noding/SegmentNodeList.cpp
@@ -301,6 +301,8 @@ SegmentNodeList::getSplitCoordinates()
         addEdgeCoordinates(eiPrev, ei, *coordList);
         eiPrev = ei;
     }
+    // Remove duplicate Coordinates from coordList
+    coordList->erase(std::unique(coordList->begin(), coordList->end()), coordList->end());
     return coordList;
 }
 
@@ -312,8 +314,6 @@ SegmentNodeList::addEdgeCoordinates(const SegmentNode* ei0, const SegmentNode* e
     createSplitEdgePts(ei0, ei1, pts);
     // Append pts to coordList
     coordList.insert(coordList.end(), pts.begin(), pts.end());
-    // Remove duplicate Coordinates from coordList
-    coordList.erase(std::unique(coordList.begin(), coordList.end()), coordList.end());
 }