From 6608f692e17900251e69c3b31182fa400a9a3c56 Mon Sep 17 00:00:00 2001
From: William F Godoy <williamfgc@yahoo.com>
Date: Thu, 20 Apr 2017 17:42:04 -0400
Subject: [PATCH] Started to implement BP1 memory buffer policy

Created public function ResizeBuffer in BP1Writer
---
 source/adios2/core/adiosFunctions.cpp         | 62 ++++++---------
 source/adios2/core/adiosFunctions.h           | 21 +----
 source/adios2/engine/bp/BPFileWriter.cpp      |  6 +-
 source/adios2/utilities/format/bp1/BP1Base.h  | 13 ++++
 .../adios2/utilities/format/bp1/BP1Writer.cpp |  9 ++-
 .../adios2/utilities/format/bp1/BP1Writer.h   | 70 +++++++++--------
 .../adios2/utilities/format/bp1/BP1Writer.tcc | 77 +++++++++++--------
 7 files changed, 131 insertions(+), 127 deletions(-)

diff --git a/source/adios2/core/adiosFunctions.cpp b/source/adios2/core/adiosFunctions.cpp
index ffb5c81f4..12988d0ac 100644
--- a/source/adios2/core/adiosFunctions.cpp
+++ b/source/adios2/core/adiosFunctions.cpp
@@ -24,6 +24,7 @@
 #include <stdexcept>
 #include <thread> //std::thread
 
+#include "adios2/ADIOSTypes.h"
 #include "adios2/core/Support.h"
 
 #ifdef ADIOS2_HAVE_BZIP2
@@ -661,54 +662,37 @@ void ConvertUint64VectorToSizetVector(const std::vector<std::uint64_t> &in,
     }
 }
 
-bool CheckBufferAllocation(const std::size_t newSize, const float growthFactor,
-                           const std::size_t maxBufferSize,
-                           std::vector<char> &buffer)
+int GrowBuffer(const size_t incomingDataSize, const float growthFactor,
+               std::vector<char> &buffer, const size_t position)
 {
-    // Check if data in buffer needs to be reallocated
-    const std::size_t requiredDataSize =
-        buffer.size() + newSize + 100; // adding some bytes for tolerance
-    // might need to write payload in batches
-    bool doTransportsFlush = (requiredDataSize > maxBufferSize) ? true : false;
+    const size_t currentCapacity = buffer.capacity();
+    const size_t availableSpace = currentCapacity - position;
+    const double gf = static_cast<double>(growthFactor);
 
-    if (GrowBuffer(requiredDataSize, growthFactor, buffer) == -1)
+    if (incomingDataSize < availableSpace)
     {
-        doTransportsFlush = true;
+        return 0;
     }
 
-    return doTransportsFlush;
-}
+    const size_t neededCapacity = incomingDataSize + position;
+    const double numerator = std::log(static_cast<double>(neededCapacity) /
+                                      static_cast<double>(currentCapacity));
+    const double denominator = std::log(gf);
 
-int GrowBuffer(const std::size_t incomingDataSize, const float growthFactor,
-               std::vector<char> &buffer)
-{
-    const std::size_t currentCapacity = buffer.capacity();
-    const std::size_t availableSpace = currentCapacity - buffer.size();
-    const double gf = static_cast<double>(growthFactor);
+    const double n = std::ceil(numerator / denominator);
+    const size_t newSize =
+        static_cast<size_t>(std::ceil(std::pow(gf, n) * currentCapacity));
 
-    if (incomingDataSize > availableSpace)
+    try
     {
-        const std::size_t neededCapacity = incomingDataSize + buffer.size();
-        const double numerator = std::log(static_cast<double>(neededCapacity) /
-                                          static_cast<double>(currentCapacity));
-        const double denominator = std::log(gf);
-
-        double n = std::ceil(numerator / denominator);
-        const std::size_t newSize = static_cast<std::size_t>(
-            std::ceil(std::pow(gf, n) * currentCapacity));
-
-        try
-        {
-            buffer.reserve(newSize);
-        }
-        catch (std::bad_alloc &e)
-        {
-            return -1;
-        }
-
-        return 1;
+        buffer.resize(newSize);
     }
-    return 0;
+    catch (std::bad_alloc &e)
+    {
+        return -1;
+    }
+
+    return 1;
 }
 
 bool IsLittleEndian() noexcept
