Skip to content

Commit

Permalink
os/bluestore: simplify BufferSpace writing_map / list
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
liewegas committed Oct 5, 2016
1 parent 6bbac0e commit 0932d39
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 44 deletions.
37 changes: 18 additions & 19 deletions src/os/bluestore/BlueStore.cc
Expand Up @@ -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");
Expand Down
32 changes: 7 additions & 25 deletions src/os/bluestore/BlueStore.h
Expand Up @@ -208,22 +208,10 @@ class BlueStore : public ObjectStore,
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) {
Expand All @@ -232,7 +220,7 @@ class BlueStore : public ObjectStore,
}
~BufferSpace() {
assert(buffer_map.empty());
assert(writing_map.empty());
assert(writing.empty());
if (cache) {
cache->rm_blob();
}
Expand All @@ -242,7 +230,7 @@ class BlueStore : public ObjectStore,
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);
}
Expand All @@ -254,13 +242,7 @@ class BlueStore : public ObjectStore,
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());
}
Expand Down

0 comments on commit 0932d39

Please sign in to comment.