os/bluestore: simplify BufferSpace writing_map / list

We were tracking buffers by seq.  In reality, though, we
won't have very many buffers that have in flight IO to
the same blob at the same time, so we can get by with
just a list here.  We can rely on the fact that the
list will be sorted (seq's increase, and we put new
buffers at the end).

This saves us a memory allocation.  No change in size of
BufferSpace and Blob (std::list -> intrusive::list).

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 1e963e3e523748479907ab53f3caec92dbcc6748..dd88ac57c1f3825f51c58d218c1599ff38b73aa4 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1043,29 +1043,28 @@ void BlueStore::BufferSpace::finish_write(uint64_t seq)
 {
   std::lock_guard<std::recursive_mutex> l(cache->lock);
 
-  auto i = writing_map.begin();
-  while (i != writing_map.end()) {
-    if (i->first > seq)
+  auto i = writing.begin();
+  while (i != writing.end()) {
+    if (i->seq > seq) {
       break;
+    }
+    if (i->seq < seq) {
+      ++i;
+      continue;
+    }
 
-    auto l = i->second.begin();
-    while (l != i->second.end()) {
-      Buffer *b = &*l;
-      dout(20) << __func__ << " " << *b << dendl;
-      assert(b->is_writing());
+    Buffer *b = &*i;
+    dout(20) << __func__ << " " << *b << dendl;
+    assert(b->is_writing());
 
-      if (b->flags & Buffer::FLAG_NOCACHE) {
-        i->second.erase(l++);
-        buffer_map.erase(b->offset);
-      } else {
-        b->state = Buffer::STATE_CLEAN;
-        i->second.erase(l++);
-        cache->_add_buffer(b, 1, nullptr);
-      }
+    if (b->flags & Buffer::FLAG_NOCACHE) {
+      writing.erase(i++);
+      buffer_map.erase(b->offset);
+    } else {
+      b->state = Buffer::STATE_CLEAN;
+      writing.erase(i++);
+      cache->_add_buffer(b, 1, nullptr);
     }
-
-    assert(i->second.empty());
-    writing_map.erase(i++);
   }
 
   cache->_audit("finish_write end");

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 416821389f2d77dee5cb3516b026cb1ea14ac019..ca9035b0a87b77be4a818d08189f2dc3177b631a 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -208,22 +208,10 @@ public:
     map<uint64_t,std::unique_ptr<Buffer>> buffer_map;
     Cache *cache;
 
-    // we use a list here instead of std::map because it uses less memory and
-    // we expect this to be very small (very few IOs in flight to the same
-    // Blob at the same time).
-    list<pair<uint64_t,state_list_t>> writing_map;
-
-    list<pair<uint64_t,state_list_t>>::iterator get_writing_map(uint64_t seq) {
-      for (auto p = writing_map.begin(); p != writing_map.end(); ++p) {
-       if (p->first == seq) {
-         return p;
-       }
-      }
-      writing_map.push_back(make_pair(seq, state_list_t()));
-      auto p = writing_map.end();
-      --p;
-      return p;
-    }
+    // we use a bare intrusive list here instead of std::map because
+    // it uses less memory and we expect this to be very small (very
+    // few IOs in flight to the same Blob at the same time).
+    state_list_t writing;   ///< writing buffers, sorted by seq, ascending
 
     BufferSpace(Cache *c) : cache(c) {
       if (cache) {
@@ -232,7 +220,7 @@ public:
     }
     ~BufferSpace() {
       assert(buffer_map.empty());
-      assert(writing_map.empty());
+      assert(writing.empty());
       if (cache) {
        cache->rm_blob();
       }
@@ -242,7 +230,7 @@ public:
       cache->_audit("_add_buffer start");
       buffer_map[b->offset].reset(b);
       if (b->is_writing()) {
-        get_writing_map(b->seq)->second.push_back(*b);
+        writing.push_back(*b);
       } else {
        cache->_add_buffer(b, level, near);
       }
@@ -254,13 +242,7 @@ public:
     void _rm_buffer(map<uint64_t,std::unique_ptr<Buffer>>::iterator p) {
       cache->_audit("_rm_buffer start");
       if (p->second->is_writing()) {
-        uint64_t seq = (*p->second.get()).seq;
-        auto it = get_writing_map(seq);
-        assert(!it->second.empty());
-        it->second.erase(it->second.iterator_to(*p->second));
-        if (it->second.empty()) {
-          writing_map.erase(it);
-       }
+        writing.erase(writing.iterator_to(*p->second));
       } else {
        cache->_rm_buffer(p->second.get());
       }