diff --git a/source/adios2/core/adiosFunctions.h b/source/adios2/core/adiosFunctions.h
index 07f05fae2..bfedf18b0 100644
--- a/source/adios2/core/adiosFunctions.h
+++ b/source/adios2/core/adiosFunctions.h
@@ -195,33 +195,18 @@ void ConvertUint64VectorToSizetVector(const std::vector<std::uint64_t> &in,
  */
 std::string DimsToCSV(const std::vector<std::size_t> &dims);
 
-/**
- * Common strategy to check for heap buffer allocation for data and metadata
- * typically calculated in Write
- * @param newSize new data size
- * @param growthFactor user provided growth factor for index and data memory
- * buffers ( default = 1.5 )
- * @param maxBufferSize user provided maximum buffer size
- * @param buffer to be reallocated
- * @return true: must do a transport flush, false: buffer sizes are enough
- * to
- * contain incoming data, no need for transport flush
- */
-bool CheckBufferAllocation(const std::size_t newSize, const float growthFactor,
-                           const std::size_t maxBufferSize,
-                           std::vector<char> &buffer);
-
 /**
  * Grows a buffer by a factor of  n . growthFactor . currentCapacity to
  * accommodate for incomingDataSize
  * @param incomingDataSize size of new data required to be stored in buffer
  * @param growthFactor buffer grows in multiples of the growth buffer
  * @param buffer to be resized
+ * @param position, current buffer position
  * @return -1: failed to allocate (bad_alloc), 0: didn't have to allocate
  * (enough space), 1: successful allocation
  */
-int GrowBuffer(const std::size_t incomingDataSize, const float growthFactor,
-               std::vector<char> &buffer);
+int GrowBuffer(const size_t incomingDataSize, const float growthFactor,
+               std::vector<char> &buffer, const size_t position);
 
 /**
  * Check if system is little endian
diff --git a/source/adios2/engine/bp/BPFileWriter.cpp b/source/adios2/engine/bp/BPFileWriter.cpp
index 3e650472b..314519912 100644
--- a/source/adios2/engine/bp/BPFileWriter.cpp
+++ b/source/adios2/engine/bp/BPFileWriter.cpp
@@ -273,7 +273,7 @@ void BPFileWriter::Close(const int transportIndex)
 
         if (allClose == true) // aggregate and write profiling.log
         {
-            m_BP1Writer.DumpProfilingLogFile(m_Name, m_RankMPI, m_Transports);
+            m_BP1Writer.WriteProfilingLogFile(m_Name, m_RankMPI, m_Transports);
         }
     }
 }
@@ -395,8 +395,8 @@ void BPFileWriter::InitTransports()
     {
         auto itProfile = parameters.find("profile_units");
         bool doProfiling = false;
-        Support::Resolutions resolution =
-            Support::Resolutions::s; // default is seconds
+        // default is seconds for this engine
+        Support::Resolutions resolution = Support::Resolutions::s;
         if (itProfile != parameters.end())
         {
             if (itProfile->second == "mus" ||
diff --git a/source/adios2/utilities/format/bp1/BP1Base.h b/source/adios2/utilities/format/bp1/BP1Base.h
index 80b891462..0c9a2ff52 100644
--- a/source/adios2/utilities/format/bp1/BP1Base.h
+++ b/source/adios2/utilities/format/bp1/BP1Base.h
@@ -62,6 +62,8 @@ public:
      */
     BP1Base(MPI_Comm mpiComm, const bool debugMode);
 
+    virtual ~BP1Base() = default;
+
     /**
      * Checks if input name has .bp extension and returns a .bp directory name
      * @param name input (might or not have .bp)
@@ -80,6 +82,17 @@ public:
     void OpenRankFiles(const std::string name, const std::string accessMode,
                        Transport &file) const;
 
+    /**
+     * Return type of the CheckAllocation function.
+     */
+    enum class ResizeResult
+    {
+        FAILURE,   //!< FAILURE, caught a std::bad_alloc
+        UNCHANGED, //!< UNCHANGED, no need to resize (sufficient capacity)
+        SUCCESS,   //!< SUCCESS, resize was successful
+        FLUSH      //!< FLUSH, need to flush to transports for current variable
+    };
+
 protected:
     /** might be used in large payload copies to buffer */
     unsigned int m_Threads = 1;
diff --git a/source/adios2/utilities/format/bp1/BP1Writer.cpp b/source/adios2/utilities/format/bp1/BP1Writer.cpp
index 76814161e..666a3e6ed 100644
--- a/source/adios2/utilities/format/bp1/BP1Writer.cpp
+++ b/source/adios2/utilities/format/bp1/BP1Writer.cpp
@@ -156,12 +156,12 @@ void BP1Writer::Close(Transport &transport, bool &isFirstClose,
     }
 }
 
