From df5a4e3af7d45668db60b89a8b5644d0312d9095 Mon Sep 17 00:00:00 2001 From: abell Date: Wed, 26 Oct 2022 13:17:55 -0400 Subject: [PATCH 1/3] Preserve the correct cell when flushing. --- epf/Cell.cpp | 61 +++++++++++++++++++++++-------------------- epf/Cell.hpp | 25 ++++++++++-------- epf/FileProcessor.cpp | 4 +-- epf/Grid.cpp | 2 +- epf/Grid.hpp | 2 +- 5 files changed, 50 insertions(+), 44 deletions(-) diff --git a/epf/Cell.cpp b/epf/Cell.cpp index b8324b2..2f522dd 100644 --- a/epf/Cell.cpp +++ b/epf/Cell.cpp @@ -19,22 +19,6 @@ namespace untwine namespace epf { -void Cell::initialize() -{ - m_buf = m_writer->fetchBuffer(); - - // If we couldn't fetch a buffer, flush all the the buffers for this processor and - // try again, but block. - if (!m_buf) - { - m_flush(this); - m_buf = m_writer->fetchBufferBlocking(); - } - m_pos = m_buf->data(); - - m_endPos = m_pos + m_pointSize * (BufSize / m_pointSize); -} - // NOTE - After write(), the cell is invalid and must be initialized or destroyed. void Cell::write() { @@ -45,13 +29,21 @@ void Cell::write() m_writer->replace(std::move(m_buf)); } +void Cell::initialize(const Cell *exclude) +{ + m_buf = m_cellMgr->getBuffer(exclude); + //ABELL - review this. + m_pos = m_buf->data(); + m_endPos = m_pos + m_pointSize * (BufSize / m_pointSize); +} + void Cell::advance() { m_pos += m_pointSize; if (m_pos >= m_endPos) { write(); - initialize(); + initialize(this); } } @@ -62,33 +54,46 @@ void Cell::advance() CellMgr::CellMgr(int pointSize, Writer *writer) : m_pointSize(pointSize), m_writer(writer) {} - -Cell *CellMgr::get(const VoxelKey& key) +Cell *CellMgr::get(const VoxelKey& key, const Cell *lastCell) { auto it = m_cells.find(key); if (it == m_cells.end()) { - Cell::FlushFunc f = [this](Cell *exclude) - { - flush(exclude); - }; - std::unique_ptr cell(new Cell(key, m_pointSize, m_writer, f)); + std::unique_ptr cell(new Cell(key, m_pointSize, m_writer, this, lastCell)); it = m_cells.insert( {key, std::move(cell)} ).first; } - Cell& c = *(it->second); - return &c; + + return it->second.get(); +} + +DataVecPtr CellMgr::getBuffer(const Cell *exclude) +{ + DataVecPtr b = m_writer->fetchBuffer(); + + // If we couldn't fetch a buffer, flush all the the buffers for this processor and + // try again, but block. + if (!b) + { + flush(exclude); + b = m_writer->fetchBufferBlocking(); + } + return b; } // Eliminate all the cells and their associated data buffers except the `exclude` // cell. -void CellMgr::flush(Cell *exclude) +void CellMgr::flush(const Cell *exclude) { CellMap::iterator it = m_cells.end(); if (exclude) + { it = m_cells.find(exclude->key()); + assert(it != m_cells.end()); + } - // If there was no exclude cell or it isn't in our list, just clear the cells. + // If there was no exclude cell just clear the cells. // Otherwise, save the exclude cell, clear the list, and reinsert. + // Cells are written when they are destroyed. if (it == m_cells.end()) m_cells.clear(); else diff --git a/epf/Cell.hpp b/epf/Cell.hpp index f1959e8..ed0833a 100644 --- a/epf/Cell.hpp +++ b/epf/Cell.hpp @@ -10,7 +10,6 @@ * * ****************************************************************************/ - #pragma once #include @@ -29,6 +28,7 @@ namespace epf { class Writer; +class CellMgr; // A cell represents a voxel that contains points. All cells are the same size. A cell has // a buffer which is filled by points. When the buffer is filled, it's passed to the writer @@ -38,18 +38,18 @@ class Cell public: using FlushFunc = std::function; - Cell(const VoxelKey& key, int pointSize, Writer *writer, FlushFunc flush) : - m_key(key), m_pointSize(pointSize), m_writer(writer), m_flush(flush) + Cell(const VoxelKey& key, int pointSize, Writer *writer, CellMgr *mgr, const Cell *lastCell) : + m_key(key), m_pointSize(pointSize), m_writer(writer), m_cellMgr(mgr) { assert(pointSize < BufSize); - initialize(); + initialize(lastCell); } ~Cell() { write(); } - void initialize(); + void initialize(const Cell *exclude); Point point() { return Point(m_pos); } VoxelKey key() const @@ -58,31 +58,34 @@ class Cell { std::copy(b.data(), b.data() + m_pointSize, m_pos); } void advance(); -private: +//private: DataVecPtr m_buf; VoxelKey m_key; - int m_pointSize; - Writer *m_writer; uint8_t *m_pos; uint8_t *m_endPos; - FlushFunc m_flush; + int m_pointSize; + Writer *m_writer; + CellMgr *m_cellMgr; void write(); }; class CellMgr { + friend class Cell; public: CellMgr(int pointSize, Writer *writer); - Cell *get(const VoxelKey& key); - void flush(Cell *exclude); + Cell *get(const VoxelKey& key, const Cell *lastCell = nullptr); private: using CellMap = std::unordered_map>; int m_pointSize; Writer *m_writer; CellMap m_cells; + + DataVecPtr getBuffer(const Cell* exclude); + void flush(const Cell *exclude); }; diff --git a/epf/FileProcessor.cpp b/epf/FileProcessor.cpp index 9367768..ac19be0 100644 --- a/epf/FileProcessor.cpp +++ b/epf/FileProcessor.cpp @@ -30,7 +30,6 @@ FileProcessor::FileProcessor(const FileInfo& fi, size_t pointSize, const Grid& g void FileProcessor::run() { - pdal::Options opts; opts.add("filename", m_fi.filename); opts.add("count", m_fi.numPoints); @@ -43,7 +42,6 @@ void FileProcessor::run() pdal::Stage *s = factory.createStage(m_fi.driver); s->setOptions(opts); - PointCount count = 0; // We need to move the data from the PointRef to some output buffer. We copy the data @@ -70,7 +68,7 @@ void FileProcessor::run() VoxelKey cellIndex = m_grid.key(p.x(), p.y(), p.z()); if (cellIndex != cell->key()) { - cell = m_cellMgr.get(cellIndex); + cell = m_cellMgr.get(cellIndex, cell); cell->copyPoint(p); } // Advance the cell - move the buffer pointer so when we refer to the cell's diff --git a/epf/Grid.cpp b/epf/Grid.cpp index 89f6a37..4c37f56 100644 --- a/epf/Grid.cpp +++ b/epf/Grid.cpp @@ -89,7 +89,7 @@ void Grid::resetLevel(int level) } } -VoxelKey Grid::key(double x, double y, double z) +VoxelKey Grid::key(double x, double y, double z) const { int xi = (int)std::floor((x - m_bounds.minx) / m_xsize); int yi = (int)std::floor((y - m_bounds.miny) / m_ysize); diff --git a/epf/Grid.hpp b/epf/Grid.hpp index 95805c9..921ea63 100644 --- a/epf/Grid.hpp +++ b/epf/Grid.hpp @@ -31,7 +31,7 @@ class Grid void expand(const pdal::BOX3D& bounds, size_t points); int calcLevel(); void resetLevel(int level); - VoxelKey key(double x, double y, double z); + VoxelKey key(double x, double y, double z) const; pdal::BOX3D processingBounds() const { return m_cubic ? m_cubicBounds : m_bounds; } pdal::BOX3D cubicBounds() const From d94694c2c5e4ff55131b0b3210d583dfdb1aa177 Mon Sep 17 00:00:00 2001 From: abell Date: Mon, 31 Oct 2022 19:17:26 -0400 Subject: [PATCH 2/3] Remove debug cruft. --- .gitignore | 5 ++++- epf/Cell.cpp | 5 +---- epf/Cell.hpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 84cfcd3..ff01eb2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -# MacOS +# MacOS .DS_Store # Prerequisites @@ -33,3 +33,6 @@ *.exe *.out *.app + +# Cmake +build diff --git a/epf/Cell.cpp b/epf/Cell.cpp index 2f522dd..1e1070e 100644 --- a/epf/Cell.cpp +++ b/epf/Cell.cpp @@ -32,7 +32,6 @@ void Cell::write() void Cell::initialize(const Cell *exclude) { m_buf = m_cellMgr->getBuffer(exclude); - //ABELL - review this. m_pos = m_buf->data(); m_endPos = m_pos + m_pointSize * (BufSize / m_pointSize); } @@ -85,11 +84,9 @@ DataVecPtr CellMgr::getBuffer(const Cell *exclude) void CellMgr::flush(const Cell *exclude) { CellMap::iterator it = m_cells.end(); + if (exclude) - { it = m_cells.find(exclude->key()); - assert(it != m_cells.end()); - } // If there was no exclude cell just clear the cells. // Otherwise, save the exclude cell, clear the list, and reinsert. diff --git a/epf/Cell.hpp b/epf/Cell.hpp index ed0833a..94f0fac 100644 --- a/epf/Cell.hpp +++ b/epf/Cell.hpp @@ -58,7 +58,7 @@ class Cell { std::copy(b.data(), b.data() + m_pointSize, m_pos); } void advance(); -//private: +private: DataVecPtr m_buf; VoxelKey m_key; uint8_t *m_pos; From 8a78bb49dfbdf8571e152783d7820bbc518759b1 Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Mon, 31 Oct 2022 19:25:45 -0400 Subject: [PATCH 3/3] Add perhaps helpful comment. --- epf/FileProcessor.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/epf/FileProcessor.cpp b/epf/FileProcessor.cpp index ac19be0..a5c98b8 100644 --- a/epf/FileProcessor.cpp +++ b/epf/FileProcessor.cpp @@ -68,6 +68,9 @@ void FileProcessor::run() VoxelKey cellIndex = m_grid.key(p.x(), p.y(), p.z()); if (cellIndex != cell->key()) { + // Make sure that we exclude the current cell from any potential flush so + // that no other thread can claim its data buffer and overwrite it before + // we have a chance to copy from it in copyPoint(). cell = m_cellMgr.get(cellIndex, cell); cell->copyPoint(p); }