[geos-commits] [SCM] GEOS branch master updated. d4a14d2dcd7d7eda13a9caf686725fd0af579b4a

git at osgeo.org git at osgeo.org
Wed Dec 23 08:14:53 PST 2020


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GEOS".

The branch, master has been updated
       via  d4a14d2dcd7d7eda13a9caf686725fd0af579b4a (commit)
      from  e0cc6ad090738fce5e954c24073f85c0d43d9aaa (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit d4a14d2dcd7d7eda13a9caf686725fd0af579b4a
Author: Paul Ramsey <pramsey at cleverelephant.ca>
Date:   Wed Dec 23 08:14:48 2020 -0800

    Change API to MonotoneChainBuilder and lifecycle of vectors of MonotoneChain to avoid heap allocations and promote memory locality. 10% performance win on large overlay.

diff --git a/include/geos/index/chain/MonotoneChain.h b/include/geos/index/chain/MonotoneChain.h
index 7e29509..531de3f 100644
--- a/include/geos/index/chain/MonotoneChain.h
+++ b/include/geos/index/chain/MonotoneChain.h
@@ -141,18 +141,6 @@ public:
     void computeOverlaps(MonotoneChain* mc, double overlapTolerance,
                          MonotoneChainOverlapAction* mco);
 
-    void
-    setId(int nId)
-    {
-        id = nId;
-    }
-
-    inline int
-    getId() const
-    {
-        return id;
-    }
-
     void*
     getContext()
     {
@@ -193,15 +181,11 @@ private:
 
     /// Owned by this class
     geom::Envelope env;
-    bool envIsSet;
-
-    /// useful for optimizing chain comparisons
-    int id;
-
 
     // Declare type as noncopyable
-    MonotoneChain(const MonotoneChain& other) = delete;
-    MonotoneChain& operator=(const MonotoneChain& rhs) = delete;
+    // XXXXXX uncomment for MSVC support
+    // MonotoneChain(const MonotoneChain& other) = delete;
+    // MonotoneChain& operator=(const MonotoneChain& rhs) = delete;
 };
 
 } // namespace geos::index::chain
