From f89069eb1aa36d5a15575086355003ebc70695f3 Mon Sep 17 00:00:00 2001 From: Daniel Baston Date: Wed, 15 Feb 2023 21:14:17 -0500 Subject: [PATCH] MCIndexSegmentSetMutualIntersector: Allow usage from multiple threads --- .../chain/MonotoneChainBuilderPerfTest.cpp | 2 +- .../index/chain/MonotoneChainPerfTest.cpp | 4 +- include/geos/index/chain/MonotoneChain.h | 3 +- .../geos/index/chain/MonotoneChainBuilder.h | 1 + .../noding/FastSegmentSetIntersectionFinder.h | 5 +- .../MCIndexSegmentSetMutualIntersector.h | 28 +++------ .../geos/noding/SegmentIntersectionDetector.h | 6 +- .../prep/AbstractPreparedPolygonContains.cpp | 4 +- src/index/chain/MonotoneChain.cpp | 21 +++---- src/index/chain/MonotoneChainBuilder.cpp | 9 ++- .../FastSegmentSetIntersectionFinder.cpp | 9 +-- src/noding/MCIndexNoder.cpp | 4 +- .../MCIndexSegmentSetMutualIntersector.cpp | 63 ++++++++----------- src/noding/SegmentIntersectionDetector.cpp | 8 +-- src/operation/buffer/SegmentMCIndex.cpp | 2 +- tests/unit/geom/prep/PreparedGeometryTest.cpp | 20 +++++- .../index/chain/MonotoneChainBuilderTest.cpp | 2 +- tests/unit/operation/relate/RelateOpTest.cpp | 2 +- 18 files changed, 85 insertions(+), 108 deletions(-) diff --git a/benchmarks/index/chain/MonotoneChainBuilderPerfTest.cpp b/benchmarks/index/chain/MonotoneChainBuilderPerfTest.cpp index 2b7ecd8a2..4218305e8 100644 --- a/benchmarks/index/chain/MonotoneChainBuilderPerfTest.cpp +++ b/benchmarks/index/chain/MonotoneChainBuilderPerfTest.cpp @@ -60,7 +60,7 @@ static void BM_MonotoneChainBuilder(benchmark::State& state) { for (auto _ : state) { std::vector chains; - MonotoneChainBuilder::getChains(&cs, nullptr, chains); + MonotoneChainBuilder::getChains(&cs, nullptr, 0, chains); } } diff --git a/benchmarks/index/chain/MonotoneChainPerfTest.cpp b/benchmarks/index/chain/MonotoneChainPerfTest.cpp index 2cf4a34c0..be6dfc99f 100644 --- a/benchmarks/index/chain/MonotoneChainPerfTest.cpp +++ b/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 diff --git a/include/geos/index/chain/MonotoneChain.h b/include/geos/index/chain/MonotoneChain.h index 4b0dbb427..1cb4c2b11 100644 --- a/include/geos/index/chain/MonotoneChain.h +++ b/include/geos/index/chain/MonotoneChain.h @@ -96,13 +96,12 @@ class GEOS_DLL MonotoneChain { /// Ownership left to caller, this class holds a reference. /// 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 diff --git a/include/geos/index/chain/MonotoneChainBuilder.h b/include/geos/index/chain/MonotoneChainBuilder.h index 815c39b3d..7db4bfd4a 100644 --- a/include/geos/index/chain/MonotoneChainBuilder.h +++ b/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& mcList); /** diff --git a/include/geos/noding/FastSegmentSetIntersectionFinder.h b/include/geos/noding/FastSegmentSetIntersectionFinder.h index c77ad8685..b6ba5df5b 100644 --- a/include/geos/noding/FastSegmentSetIntersectionFinder.h +++ b/include/geos/noding/FastSegmentSetIntersectionFinder.h @@ -49,8 +49,7 @@ namespace noding { // geos::noding */ class FastSegmentSetIntersectionFinder { private: - std::unique_ptr segSetMutInt; - std::unique_ptr 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); diff --git a/include/geos/noding/MCIndexSegmentSetMutualIntersector.h b/include/geos/noding/MCIndexSegmentSetMutualIntersector.h index abc687919..d3c1ae48f 100644 --- a/include/geos/noding/MCIndexSegmentSetMutualIntersector.h +++ b/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,7 +92,6 @@ class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector { private: typedef std::vector MonoChains; - MonoChains monoChains; /* * The index::SpatialIndex used should be something that supports @@ -106,23 +99,18 @@ class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector { * or index::strtree::STRtree). */ index::strtree::TemplateSTRtree 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; }; diff --git a/include/geos/noding/SegmentIntersectionDetector.h b/include/geos/noding/SegmentIntersectionDetector.h index 2165becb4..a99e9b0d5 100644 --- a/include/geos/noding/SegmentIntersectionDetector.h +++ b/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; } diff --git a/src/geom/prep/AbstractPreparedPolygonContains.cpp b/src/geom/prep/AbstractPreparedPolygonContains.cpp index cfc87436c..b2821653b 100644 --- a/src/geom/prep/AbstractPreparedPolygonContains.cpp +++ b/src/geom/prep/AbstractPreparedPolygonContains.cpp @@ -83,9 +83,7 @@ AbstractPreparedPolygonContains::findAndClassifyIntersections(const geom::Geomet noding::SegmentString::ConstVect lineSegStr; noding::SegmentStringUtil::extractSegmentStrings(geom, lineSegStr); - algorithm::LineIntersector li; - - noding::SegmentIntersectionDetector intDetector(&li); + noding::SegmentIntersectionDetector intDetector; intDetector.setFindAllIntersectionTypes(true); prepPoly->getIntersectionFinder()->intersects(&lineSegStr, &intDetector); diff --git a/src/index/chain/MonotoneChain.cpp b/src/index/chain/MonotoneChain.cpp index e3d336a33..2e3703bf7 100644 --- a/src/index/chain/MonotoneChain.cpp +++ b/src/index/chain/MonotoneChain.cpp @@ -32,29 +32,22 @@ namespace index { // geos.index namespace chain { // geos.index.chain MonotoneChain::MonotoneChain(const geom::CoordinateSequence& newPts, - std::size_t nstart, std::size_t nend, void* nContext) + std::size_t nstart, std::size_t nend, void* nContext, + double expansionDistance) : pts(&newPts) , context(nContext) , start(nstart) , end(nend) - , env() -{} - -const Envelope& -MonotoneChain::getEnvelope() const + , env(pts->getAt(start), pts->getAt(end)) { - return getEnvelope(0.0); + if (expansionDistance > 0.0) { + env.expandBy(expansionDistance); + } } const Envelope& -MonotoneChain::getEnvelope(double expansionDistance) const +MonotoneChain::getEnvelope() const { - if (env.isNull()) { - env.init(pts->getAt(start), pts->getAt(end)); - if (expansionDistance > 0.0) { - env.expandBy(expansionDistance); - } - } return env; } diff --git a/src/index/chain/MonotoneChainBuilder.cpp b/src/index/chain/MonotoneChainBuilder.cpp index 9185fe83a..44bd9a579 100644 --- a/src/index/chain/MonotoneChainBuilder.cpp +++ b/src/index/chain/MonotoneChainBuilder.cpp @@ -48,13 +48,14 @@ namespace chain { // geos.index.chain */ class ChainBuilder : public CoordinateFilter { public: - ChainBuilder(const CoordinateSequence* pts, void* context, std::vector & list) : + ChainBuilder(const CoordinateSequence* pts, void* context, double expansionDistance, std::vector & list) : m_prev(nullptr), m_i(0), m_quadrant(-1), m_start(0), m_seq(pts), m_context(context), + m_distance(expansionDistance), m_list(list) {} void filter_ro(const CoordinateXY* c) override { @@ -72,7 +73,7 @@ class ChainBuilder : public CoordinateFilter { void finishChain() { if ( m_i == 0 ) return; std::size_t chainEnd = m_i - 1; - m_list.emplace_back(*m_seq, m_start, chainEnd, m_context); + m_list.emplace_back(*m_seq, m_start, chainEnd, m_context, m_distance); m_start = chainEnd; } @@ -99,6 +100,7 @@ class ChainBuilder : public CoordinateFilter { std::size_t m_start; const CoordinateSequence* m_seq; void* m_context; + double m_distance; std::vector& m_list; }; @@ -106,8 +108,9 @@ class ChainBuilder : public CoordinateFilter { /* static public */ void MonotoneChainBuilder::getChains(const CoordinateSequence* pts, void* context, + double expansionDistance, std::vector& mcList) { - ChainBuilder builder(pts, context, mcList); + ChainBuilder builder(pts, context, expansionDistance, mcList); pts->apply_ro(&builder); builder.finish(); } diff --git a/src/noding/FastSegmentSetIntersectionFinder.cpp b/src/noding/FastSegmentSetIntersectionFinder.cpp index c9f528922..8960b4e9b 100644 --- a/src/noding/FastSegmentSetIntersectionFinder.cpp +++ b/src/noding/FastSegmentSetIntersectionFinder.cpp @@ -39,17 +39,15 @@ namespace noding { // geos::noding */ FastSegmentSetIntersectionFinder:: FastSegmentSetIntersectionFinder(noding::SegmentString::ConstVect* baseSegStrings) - : segSetMutInt(new MCIndexSegmentSetMutualIntersector()), - lineIntersector(new algorithm::LineIntersector()) { - segSetMutInt->setBaseSegments(baseSegStrings); + segSetMutInt.setBaseSegments(baseSegStrings); } bool FastSegmentSetIntersectionFinder:: intersects(noding::SegmentString::ConstVect* segStrings) { - SegmentIntersectionDetector intFinder(lineIntersector.get()); + SegmentIntersectionDetector intFinder; return this->intersects(segStrings, &intFinder); } @@ -59,8 +57,7 @@ FastSegmentSetIntersectionFinder:: intersects(noding::SegmentString::ConstVect* segStrings, SegmentIntersectionDetector* intDetector) { - segSetMutInt->setSegmentIntersector(intDetector); - segSetMutInt->process(segStrings); + segSetMutInt.process(segStrings, intDetector); return intDetector->hasIntersection(); } diff --git a/src/noding/MCIndexNoder.cpp b/src/noding/MCIndexNoder.cpp index 47ffaa4b4..d414d2cd9 100644 --- a/src/noding/MCIndexNoder.cpp +++ b/src/noding/MCIndexNoder.cpp @@ -51,7 +51,7 @@ MCIndexNoder::computeNodes(SegmentString::NonConstVect* inputSegStrings) if (!indexBuilt) { for(const auto& mc : monoChains) { - index.insert(mc.getEnvelope(overlapTolerance), &mc); + index.insert(mc.getEnvelope(), &mc); } indexBuilt = true; } @@ -84,7 +84,7 @@ MCIndexNoder::add(SegmentString* segStr) // segChains will contain newly allocated MonotoneChain objects MonotoneChainBuilder::getChains(segStr->getCoordinates(), - segStr, monoChains); + segStr, overlapTolerance, monoChains); } diff --git a/src/noding/MCIndexSegmentSetMutualIntersector.cpp b/src/noding/MCIndexSegmentSetMutualIntersector.cpp index 148ace410..1f043b250 100644 --- a/src/noding/MCIndexSegmentSetMutualIntersector.cpp +++ b/src/noding/MCIndexSegmentSetMutualIntersector.cpp @@ -13,14 +13,11 @@ **********************************************************************/ #include -#include #include #include -#include #include #include #include -#include // std #include @@ -33,37 +30,26 @@ namespace noding { // geos::noding /*private*/ void -MCIndexSegmentSetMutualIntersector::addToIndex(SegmentString* segStr) -{ - MonotoneChainBuilder::getChains(segStr->getCoordinates(), - segStr, indexChains); - -} - - -/*private*/ -void -MCIndexSegmentSetMutualIntersector::addToMonoChains(SegmentString* segStr) +MCIndexSegmentSetMutualIntersector::addChains(const SegmentString* segStr, MonoChains& chains) const { if (segStr->size() == 0) return; MonotoneChainBuilder::getChains(segStr->getCoordinates(), - segStr, monoChains); + (void*) segStr, overlapTolerance, chains); } /*private*/ void -MCIndexSegmentSetMutualIntersector::intersectChains() +MCIndexSegmentSetMutualIntersector::intersectChains(const MonoChains& chains, SegmentIntersector& segmentIntersector) { - MCIndexSegmentSetMutualIntersector::SegmentOverlapAction overlapAction(*segInt); + MCIndexSegmentSetMutualIntersector::SegmentOverlapAction overlapAction(segmentIntersector); - for(auto& queryChain : monoChains) { - index.query(queryChain.getEnvelope(overlapTolerance), [&queryChain, &overlapAction, this](const MonotoneChain* testChain) -> bool { + for(auto& queryChain : chains) { + index.query(queryChain.getEnvelope(), [&queryChain, &overlapAction, &segmentIntersector, this](const MonotoneChain* testChain) -> bool { queryChain.computeOverlaps(testChain, overlapTolerance, &overlapAction); - nOverlaps++; - return !segInt->isDone(); // abort early if segInt->isDone() + return !segmentIntersector.isDone(); // abort early if segmentIntersector->isDone() }); } } @@ -73,13 +59,8 @@ MCIndexSegmentSetMutualIntersector::intersectChains() void MCIndexSegmentSetMutualIntersector::setBaseSegments(SegmentString::ConstVect* segStrings) { - // NOTE - mloskot: const qualifier is removed silently, dirty. - for(const SegmentString* css: *segStrings) { - if (css->size() == 0) - continue; - SegmentString* ss = const_cast(css); - addToIndex(ss); + addChains(css, indexChains); } } @@ -87,23 +68,29 @@ MCIndexSegmentSetMutualIntersector::setBaseSegments(SegmentString::ConstVect* se void MCIndexSegmentSetMutualIntersector::process(SegmentString::ConstVect* segStrings) { - if (!indexBuilt) { + process(segStrings, segInt); +} + +/*public*/ +void +MCIndexSegmentSetMutualIntersector::process(SegmentString::ConstVect* segStrings, + SegmentIntersector* segmentIntersector) +{ + std::call_once(indexBuilt, [this]() { for (auto& mc: indexChains) { - index.insert(&(mc.getEnvelope(overlapTolerance)), &mc); + index.insert(&(mc.getEnvelope()), &mc); } - indexBuilt = true; - } - - // Reset counters for new inputs - monoChains.clear(); - processCounter = indexCounter + 1; - nOverlaps = 0; + }); + // TODO: Rework MonotoneChain extraction to take a callback, so we can pass the chains + // to intersectChains as they are identified. + MonoChains monoChains; for(const SegmentString* css: *segStrings) { SegmentString* ss = const_cast(css); - addToMonoChains(ss); + addChains(ss, monoChains); } - intersectChains(); + + intersectChains(monoChains, *segmentIntersector); } diff --git a/src/noding/SegmentIntersectionDetector.cpp b/src/noding/SegmentIntersectionDetector.cpp index 97924018b..c25e4ffab 100644 --- a/src/noding/SegmentIntersectionDetector.cpp +++ b/src/noding/SegmentIntersectionDetector.cpp @@ -43,13 +43,13 @@ processIntersections( const CoordinateXY& p10 = e1->getCoordinate( segIndex1 ); const CoordinateXY& p11 = e1->getCoordinate( segIndex1 + 1 ); - li->computeIntersection(p00, p01, p10, p11); + li.computeIntersection(p00, p01, p10, p11); - if(li->hasIntersection()) { + if(li.hasIntersection()) { // record intersection info _hasIntersection = true; - bool isProper = li->isProper(); + bool isProper = li.isProper(); if(isProper) { _hasProperIntersection = true; @@ -69,7 +69,7 @@ processIntersections( if(!intPt || saveLocation) { // record intersection location (approximate) - intPt = &li->getIntersection(0); + intPt = &li.getIntersection(0); delete intSegments; diff --git a/src/operation/buffer/SegmentMCIndex.cpp b/src/operation/buffer/SegmentMCIndex.cpp index 9441f5a9e..0c2b5f812 100644 --- a/src/operation/buffer/SegmentMCIndex.cpp +++ b/src/operation/buffer/SegmentMCIndex.cpp @@ -37,7 +37,7 @@ SegmentMCIndex::SegmentMCIndex(const CoordinateSequence* segs) void SegmentMCIndex::buildIndex(const CoordinateSequence* segs) { - chain::MonotoneChainBuilder::getChains(segs, nullptr, segChains); + chain::MonotoneChainBuilder::getChains(segs, nullptr, 0.0, segChains); for (chain::MonotoneChain& mc : segChains) { index.insert(&(mc.getEnvelope()), &mc); } diff --git a/tests/unit/geom/prep/PreparedGeometryTest.cpp b/tests/unit/geom/prep/PreparedGeometryTest.cpp index ca9a4f949..1d0b318d1 100644 --- a/tests/unit/geom/prep/PreparedGeometryTest.cpp +++ b/tests/unit/geom/prep/PreparedGeometryTest.cpp @@ -76,7 +76,8 @@ void object::test<2> () { std::vector> geoms; - std::vector> pgeoms; + std::vector> ppolys; + std::vector> plines; std::vector threads; constexpr std::size_t nrow = 10; @@ -89,18 +90,31 @@ void object::test<2> auto pt = factory->createPoint(c); geoms.emplace_back(pt->buffer(1.5)); - pgeoms.push_back(prep::PreparedGeometryFactory::prepare(geoms.back().get())); + ppolys.push_back(prep::PreparedGeometryFactory::prepare(geoms.back().get())); + plines.push_back(prep::PreparedGeometryFactory::prepare(static_cast(geoms.back().get())->getExteriorRing())); } } auto findIntersects = [&geoms](const PreparedGeometry* pg) { for (const auto& geom : geoms) { pg->intersects(geom.get()); + pg->distance(geom.get()); } }; + // check PreparedPolygon for (std::size_t i = 0; i < nthreads; i++) { - threads.emplace_back(findIntersects, pgeoms[i].get()); + threads.emplace_back(findIntersects, ppolys[0].get()); + } + + for (auto& thread : threads) { + thread.join(); + } + + // check PreparedLineString + threads.clear(); + for (std::size_t i = 0; i < nthreads; i++) { + threads.emplace_back(findIntersects, plines[0].get()); } for (auto& thread : threads) { diff --git a/tests/unit/index/chain/MonotoneChainBuilderTest.cpp b/tests/unit/index/chain/MonotoneChainBuilderTest.cpp index 50b615ae6..4fd9265b2 100644 --- a/tests/unit/index/chain/MonotoneChainBuilderTest.cpp +++ b/tests/unit/index/chain/MonotoneChainBuilderTest.cpp @@ -40,7 +40,7 @@ void object::test<1> std::vector chain; geos::geom::CoordinateSequence pts; - geos::index::chain::MonotoneChainBuilder::getChains(&pts, 0, chain); + geos::index::chain::MonotoneChainBuilder::getChains(&pts, nullptr, 0, chain); ensure_equals(chain.size(), 0u); } diff --git a/tests/unit/operation/relate/RelateOpTest.cpp b/tests/unit/operation/relate/RelateOpTest.cpp index f65cd9a6f..5d93997cf 100644 --- a/tests/unit/operation/relate/RelateOpTest.cpp +++ b/tests/unit/operation/relate/RelateOpTest.cpp @@ -111,7 +111,7 @@ void object::test<3>() // Launch some threads to check relationships between polygons std::vector threads; for (std::size_t i = 0; i < numThreads; i++) { - threads.emplace_back(runRelate, geoms[i].get(), std::ref(geoms)); + threads.emplace_back(runRelate, geoms[0].get(), std::ref(geoms)); } // Wait for threads to complete