refs #27: Fix hidden bugs in FixedRing via unit tests.

include/nba/core/queue.hh

@@ -6,6 +6,8 @@
 
 #include <rte_malloc.h>
 
+namespace nba {
+
 template<typename T, T const default_value, size_t max_size>
 class FixedArray;
 
@@ -157,20 +159,20 @@ class FixedRing
 
 public:
     FixedRing()
-        : v_(nullptr), push_idx(0), pop_idx(0), count(0), max_size(0)
+        : v_(nullptr), is_external(false), push_idx(0), pop_idx(0), count(0), max_size(0)
     {
         /* Default constructor. You must explicitly call init() to use the instance. */
     }
 
     FixedRing(size_t max_size, int numa_node = 0, T *xmem = nullptr)
-        : v_(nullptr), push_idx(0), pop_idx(0), count(0), max_size(max_size)
+        : v_(nullptr), is_external(false), push_idx(0), pop_idx(0), count(0), max_size(max_size)
     {
         init(max_size, numa_node, xmem);
     }
 
     virtual ~FixedRing()
     {
-        if (v_ != nullptr)
+        if (v_ != nullptr && !is_external)
             rte_free(v_);
     }
 
@@ -181,8 +183,10 @@ public:
         this->max_size = max_size;
         if (xmem == nullptr) {
             v_ = (T*) rte_malloc_socket("fixedring", sizeof(T) * max_size, 64, numa_node);
+            is_external = false;
         } else {
             v_ = xmem;
+            is_external = true;
         }
         assert(v_ != nullptr);
     }
@@ -198,8 +202,9 @@ public:
     void push_front(T t)
     {
         assert(count < max_size);
-        v_[pop_idx - 1] = t;
-        pop_idx = (pop_idx - 1) % max_size;
+        size_t new_pop_idx = (max_size + pop_idx - 1) % max_size;
+        v_[new_pop_idx] = t;
+        pop_idx = new_pop_idx;
         count ++;
     }
 
@@ -258,12 +263,14 @@ public:
 
 private:
     T *v_;
+    bool is_external;
     size_t push_idx;
     size_t pop_idx;
     size_t count;
     size_t max_size;
 };
 
+} /* endns(nba) */
 
 #endif
 // vim: ts=8 sts=4 sw=4 et

src/lib/offloadtask.cc

@@ -1,4 +1,5 @@
 #include <nba/core/intrinsic.hh>
+#include <nba/core/enumerate.hh>
 #include <nba/framework/logging.hh>
 #include <nba/framework/datablock.hh>
 #include <nba/framework/elementgraph.hh>
@@ -217,8 +218,9 @@ bool OffloadTask::copy_h2d()
         dbarg_h->total_item_count_in  = 0;
         dbarg_h->total_item_count_out = 0;
 
-        int b = 0;
-        for (PacketBatch *batch : batches) {
+        for (auto&& p : enumerate(batches)) {
+            size_t b = p.first;
+            PacketBatch *&batch = p.second;
             assert(batch->datablock_states != nullptr);
             struct datablock_tracker *t = &batch->datablock_states[dbid];
 
@@ -253,7 +255,6 @@ bool OffloadTask::copy_h2d()
             dbarg_h->batches[b].buffer_bases_out = t->dev_out_ptr.ptr; // FIXME: generalize to CL?
             dbarg_h->batches[b].item_count_out   = t->out_count;
             dbarg_h->total_item_count_out       += t->out_count;
-            b++;
         } /* endfor(batches) */
     } /* endfor(dbid) */
     return true;

tests/test_core_fixedarray.cc