diff --git a/include/geos/index/chain/MonotoneChainBuilder.h b/include/geos/index/chain/MonotoneChainBuilder.h
index a9b2739..2d4bb29 100644
--- a/include/geos/index/chain/MonotoneChainBuilder.h
+++ b/include/geos/index/chain/MonotoneChainBuilder.h
@@ -53,14 +53,6 @@ public:
     MonotoneChainBuilder() {}
 
     /** \brief
-     * Return a newly-allocated vector of newly-allocated
-     * MonotoneChain objects for the given CoordinateSequence.
-     */
-    static std::unique_ptr<std::vector<std::unique_ptr<MonotoneChain>>> getChains(
-        const geom::CoordinateSequence* pts,
-        void* context);
-
-    /** \brief
      * Computes a list of the {@link MonotoneChain}s for a list of coordinates,
      * attaching a context data object to each.
      *
@@ -70,13 +62,7 @@ public:
      */
     static void getChains(const geom::CoordinateSequence* pts,
                           void* context,
-                          std::vector<std::unique_ptr<MonotoneChain>>& mcList);
-
-    static std::unique_ptr<std::vector<std::unique_ptr<MonotoneChain>>>
-    getChains(const geom::CoordinateSequence* pts)
-    {
-        return getChains(pts, nullptr);
-    }
+                          std::vector<MonotoneChain>& mcList);
 
     /**
      * Disable copy construction and assignment. Apparently needed to make this
diff --git a/include/geos/noding/MCIndexNoder.h b/include/geos/noding/MCIndexNoder.h
index f5b450f..0b8ef59 100644
--- a/include/geos/noding/MCIndexNoder.h
+++ b/include/geos/noding/MCIndexNoder.h
@@ -24,6 +24,7 @@
 #include <geos/inline.h>
 
 #include <geos/index/chain/MonotoneChainOverlapAction.h> // for inheritance
+#include <geos/index/chain/MonotoneChain.h>
 #include <geos/noding/SinglePassNoder.h> // for inheritance
 #include <geos/index/strtree/SimpleSTRtree.h> // for composition
 #include <geos/util.h>
@@ -65,13 +66,13 @@ namespace noding { // geos.noding
 class GEOS_DLL MCIndexNoder : public SinglePassNoder {
 
 private:
-    std::vector<index::chain::MonotoneChain*> monoChains;
+    std::vector<index::chain::MonotoneChain> monoChains;
     index::strtree::SimpleSTRtree index;
-    int idCounter;
     std::vector<SegmentString*>* nodedSegStrings;
     // statistics
     int nOverlaps;
     double overlapTolerance;
+    bool indexBuilt;
 
     void intersectChains();
 
@@ -80,18 +81,18 @@ private:
 public:
 
     MCIndexNoder(SegmentIntersector* nSegInt = nullptr, double p_overlapTolerance = 0.0)
-        :
-        SinglePassNoder(nSegInt),
-        idCounter(0),
-        nodedSegStrings(nullptr),
-        nOverlaps(0),
-        overlapTolerance(p_overlapTolerance)
+        : SinglePassNoder(nSegInt)
+        , nodedSegStrings(nullptr)
+        , nOverlaps(0)
+        , overlapTolerance(p_overlapTolerance)
+        , indexBuilt(false)
     {}
 
-    ~MCIndexNoder() override;
+    ~MCIndexNoder() override {};
+
 
     /// \brief Return a reference to this instance's std::vector of MonotoneChains
-    std::vector<index::chain::MonotoneChain*>&
+    std::vector<index::chain::MonotoneChain>&
     getMonotoneChains()
     {
         return monoChains;
diff --git a/include/geos/noding/MCIndexSegmentSetMutualIntersector.h b/include/geos/noding/MCIndexSegmentSetMutualIntersector.h
index 888e837..da3a9c0 100644
--- a/include/geos/noding/MCIndexSegmentSetMutualIntersector.h
+++ b/include/geos/noding/MCIndexSegmentSetMutualIntersector.h
@@ -22,13 +22,14 @@
 
 #include <geos/noding/SegmentSetMutualIntersector.h> // inherited
 #include <geos/index/chain/MonotoneChainOverlapAction.h> // inherited
+#include <geos/index/chain/MonotoneChain.h> // inherited
+#include <geos/index/strtree/SimpleSTRtree.h> // inherited
 
 namespace geos {
 namespace index {
 class SpatialIndex;
 
 namespace chain {
-class MonotoneChain;
 }
 namespace strtree {
 //class STRtree;
@@ -55,14 +56,22 @@ namespace noding { // geos::noding
 class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector {
 public:
 
-    MCIndexSegmentSetMutualIntersector();
+    MCIndexSegmentSetMutualIntersector()
+        : monoChains()
+        , index(new index::strtree::SimpleSTRtree())
+        , indexCounter(0)
+        , processCounter(0)
+        , nOverlaps(0)
+        , indexBuilt(false)
+    {}
 
-    ~MCIndexSegmentSetMutualIntersector() override;
+    ~MCIndexSegmentSetMutualIntersector() override
+    {};
 
     index::SpatialIndex*
     getIndex()
     {
-        return index;
+        return index.get();
     }
 
     void setBaseSegments(SegmentString::ConstVect* segStrings) override;
@@ -96,7 +105,7 @@ public:
 
 private:
 
-    typedef std::vector<std::unique_ptr<index::chain::MonotoneChain>> MonoChains;
+    typedef std::vector<index::chain::MonotoneChain> MonoChains;
     MonoChains monoChains;
 
     /*
@@ -104,7 +113,7 @@ private:
      * envelope (range) queries efficiently (such as a index::quadtree::Quadtree
      * or index::strtree::STRtree).
      */
-    index::SpatialIndex* index;
+    std::unique_ptr<index::SpatialIndex> index;
     int indexCounter;
     int processCounter;
     // statistics
@@ -113,7 +122,8 @@ private:
     /* memory management helper, holds MonotoneChain objects used
      * in the SpatialIndex. It's cleared when the SpatialIndex is
      */