-void BP1Writer::DumpProfilingLogFile(
-    const std::string name, const unsigned int rank,
+void BP1Writer::WriteProfilingLogFile(
+    const std::string &name, const unsigned int rank,
     const std::vector<std::shared_ptr<Transport>> &transports) noexcept
 {
     const std::string fileName(GetDirectoryName(name) + "/profiling.log");
-    const std::string rankLog = GetRankProfilingLog(rank, transports);
+    const std::string rankLog(GetRankProfilingLog(rank, transports));
     m_BP1Aggregator.WriteProfilingLog(fileName, rankLog);
 }
 
@@ -447,6 +447,9 @@ std::string BP1Writer::GetRankProfilingLog(
 // Explicit instantiation of only public templates
 
 #define declare_template_instantiation(T)                                      \
+    template BP1Writer::ResizeResult BP1Writer::ResizeBuffer(                  \
+        const Variable<T> &variable);                                          \
+                                                                               \
     template void BP1Writer::WriteVariablePayload(                             \
         const Variable<T> &variable) noexcept;                                 \
                                                                                \
diff --git a/source/adios2/utilities/format/bp1/BP1Writer.h b/source/adios2/utilities/format/bp1/BP1Writer.h
index afffaa187..821217336 100644
--- a/source/adios2/utilities/format/bp1/BP1Writer.h
+++ b/source/adios2/utilities/format/bp1/BP1Writer.h
@@ -43,51 +43,35 @@ public:
      */
     BP1Writer(MPI_Comm mpiComm, const bool debugMode = false);
 
-    /**
-     * Calculates the Process Index size in bytes according to the BP format,
-     * including list of method with no parameters (for now)
-     * @param name
-     * @param timeStepName
-     * @param numberOfTransports
-     * @return size of pg index
-     */
-    size_t GetProcessGroupIndexSize(const std::string name,
-                                    const std::string timeStepName,
-                                    const size_t numberOfTransports) const
-        noexcept;
+    virtual ~BP1Writer() = default;
 
     /**
      * Writes a process group index PGIndex and list of methods (from
-     * transports),
-     * done at Open or aggregation of new time step
-     * Version that operates on a single heap buffer and metadataset.
+     * transports). Done at Open or Advance.
      * @param isFortran
-     * @param name
+     * @param name group name, taking the rank
      * @param processID
      * @param transports
-     * @param buffer
-     * @param metadataSet
      */
     void WriteProcessGroupIndex(
         const bool isFortran, const std::string name, const uint32_t processID,
         const std::vector<std::shared_ptr<Transport>> &transports) noexcept;
 
     /**
-     * Returns the estimated variable index size
-     * @param group
-     * @param variableName
+     *
      * @param variable
-     * @param verbosity
-     * @return variable index size
+     * @return
+     * -1: allocation failed,
+     *  0: no allocation needed,
+     *  1: reallocation is sucessful
+     *  2: need a transport flush
      */
     template <class T>
-    size_t GetVariableIndexSize(const Variable<T> &variable) const noexcept;
+    ResizeResult ResizeBuffer(const Variable<T> &variable);
 
     /**
      * Write metadata for a given variable
      * @param variable
-     * @param heap
-     * @param metadataSet
      */
     template <class T>
     void WriteVariableMetadata(const Variable<T> &variable) noexcept;
@@ -96,16 +80,11 @@ public:
      * Expensive part this is only for heap buffers need to adapt to vector of
      * capsules
      * @param variable
-     * @param buffer
      */
     template <class T>
     void WriteVariablePayload(const Variable<T> &variable) noexcept;
 
-    /**
-     * Flattens data
-     * @param metadataSet
-     * @param buffer
-     */
+    /** Flattens data buffer */
     void Advance();
 
     /**
@@ -120,14 +99,34 @@ public:
     void Close(Transport &transport, bool &isFirstClose,
                const bool doAggregation) noexcept;
 
-    void DumpProfilingLogFile(
-        const std::string name, const unsigned int rank,
+    void WriteProfilingLogFile(
+        const std::string &name, const unsigned int rank,
         const std::vector<std::shared_ptr<Transport>> &transports) noexcept;
 
 private:
     /** BP format version */
     const std::uint8_t m_Version = 3;
 
+    /**
+     * Calculates the Process Index size in bytes according to the BP format,
+     * including list of method with no parameters (for now)
+     * @param name
+     * @param timeStepName
+     * @param numberOfTransports
+     * @return size of pg index
+     */
+    size_t GetProcessGroupIndexSize(const std::string name,
+                                    const std::string timeStepName,
+                                    const size_t numberOfTransports) const
+        noexcept;
+
+    /**
+     * Returns the estimated variable index size
+     * @param variable
+     */
+    template <class T>
+    size_t GetVariableIndexSize(const Variable<T> &variable) const noexcept;
+
     /**
      * Get variable statistics
      * @param variable
@@ -277,6 +276,9 @@ private:
 };
 
 #define declare_template_instantiation(T)                                      \
+    extern template BP1Writer::ResizeResult BP1Writer::ResizeBuffer(           \
+        const Variable<T> &variable) noexcept;                                 \
+                                                                               \
     extern template void BP1Writer::WriteVariablePayload(                      \
         const Variable<T> &variable) noexcept;                                 \
                                                                                \
diff --git a/source/adios2/utilities/format/bp1/BP1Writer.tcc b/source/adios2/utilities/format/bp1/BP1Writer.tcc
index dab05715c..50facc41a 100644
--- a/source/adios2/utilities/format/bp1/BP1Writer.tcc
+++ b/source/adios2/utilities/format/bp1/BP1Writer.tcc
@@ -19,40 +19,20 @@ namespace format
 
 // PUBLIC
 template <class T>
-size_t BP1Writer::GetVariableIndexSize(const Variable<T> &variable) const
-    noexcept
+BP1Writer::ResizeResult BP1Writer::ResizeBuffer(const Variable<T> &variable)
 {
-    // size_t indexSize = varEntryLength + memberID + lengthGroupName +
-    // groupName + lengthVariableName + lengthOfPath + path + datatype
-    size_t indexSize = 23; // without characteristics
-    indexSize += variable.m_Name.size();
-
-    // characteristics 3 and 4, check variable number of dimensions
-    const size_t dimensions = variable.DimensionsSize();
-    indexSize += 28 * dimensions; // 28 bytes per dimension
-    indexSize += 1;               // id
+    size_t variableData =
+        GetVariableIndexSize(variable) + variable.PayLoadSize();
+    size_t requiredCapacity = variableData + m_Heap.m_DataPosition;
 
-    // characteristics, offset + payload offset in data
-    indexSize += 2 * (1 + 8);
-    // characteristic 0, if scalar add value, for now only allowing string
-    if (dimensions == 1)
+    if (requiredCapacity > m_MaxBufferSize && m_MaxBufferSize > 0) // is set
     {
-        indexSize += sizeof(T);
-        indexSize += 1; // id
-        // must have an if here
-        indexSize += 2 + variable.m_Name.size();
-        indexSize += 1; // id
-    }
-
-    // characteristic statistics
-    if (m_Verbosity == 0) // default, only min and max
-    {
-        indexSize += 2 * (sizeof(T) + 1);
-        indexSize += 1 + 1; // id
+        if (m_Heap.GetDataSize() < m_MaxBufferSize)
+        {
+            m_Heap.ResizeData(m_MaxBufferSize);
+            return ResizeResult::FLUSH;
+        }
     }
-
-    return indexSize + 12; // extra 12 bytes in case of attributes
-    // need to add transform characteristics
 }
 
 template <class T>
@@ -88,6 +68,43 @@ void BP1Writer::WriteVariablePayload(const Variable<T> &variable) noexcept
 }
 
 // PRIVATE
+template <class T>
+size_t BP1Writer::GetVariableIndexSize(const Variable<T> &variable) const
+    noexcept
+{
+    // size_t indexSize = varEntryLength + memberID + lengthGroupName +
+    // groupName + lengthVariableName + lengthOfPath + path + datatype
+    size_t indexSize = 23; // without characteristics
+    indexSize += variable.m_Name.size();
+
+    // characteristics 3 and 4, check variable number of dimensions
+    const size_t dimensions = variable.DimensionsSize();
+    indexSize += 28 * dimensions; // 28 bytes per dimension
+    indexSize += 1;               // id
+
+    // characteristics, offset + payload offset in data
+    indexSize += 2 * (1 + 8);
+    // characteristic 0, if scalar add value, for now only allowing string
+    if (dimensions == 1)
+    {
+        indexSize += sizeof(T);
+        indexSize += 1; // id
+        // must have an if here
+        indexSize += 2 + variable.m_Name.size();
+        indexSize += 1; // id
+    }
+
+    // characteristic statistics
+    if (m_Verbosity == 0) // default, only min and max
+    {
+        indexSize += 2 * (sizeof(T) + 1);
+        indexSize += 1 + 1; // id
+    }
+
+    return indexSize + 12; // extra 12 bytes in case of attributes
+    // need to add transform characteristics
+}
+
 template <class T>
 BP1Writer::Stats<typename TypeInfo<T>::ValueType>
 BP1Writer::GetStats(const Variable<T> &variable) const noexcept
-- 
GitLab