Preserve the correct cell when flushing. (#132)

* Preserve the correct cell when flushing.

* Remove debug cruft.

* Add perhaps helpful comment.

Co-authored-by: abell <abell@abells-MacBook-Pro.local>

.gitignore

@@ -1,4 +1,4 @@
-# MacOS 
+# MacOS
 .DS_Store
 
 # Prerequisites
@@ -33,3 +33,6 @@
 *.exe
 *.out
 *.app
+
+# Cmake
+build

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,20 @@ void Cell::write()
         m_writer->replace(std::move(m_buf));
 }
 
+void Cell::initialize(const Cell *exclude)
+{
+    m_buf = m_cellMgr->getBuffer(exclude);
+    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 +53,44 @@ 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> cell(new Cell(key, m_pointSize, m_writer, f));
+        std::unique_ptr<Cell> 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());
 
-    // 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

epf/Cell.hpp

@@ -10,7 +10,6 @@
  *                                                                           *
  ****************************************************************************/
 
-
 #pragma once
 
 #include <cstdint>
@@ -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<void(Cell *)>;
 
-    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
@@ -61,28 +61,31 @@ class Cell
 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<VoxelKey, std::unique_ptr<Cell>>;
     int m_pointSize;
     Writer *m_writer;
     CellMap m_cells;
+
+    DataVecPtr getBuffer(const Cell* exclude);
+    void flush(const Cell *exclude);
 };
 
 

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,10 @@ void FileProcessor::run()
             VoxelKey cellIndex = m_grid.key(p.x(), p.y(), p.z());
             if (cellIndex != cell->key())
             {
-                cell = m_cellMgr.get(cellIndex);
+                // 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);
             }
             // Advance the cell - move the buffer pointer so when we refer to the cell's

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);

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