-    MonoChains chainStore;
+    bool indexBuilt;
+    MonoChains indexChains;
 
     void addToIndex(SegmentString* segStr);
 
diff --git a/src/index/chain/MonotoneChain.cpp b/src/index/chain/MonotoneChain.cpp
index d0e3470..cdd155d 100644
--- a/src/index/chain/MonotoneChain.cpp
+++ b/src/index/chain/MonotoneChain.cpp
@@ -33,16 +33,12 @@ namespace chain { // geos.index.chain
 
 MonotoneChain::MonotoneChain(const geom::CoordinateSequence& newPts,
                              std::size_t nstart, std::size_t nend, void* nContext)
-    :
-    pts(newPts),
-    context(nContext),
-    start(nstart),
-    end(nend),
-    env(newPts[nstart], newPts[nend]),
-    envIsSet(false),
-    id(-1)
-{
-}
+    : pts(newPts)
+    , context(nContext)
+    , start(nstart)
+    , end(nend)
+    , env()
+{}
 
 const Envelope&
 MonotoneChain::getEnvelope()
@@ -53,12 +49,11 @@ MonotoneChain::getEnvelope()
 const Envelope&
 MonotoneChain::getEnvelope(double expansionDistance)
 {
-    if (!envIsSet) {
+    if (env.isNull()) {
         env.init(pts[start], pts[end]);
         if (expansionDistance > 0.0) {
             env.expandBy(expansionDistance);
         }
-        envIsSet = true;
     }
     return env;
 }
diff --git a/src/index/chain/MonotoneChainBuilder.cpp b/src/index/chain/MonotoneChainBuilder.cpp
index a87fa74..a23b2b4 100644
--- a/src/index/chain/MonotoneChainBuilder.cpp
+++ b/src/index/chain/MonotoneChainBuilder.cpp
@@ -40,26 +40,16 @@ namespace geos {
 namespace index { // geos.index
 namespace chain { // geos.index.chain
 
-/* static public */
-std::unique_ptr<std::vector<std::unique_ptr<MonotoneChain>>>
-MonotoneChainBuilder::getChains(const CoordinateSequence* pts, void* context)
-{
-    // TODO clean this up with std::make_unique (C++14)
-    std::unique_ptr<std::vector<std::unique_ptr<MonotoneChain>>> mcList{new std::vector<std::unique_ptr<MonotoneChain>>()};
-    getChains(pts, context, *mcList);
-    return mcList;
-}
 
 /* static public */
 void
 MonotoneChainBuilder::getChains(const CoordinateSequence* pts, void* context,
-                                std::vector<std::unique_ptr<MonotoneChain>>& mcList)
+                                std::vector<MonotoneChain>& mcList)
 {
     std::size_t chainStart = 0;
     do {
         std::size_t chainEnd = findChainEnd(*pts, chainStart);
-        MonotoneChain *mc = new MonotoneChain(*pts, chainStart, chainEnd, context);
-        mcList.emplace_back(mc);
+        mcList.emplace_back(*pts, chainStart, chainEnd, context);
         chainStart = chainEnd;
     }
     while (chainStart < (pts->size() - 1));
diff --git a/src/noding/MCIndexNoder.cpp b/src/noding/MCIndexNoder.cpp
index 7db17b0..acb0bbc 100644
--- a/src/noding/MCIndexNoder.cpp
+++ b/src/noding/MCIndexNoder.cpp
@@ -53,10 +53,17 @@ MCIndexNoder::computeNodes(SegmentString::NonConstVect* inputSegStrings)
         add(s);
     }
 
+    if (!indexBuilt) {
+        for(auto& mc : monoChains) {
+            index.insert(&(mc.getEnvelope(overlapTolerance)), &mc);
+        }
+        indexBuilt = true;
+    }
+
     intersectChains();
-//cerr<<"MCIndexNoder: # chain overlaps = "<<nOverlaps<<endl;
 }
 
+
 /*private*/
 void
 MCIndexNoder::intersectChains()
@@ -66,12 +73,11 @@ MCIndexNoder::intersectChains()
     SegmentOverlapAction overlapAction(*segInt);
 
     std::vector<void*> overlapChains;
-    for(MonotoneChain* queryChain : monoChains) {
+    for(MonotoneChain& queryChain : monoChains) {
         GEOS_CHECK_FOR_INTERRUPTS();
 
-        assert(queryChain);
         overlapChains.clear();
-        const geom::Envelope& queryEnv = queryChain->getEnvelope(overlapTolerance);
+        const geom::Envelope& queryEnv = queryChain.getEnvelope(overlapTolerance);
         index.query(&queryEnv, overlapChains);
         for(void* hit : overlapChains) {
             MonotoneChain* testChain = static_cast<MonotoneChain*>(hit);
@@ -82,8 +88,8 @@ MCIndexNoder::intersectChains()
              * pair of chains once and that we don't compare a
              * chain to itself
              */
-            if(testChain->getId() > queryChain->getId()) {
-                queryChain->computeOverlaps(testChain, overlapTolerance, &overlapAction);
+            if(testChain > &queryChain) {
+                queryChain.computeOverlaps(testChain, overlapTolerance, &overlapAction);
                 nOverlaps++;
             }
 
@@ -100,31 +106,14 @@ MCIndexNoder::intersectChains()
 void
 MCIndexNoder::add(SegmentString* segStr)
 {
-    std::vector<std::unique_ptr<MonotoneChain>> segChains;
+    // std::vector<std::unique_ptr<MonotoneChain>> segChains;
 
     // segChains will contain newly allocated MonotoneChain objects
     MonotoneChainBuilder::getChains(segStr->getCoordinates(),
-                                    segStr, segChains);
-
-    for(auto& mc : segChains) {
-        assert(mc);
+                                    segStr, monoChains);
 
-        mc->setId(idCounter++);
-        // index.insert(&(mc->getEnvelope()), mc.get());
-        index.insert(&(mc->getEnvelope(overlapTolerance)), mc.get());
-
-        // MonotoneChain objects deletion delegated to destructor
-        monoChains.push_back(mc.release());
-    }
 }
 
-MCIndexNoder::~MCIndexNoder()
-{
-    for(MonotoneChain* mc: monoChains) {
-        assert(mc);
-        delete mc;
-    }
-}
 
 void
 MCIndexNoder::SegmentOverlapAction::overlap(MonotoneChain& mc1, std::size_t start1,
diff --git a/src/noding/MCIndexSegmentSetMutualIntersector.cpp b/src/noding/MCIndexSegmentSetMutualIntersector.cpp
index afc3470..0705905 100644
--- a/src/noding/MCIndexSegmentSetMutualIntersector.cpp
+++ b/src/noding/MCIndexSegmentSetMutualIntersector.cpp
@@ -33,21 +33,24 @@ using namespace geos::index::chain;
 namespace geos {
 namespace noding { // geos::noding
 
+
 /*private*/
 void
 MCIndexSegmentSetMutualIntersector::addToIndex(SegmentString* segStr)
 {
-    MonoChains segChains;
     MonotoneChainBuilder::getChains(segStr->getCoordinates(),
-                                    segStr, segChains);
-
-    MonoChains::size_type n = segChains.size();
-    chainStore.reserve(chainStore.size() + n);
-    for(auto& mc : segChains) {
-        mc->setId(indexCounter++);
-        index->insert(&(mc->getEnvelope()), mc.get());
-        chainStore.push_back(std::move(mc));
-    }
+                                    segStr, indexChains);
+
+}
+
+
+/*private*/
+void
+MCIndexSegmentSetMutualIntersector::addToMonoChains(SegmentString* segStr)
+{
+    MonotoneChainBuilder::getChains(segStr->getCoordinates(),
+                                    segStr, monoChains);
+
 }
 
 
@@ -58,15 +61,15 @@ MCIndexSegmentSetMutualIntersector::intersectChains()
     MCIndexSegmentSetMutualIntersector::SegmentOverlapAction overlapAction(*segInt);
 
     std::vector<void*> overlapChains;
-    for(const auto& queryChain : monoChains) {
+    for(auto& queryChain : monoChains) {
         overlapChains.clear();
 
-        index->query(&(queryChain->getEnvelope()), overlapChains);
+        index->query(&(queryChain.getEnvelope()), overlapChains);
 
         for(std::size_t j = 0, nj = overlapChains.size(); j < nj; j++) {
             MonotoneChain* testChain = (MonotoneChain*)(overlapChains[j]);
 
-            queryChain->computeOverlaps(testChain, &overlapAction);
+            queryChain.computeOverlaps(testChain, &overlapAction);
             nOverlaps++;
             if(segInt->isDone()) {
                 return;
@@ -75,37 +78,6 @@ MCIndexSegmentSetMutualIntersector::intersectChains()
     }
 }
 
-/*private*/
-void
-MCIndexSegmentSetMutualIntersector::addToMonoChains(SegmentString* segStr)
-{
-    MonoChains segChains;
-    MonotoneChainBuilder::getChains(segStr->getCoordinates(),
-                                    segStr, segChains);
-
-    MonoChains::size_type n = segChains.size();
-    monoChains.reserve(monoChains.size() + n);
-    for(auto& mc : segChains) {
-        mc->setId(processCounter++);
-        monoChains.push_back(std::move(mc));
-    }
-}
-
-/* public */
-MCIndexSegmentSetMutualIntersector::MCIndexSegmentSetMutualIntersector()
-    :	monoChains(),
-      index(new geos::index::strtree::SimpleSTRtree()),
-      indexCounter(0),
-      processCounter(0),
-      nOverlaps(0)
-{
-}
-
-/* public */
-MCIndexSegmentSetMutualIntersector::~MCIndexSegmentSetMutualIntersector()
-{
-    delete index;
-}
 
 /* public */
 void
@@ -113,8 +85,7 @@ MCIndexSegmentSetMutualIntersector::setBaseSegments(SegmentString::ConstVect* se
 {
     // NOTE - mloskot: const qualifier is removed silently, dirty.
 
-    for(std::size_t i = 0, n = segStrings->size(); i < n; i++) {
-        const SegmentString* css = (*segStrings)[i];
+    for(const SegmentString* css: *segStrings) {
         SegmentString* ss = const_cast<SegmentString*>(css);
         addToIndex(ss);
     }
@@ -124,14 +95,21 @@ MCIndexSegmentSetMutualIntersector::setBaseSegments(SegmentString::ConstVect* se
 void
 MCIndexSegmentSetMutualIntersector::process(SegmentString::ConstVect* segStrings)
 {
-    processCounter = indexCounter + 1;
-    nOverlaps = 0;
+    if (!indexBuilt) {
+        for (auto& mc: indexChains) {
+            index->insert(&(mc.getEnvelope()), &mc);
+        }
+        indexBuilt = true;
+    }
 
+    // Reset counters for new inputs
     monoChains.clear();
+    processCounter = indexCounter + 1;
+    nOverlaps = 0;
 
-    for(SegmentString::ConstVect::size_type i = 0, n = segStrings->size(); i < n; i++) {
-        SegmentString* seg = (SegmentString*)((*segStrings)[i]);
-        addToMonoChains(seg);
+    for(const SegmentString* css: *segStrings) {
+        SegmentString* ss = const_cast<SegmentString*>(css);
+        addToMonoChains(ss);
     }
     intersectChains();
 }

-----------------------------------------------------------------------

Summary of changes:
 include/geos/index/chain/MonotoneChain.h           | 22 +-----
 include/geos/index/chain/MonotoneChainBuilder.h    | 16 +----
 include/geos/noding/MCIndexNoder.h                 | 21 +++---
 .../noding/MCIndexSegmentSetMutualIntersector.h    | 24 +++++--
 src/index/chain/MonotoneChain.cpp                  | 19 ++---
 src/index/chain/MonotoneChainBuilder.cpp           | 14 +---
 src/noding/MCIndexNoder.cpp                        | 39 ++++-------
 src/noding/MCIndexSegmentSetMutualIntersector.cpp  | 80 ++++++++--------------
 8 files changed, 84 insertions(+), 151 deletions(-)


hooks/post-receive
-- 
GEOS


More information about the geos-commits mailing list