Make prepared geometry (more?) thread-safe

benchmarks/index/chain/MonotoneChainBuilderPerfTest.cpp

@@ -60,7 +60,7 @@ static void BM_MonotoneChainBuilder(benchmark::State& state) {
 
     for (auto _ : state) {
         std::vector<MonotoneChain> chains;
-        MonotoneChainBuilder::getChains(&cs, nullptr, chains);
+        MonotoneChainBuilder::getChains(&cs, nullptr, 0, chains);
     }
 }
 

benchmarks/index/chain/MonotoneChainPerfTest.cpp

@@ -48,8 +48,8 @@ static void BM_MonotoneChainOverlaps(benchmark::State& state) {
         prev = c;
     }
 
-    MonotoneChain mc1(cs1, 0, cs1.size(), nullptr);
-    MonotoneChain mc2(cs2, 0, cs1.size(), nullptr);
+    MonotoneChain mc1(cs1, 0, cs1.size(), nullptr, 0.0);
+    MonotoneChain mc2(cs2, 0, cs1.size(), nullptr, 0.0);
 
     struct EmptyOverlapAction : public MonotoneChainOverlapAction {
         virtual void

include/geos/geom/prep/PreparedLineString.h

@@ -40,10 +40,13 @@ namespace prep { // geos::geom::prep
  */
 class PreparedLineString : public BasicPreparedGeometry {
 private:
-    std::unique_ptr<noding::FastSegmentSetIntersectionFinder> segIntFinder;
+    mutable std::unique_ptr<noding::FastSegmentSetIntersectionFinder> segIntFinder;
     mutable noding::SegmentString::ConstVect segStrings;
     mutable std::unique_ptr<operation::distance::IndexedFacetDistance> indexedDistance;
 
+    mutable std::once_flag segIntFinderFlag;
+    mutable std::once_flag indexedDistanceFlag;
+
 protected:
 public:
     PreparedLineString(const Geometry* geom)
@@ -54,7 +57,7 @@ class PreparedLineString : public BasicPreparedGeometry {
 
     ~PreparedLineString() override;
 
-    noding::FastSegmentSetIntersectionFinder* getIntersectionFinder();
+    noding::FastSegmentSetIntersectionFinder* getIntersectionFinder() const;
 
     bool intersects(const geom::Geometry* g) const override;
     std::unique_ptr<geom::CoordinateSequence> nearestPoints(const geom::Geometry* g) const override;

include/geos/geom/prep/PreparedLineStringIntersects.h

@@ -58,10 +58,7 @@ class PreparedLineStringIntersects {
         return op.intersects(geom);
     }
 
-    /**
-     * \todo FIXME - mloskot: Why not taking linestring through const reference?
-     */
-    PreparedLineStringIntersects(PreparedLineString& prep)
+    PreparedLineStringIntersects(const PreparedLineString& prep)
         : prepLine(prep)
     { }
 
@@ -74,7 +71,7 @@ class PreparedLineStringIntersects {
     bool intersects(const geom::Geometry* g) const;
 
 protected:
-    PreparedLineString& prepLine;
+    const PreparedLineString& prepLine;
 
     /**
      * Tests whether any representative point of the test Geometry intersects

include/geos/geom/prep/PreparedPolygon.h

@@ -50,13 +50,19 @@ namespace prep { // geos::geom::prep
  */
 class PreparedPolygon : public BasicPreparedGeometry {
 private:
-    bool isRectangle;
     mutable std::unique_ptr<noding::FastSegmentSetIntersectionFinder> segIntFinder;
     mutable std::unique_ptr<algorithm::locate::PointOnGeometryLocator> ptOnGeomLoc;
     mutable std::unique_ptr<algorithm::locate::PointOnGeometryLocator> indexedPtOnGeomLoc;
-    mutable noding::SegmentString::ConstVect segStrings;
     mutable std::unique_ptr<operation::distance::IndexedFacetDistance> indexedDistance;
 
+    mutable noding::SegmentString::ConstVect segStrings;
+
+    mutable std::once_flag segIntFinderFlag;
+    mutable std::once_flag ptOnGeomLocFlag;
+    mutable std::once_flag indexedPtOnGeomLocFlag;
+    mutable std::once_flag indexedDistanceFlag;
+    bool isRectangle;
+
 protected:
 public:
     PreparedPolygon(const geom::Geometry* geom);

include/geos/index/chain/MonotoneChain.h

@@ -95,14 +95,15 @@ class GEOS_DLL MonotoneChain {
     /// @param context
     ///   Ownership left to caller, this class holds a reference.
     ///
+    /// @param expansionDistance distance by which envelope should be expanded
+    ///
     MonotoneChain(const geom::CoordinateSequence& pts,
-                  std::size_t start, std::size_t end, void* context);
+                  std::size_t start, std::size_t end, void* context, double expansionDistance);
 
     ~MonotoneChain() = default;
 
     /// Returned envelope is owned by this class
     const geom::Envelope& getEnvelope() const;
-    const geom::Envelope& getEnvelope(double expansionDistance) const;
 
     size_t
     getStartIndex() const

include/geos/index/chain/MonotoneChainBuilder.h

@@ -58,6 +58,7 @@ class GEOS_DLL MonotoneChainBuilder {
      */
     static void getChains(const geom::CoordinateSequence* pts,
                           void* context,
+                          double expansionDistance,
                           std::vector<MonotoneChain>& mcList);
 
     /**

include/geos/index/strtree/TemplateSTRtree.h

@@ -231,7 +231,7 @@ class TemplateSTRtreeImpl {
 
     template<typename ItemDistance>
     ItemType nearestNeighbour(const BoundsType& env, const ItemType& item, ItemDistance& itemDist) {
-        build();
+        buildIfNeeded();
 
         if (getRoot() == nullptr) {
             return nullptr;
@@ -273,9 +273,7 @@ class TemplateSTRtreeImpl {
     // false values will be taken as a signal to stop the query.
     template<typename Visitor>
     void query(const BoundsType& queryEnv, Visitor &&visitor) {
-        if (!built()) {
-            build();
-        }
+        buildIfNeeded();
 
         if (root && root->boundsIntersect(queryEnv)) {
             if (root->isLeaf()) {
@@ -294,9 +292,7 @@ class TemplateSTRtreeImpl {
     // false values will be taken as a signal to stop the query.
     template<typename Visitor>
     void queryPairs(Visitor&& visitor) {
-        if (!built()) {
-            build();
-        }
+        buildIfNeeded();
 
         if (numItems < 2) {
             return;
@@ -318,7 +314,7 @@ class TemplateSTRtreeImpl {
      * Returns a depth-first iterator over all items in the tree.
      */
     Items items() {
-        build();
+        buildIfNeeded();
         return Items(*this);
     }
 
@@ -341,7 +337,7 @@ class TemplateSTRtreeImpl {
     /// @{
 
     bool remove(const BoundsType& itemEnv, const ItemType& item) {
-        build();
+        buildIfNeeded();
 
         if (root == nullptr) {
             return false;
@@ -369,16 +365,18 @@ class TemplateSTRtreeImpl {
 
     /** Determine whether the tree has been built, and no more items may be added. */
     const Node* getRoot() {
-        build();
+        buildIfNeeded();
         return root;
     }
 
     /// @}
 
     /** Build the tree if it has not already been built. */
-    void build() {
-        std::lock_guard<std::mutex> lock(lock_);
+    void buildIfNeeded() {
+        std::call_once(builtFlag, [this]() { build(); });
+    }
 
+    void build() {
         if (built()) {
             return;
         }
@@ -410,7 +408,7 @@ class TemplateSTRtreeImpl {
     }
 
 protected:
-    std::mutex lock_;
+    std::once_flag builtFlag;
     NodeList nodes;      //**< a list of all leaf and branch nodes in the tree. */
     Node* root;          //**< a pointer to the root node, if the tree has been built. */
     size_t nodeCapacity; //*< maximum number of children of each node */

include/geos/math/DD.h

@@ -92,6 +92,8 @@
 
 #pragma once
 
+#include <geos/export.h>
+
 #include <cmath>
 
 namespace geos {
@@ -115,36 +117,36 @@ class GEOS_DLL DD {
 
 
     public:
-        DD(double p_hi, double p_lo) : hi(p_hi), lo(p_lo) {};
-        DD(double x) : hi(x), lo(0.0) {};
-        DD() : hi(0.0), lo(0.0) {};
+        constexpr DD(double p_hi, double p_lo) : hi(p_hi), lo(p_lo) {};
+        constexpr DD(double x) : hi(x), lo(0.0) {};
+        constexpr DD() : hi(0.0), lo(0.0) {};
 
-        bool operator==(const DD &rhs) const
+        constexpr bool operator==(const DD &rhs) const
         {
             return hi == rhs.hi && lo == rhs.lo;
         }
 
-        bool operator!=(const DD &rhs) const
+        constexpr bool operator!=(const DD &rhs) const
         {
             return hi != rhs.hi || lo != rhs.lo;
         }
 
-        bool operator<(const DD &rhs) const
+        constexpr bool operator<(const DD &rhs) const
         {
             return (hi < rhs.hi) || (hi == rhs.hi && lo < rhs.lo);
         }
 
-        bool operator<=(const DD &rhs) const
+        constexpr bool operator<=(const DD &rhs) const
         {
             return (hi < rhs.hi) || (hi == rhs.hi && lo <= rhs.lo);
         }
 
-        bool operator>(const DD &rhs) const
+        constexpr bool operator>(const DD &rhs) const
         {
             return (hi > rhs.hi) || (hi == rhs.hi && lo > rhs.lo);
         }
 
-        bool operator>=(const DD &rhs) const
+        constexpr bool operator>=(const DD &rhs) const
         {
             return (hi > rhs.hi) || (hi == rhs.hi && lo >= rhs.lo);
         }

include/geos/noding/FastSegmentSetIntersectionFinder.h

@@ -49,8 +49,7 @@ namespace noding { // geos::noding
  */
 class FastSegmentSetIntersectionFinder {
 private:
-    std::unique_ptr<MCIndexSegmentSetMutualIntersector> segSetMutInt;
-    std::unique_ptr<geos::algorithm::LineIntersector> lineIntersector;
+    MCIndexSegmentSetMutualIntersector segSetMutInt;
 
 protected:
 public:
@@ -67,7 +66,7 @@ class FastSegmentSetIntersectionFinder {
     const SegmentSetMutualIntersector*
     getSegmentSetIntersector() const
     {
-        return segSetMutInt.get();
+        return &segSetMutInt;
     }
 
     bool intersects(SegmentString::ConstVect* segStrings);

include/geos/noding/MCIndexSegmentSetMutualIntersector.h

@@ -29,8 +29,6 @@ class SegmentIntersector;
 }
 }
 
-//using namespace geos::index::strtree;
-
 namespace geos {
 namespace noding { // geos::noding
 
@@ -45,12 +43,7 @@ class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector {
 public:
 
     MCIndexSegmentSetMutualIntersector(double p_tolerance)
-        : monoChains()
-        , indexCounter(0)
-        , processCounter(0)
-        , nOverlaps(0)
-        , overlapTolerance(p_tolerance)
-        , indexBuilt(false)
+        : overlapTolerance(p_tolerance)
     {}
 
     MCIndexSegmentSetMutualIntersector()
@@ -68,9 +61,10 @@ class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector {
 
     void setBaseSegments(SegmentString::ConstVect* segStrings) override;
 
-    // NOTE: re-populates the MonotoneChain vector with newly created chains
     void process(SegmentString::ConstVect* segStrings) override;
 
+    void process(SegmentString::ConstVect* segStrings, SegmentIntersector* si);
+
     class SegmentOverlapAction : public index::chain::MonotoneChainOverlapAction {
     private:
         SegmentIntersector& si;
@@ -98,31 +92,25 @@ class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector {
 private:
 
     typedef std::vector<index::chain::MonotoneChain> MonoChains;
-    MonoChains monoChains;
 
     /*
      * The index::SpatialIndex used should be something that supports
      * envelope (range) queries efficiently (such as a index::quadtree::Quadtree
      * or index::strtree::STRtree).
      */
     index::strtree::TemplateSTRtree<const index::chain::MonotoneChain*> index;
-    int indexCounter;
-    int processCounter;
-    // statistics
-    int nOverlaps;
-    double overlapTolerance;
+
+    const double overlapTolerance;
 
     /* memory management helper, holds MonotoneChain objects used
      * in the SpatialIndex. It's cleared when the SpatialIndex is
      */
-    bool indexBuilt;
+    std::once_flag indexBuilt;
     MonoChains indexChains;
 
-    void addToIndex(SegmentString* segStr);
-
-    void intersectChains();
+    void intersectChains(const MonoChains& chains, SegmentIntersector& segmentIntersector);
 
-    void addToMonoChains(SegmentString* segStr);
+    void addChains(const SegmentString* segStr, MonoChains& chains) const;
 
 };
 

include/geos/noding/SegmentIntersectionDetector.h

@@ -40,7 +40,7 @@ namespace noding { // geos::noding
  */
 class SegmentIntersectionDetector : public SegmentIntersector {
 private:
-    algorithm::LineIntersector* li;
+    algorithm::LineIntersector li;
 
     bool findProper;
     bool findAllTypes;
@@ -54,9 +54,8 @@ class SegmentIntersectionDetector : public SegmentIntersector {
 
 protected:
 public:
-    SegmentIntersectionDetector(algorithm::LineIntersector* p_li)
+    SegmentIntersectionDetector()
         :
-        li(p_li),
         findProper(false),
         findAllTypes(false),
         _hasIntersection(false),
@@ -68,7 +67,6 @@ class SegmentIntersectionDetector : public SegmentIntersector {
 
     ~SegmentIntersectionDetector() override
     {
-        //delete intPt;
         delete intSegments;
     }
 

src/algorithm/CGAlgorithmsDD.cpp

@@ -31,7 +31,8 @@ namespace {
 inline int
 OrientationDD(const DD &dd)
 {
-    static DD const zero(0.0);
+    constexpr const DD zero(0.0);
+
     if(dd < zero) {
         return CGAlgorithmsDD::RIGHT;
     }

src/algorithm/locate/IndexedPointInAreaLocator.cpp

@@ -100,15 +100,12 @@ IndexedPointInAreaLocator::buildIndex(const geom::Geometry& g)
 IndexedPointInAreaLocator::IndexedPointInAreaLocator(const geom::Geometry& g)
     :	areaGeom(g)
 {
+    buildIndex(areaGeom);
 }
 
 geom::Location
 IndexedPointInAreaLocator::locate(const geom::CoordinateXY* /*const*/ p)
 {
-    if (index == nullptr) {
-        buildIndex(areaGeom);
-    }
-
     algorithm::RayCrossingCounter rcc(*p);
 
     index->query(p->y, p->y, [&rcc](const SegmentView& ls) {