@@ -0,0 +1,137 @@
+#include <nba/core/queue.hh>
+#include <nba/core/enumerate.hh>
+#include <vector>
+#include <string>
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace nba;
+
+TEST(CoreFixedArrayTest, Initialization) {
+    FixedArray<int, -1, 3> A;
+    A.push_back(1);
+    EXPECT_EQ(1, A[0]);
+    EXPECT_EQ(-1, A[1]);
+    EXPECT_EQ(-1, A[2]);
+    A.push_back(2);
+    EXPECT_EQ(1, A[0]);
+    EXPECT_EQ(2, A[1]);
+    EXPECT_EQ(-1, A[2]);
+    A.push_back(3);
+    EXPECT_EQ(1, A[0]);
+    EXPECT_EQ(2, A[1]);
+    EXPECT_EQ(3, A[2]);
+}
+
+TEST(CoreFixedRingTest, PushBack) {
+    int buf[3];
+    FixedRing<int, -1> A;
+    A.init(3, 0, buf);
+    EXPECT_EQ(0, A.size());
+    EXPECT_TRUE(A.empty());
+    A.push_back(1);
+    EXPECT_EQ(1, A.front());
+    EXPECT_EQ(1, A[0]);
+    EXPECT_EQ(1, A.size());
+    EXPECT_FALSE(A.empty());
+    A.push_back(2);
+    EXPECT_EQ(1, A.front());
+    EXPECT_EQ(1, A[0]);
+    EXPECT_EQ(2, A[1]);
+    EXPECT_EQ(2, A.size());
+    A.push_back(3);
+    EXPECT_EQ(1, A.front());
+    EXPECT_EQ(1, A[0]);
+    EXPECT_EQ(2, A[1]);
+    EXPECT_EQ(3, A[2]);
+    EXPECT_EQ(3, A.size());
+}
+
+TEST(CoreFixedRingTest, PushFront) {
+    int buf[3];
+    FixedRing<int, -1> B;
+    B.init(3, 0, buf);
+    EXPECT_EQ(0, B.size());
+    EXPECT_TRUE(B.empty());
+    B.push_front(1);
+    EXPECT_EQ(1, B.front());
+    EXPECT_EQ(1, B[0]);
+    EXPECT_EQ(1, B.size());
+    EXPECT_FALSE(B.empty());
+    B.push_front(2);
+    EXPECT_EQ(2, B.front());
+    EXPECT_EQ(2, B[0]);
+    EXPECT_EQ(1, B[1]);
+    EXPECT_EQ(2, B.size());
+    B.push_front(3);
+    EXPECT_EQ(3, B.front());
+    EXPECT_EQ(3, B[0]);
+    EXPECT_EQ(2, B[1]);
+    EXPECT_EQ(1, B[2]);
+    EXPECT_EQ(3, B.size());
+    int correctB[] = {3, 2, 1};
+    for (auto&& p : enumerate(B))
+        EXPECT_EQ(correctB[p.first], p.second);
+}
+
+TEST(CoreFixedRingTest, MixedPushBackFront) {
+    int buf[3];
+    FixedRing<int, -1> C;
+    C.init(3, 0, buf);
+    EXPECT_EQ(0, C.size());
+    EXPECT_TRUE(C.empty());
+    C.push_back(1);
+    C.push_back(2);
+    C.push_front(3);
+    EXPECT_EQ(3, C[0]);
+    EXPECT_EQ(1, C[1]);
+    EXPECT_EQ(2, C[2]);
+    EXPECT_EQ(3, C.size());
+    EXPECT_FALSE(C.empty());
+    int correctC[] = {3, 1, 2};
+    for (auto&& p : enumerate(C))
+        EXPECT_EQ(correctC[p.first], p.second);
+}
+
+TEST(CoreFixedRingTest, MixedPushPop) {
+    int buf[3];
+    FixedRing<int, -1> D;
+    D.init(3, 0, buf);
+    EXPECT_EQ(0, D.size());
+    EXPECT_TRUE(D.empty());
+    D.push_back(1);
+    D.pop_front();
+    EXPECT_EQ(0, D.size());
+    EXPECT_TRUE(D.empty());
+    D.push_front(1);
+    D.pop_front();
+    EXPECT_EQ(0, D.size());
+    EXPECT_TRUE(D.empty());
+    for (int x = 0; x < 20; x++) {
+        // Unbalance push_back() and push_front()
+        // so that internal pop_idx/push_idx goes
+        // beyond the boundaries.
+        D.push_back(2);
+        EXPECT_EQ(2, D.front());
+        D.push_front(3);
+        EXPECT_EQ(3, D.front());
+        D.push_back(4);
+        EXPECT_EQ(3, D.front());
+        int correctD[] = {3, 2, 4};
+        for (auto&& p : enumerate(D))
+            EXPECT_EQ(correctD[p.first], p.second);
+        D.pop_front();
+        EXPECT_EQ(2, D.size());
+        EXPECT_FALSE(D.empty());
+        EXPECT_EQ(2, D.front());
+        D.pop_front();
+        EXPECT_EQ(1, D.size());
+        EXPECT_FALSE(D.empty());
+        EXPECT_EQ(4, D.front());
+        D.pop_front();
+        EXPECT_EQ(0, D.size());
+        EXPECT_TRUE(D.empty());
+    }
+}
+
+// vim: ts=8 sts=4 sw=4 et