ErasureCode: get rid of extra copy when encoding

The substr_of bufferlist method is used to reduce the copies to the
strict minimum. It is both able to split bufferptr when they are larger
than a chunk and to concatenate them when they are smaller.

Signed-off-by: Loic Dachary <loic@dachary.org>

diff --git a/src/osd/ErasureCodePluginJerasure/ErasureCodeJerasure.cc b/src/osd/ErasureCodePluginJerasure/ErasureCodeJerasure.cc
index 2368cc6c711619ed5eb4955f8b66f60a91a5c059..f2be1ed06e711a1cc142210a4260963470349dd1 100644 (file)
--- a/src/osd/ErasureCodePluginJerasure/ErasureCodeJerasure.cc
+++ b/src/osd/ErasureCodePluginJerasure/ErasureCodeJerasure.cc
@@ -78,32 +78,26 @@ int ErasureCodeJerasure::encode(const set<int> &want_to_encode,
 {
   unsigned alignment = get_alignment();
   unsigned tail = in.length() % alignment;
-  unsigned in_length = in.length() + ( tail ?  ( alignment - tail ) : 0 );
+  unsigned padded_length = in.length() + ( tail ?  ( alignment - tail ) : 0 );
   dout(10) << "encode adjusted buffer length from " << in.length()
-          << " to " << in_length << dendl;
-  assert(in_length % k == 0);
-  unsigned blocksize = in_length / k;
+          << " to " << padded_length << dendl;
+  assert(padded_length % k == 0);
+  unsigned blocksize = padded_length / k;
   unsigned length = blocksize * ( k + m );
   bufferlist out(in);
   bufferptr pad(length - in.length());
-  pad.zero(0, k);
+  pad.zero(0, padded_length - in.length());
   out.push_back(pad);
-  char *p = out.c_str();
-  char *data[k];
-  for (int i = 0; i < k; i++) {
-    data[i] = p + i * blocksize;
-  }
-  char *coding[m];
-  for (int i = 0; i < m; i++) {
-    coding[i] = p + ( k + i ) * blocksize;
+  char *chunks[k + m];
+  for (int i = 0; i < k + m; i++) {
+    bufferlist &chunk = (*encoded)[i];
+    chunk.substr_of(out, i * blocksize, blocksize);
+    chunks[i] = chunk.c_str();
   }
-  jerasure_encode(data, coding, blocksize);
-  const bufferptr ptr = out.buffers().front();
-  for (set<int>::iterator j = want_to_encode.begin();
-       j != want_to_encode.end();
-       j++) {
-    bufferptr chunk(ptr, (*j) * blocksize, blocksize);
-    (*encoded)[*j].push_front(chunk);
+  jerasure_encode(&chunks[0], &chunks[k], blocksize);
+  for (int i = 0; i < k + m; i++) {
+    if (want_to_encode.count(i) == 0)
+      encoded->erase(i);
   }
   return 0;
 }

diff --git a/src/test/osd/TestErasureCodeJerasure.cc b/src/test/osd/TestErasureCodeJerasure.cc
index 2e601af38b264b6fb40c8f6d3eff8e74189f7af7..a51cb853c8676f0c623900c5bd379e102d310271 100644 (file)
--- a/src/test/osd/TestErasureCodeJerasure.cc
+++ b/src/test/osd/TestErasureCodeJerasure.cc
@@ -47,9 +47,19 @@ TYPED_TEST(ErasureCodeTest, encode_decode)
   parameters["erasure-code-packetsize"] = "8";
   jerasure.init(parameters);
 
+#define LARGE_ENOUGH 2048
+  bufferptr in_ptr(LARGE_ENOUGH);
+  in_ptr.zero();
+  in_ptr.set_length(0);
+  const char *payload =
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
+  in_ptr.append(payload, strlen(payload));
   bufferlist in;
-  for (int i = 0; i < 5; i++)
-    in.append("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789");
+  in.push_front(in_ptr);
   int want_to_encode[] = { 0, 1, 2, 3 };
   map<int, bufferlist> encoded;
   EXPECT_EQ(0, jerasure.encode(set<int>(want_to_encode, want_to_encode+4),
@@ -195,6 +205,84 @@ TYPED_TEST(ErasureCodeTest, minimum_to_decode)
   }
 }
 
+TEST(ErasureCodeTest, encode)
+{
+  ErasureCodeJerasureReedSolomonVandermonde jerasure;
+  map<std::string,std::string> parameters;
+  parameters["erasure-code-k"] = "2";
+  parameters["erasure-code-m"] = "2";
+  parameters["erasure-code-w"] = "8";
+  jerasure.init(parameters);
+
+  unsigned alignment = jerasure.get_alignment();
+  {
+    //
+    // When the input bufferlist is perfectly aligned, it is 
+    // pointed to unmodified by the returned encoded chunks.
+    //
+    bufferlist in;
+    map<int,bufferlist> encoded;
+    int want_to_encode[] = { 0, 1, 2, 3 };
+    in.append(string(alignment * 2, 'X'));
+    EXPECT_EQ(alignment * 2, in.length());
+    EXPECT_EQ(0, jerasure.encode(set<int>(want_to_encode, want_to_encode+4),
+                                in,
+                                &encoded));
+    EXPECT_EQ(4u, encoded.size());
+    for(int i = 0; i < 4; i++)
+      EXPECT_EQ(alignment, encoded[i].length());
+    EXPECT_EQ(in.c_str(), encoded[0].c_str());
+    EXPECT_EQ(in.c_str() + alignment, encoded[1].c_str());
+  }
+
+  {
+    //
+    // When the input bufferlist needs to be padded because
+    // it is not properly aligned, it is padded with zeros.
+    // The beginning of the input bufferlist is pointed to 
+    // unmodified by the returned encoded chunk, only the 
+    // trailing chunk is allocated and copied.
+    //
+    bufferlist in;
+    map<int,bufferlist> encoded;
+    int want_to_encode[] = { 0, 1, 2, 3 };
+    int trail_length = 10;
+    in.append(string(alignment + trail_length, 'X'));
+    EXPECT_EQ(0, jerasure.encode(set<int>(want_to_encode, want_to_encode+4),
+                                in,
+                                &encoded));
+    EXPECT_EQ(4u, encoded.size());
+    for(int i = 0; i < 4; i++)
+      EXPECT_EQ(alignment, encoded[i].length());
+    EXPECT_EQ(in.c_str(), encoded[0].c_str());
+    EXPECT_NE(in.c_str() + alignment, encoded[1].c_str());
+    char *last_chunk = encoded[1].c_str();
+    EXPECT_EQ('X', last_chunk[0]);
+    EXPECT_EQ('\0', last_chunk[trail_length]);
+  }
+
+  {
+    //
+    // When only the first chunk is required, the encoded map only
+    // contains the first chunk. Although the jerasure encode
+    // internally allocated a buffer because of padding requirements
+    // and also computes the coding chunks, they are released before
+    // the return of the method, as shown when running the tests thru
+    // valgrind that shows there is no leak.
+    //
+    bufferlist in;
+    map<int,bufferlist> encoded;
+    set<int> want_to_encode;
+    want_to_encode.insert(0);
+    int trail_length = 10;
+    in.append(string(alignment + trail_length, 'X'));
+    EXPECT_EQ(0, jerasure.encode(want_to_encode, in, &encoded));
+    EXPECT_EQ(1u, encoded.size());
+    EXPECT_EQ(alignment, encoded[0].length());
+    EXPECT_EQ(in.c_str(), encoded[0].c_str());
+  }
+}
+
 int main(int argc, char **argv)
 {
   vector<const char*> args;