New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osd/OSDMap: make pg_temp more efficient #15291

Merged
merged 3 commits into from May 31, 2017

Conversation

Projects
None yet
2 participants
@liewegas
Member

liewegas commented May 25, 2017

On an OSDMap with 117012 remapped pgs, we go from 17.2MB -> 6.0MB RAM, and we are 3.5x faster at decoding.

We are also 3.7x faster at mapping all PGs in the osdmap (~50% of which are remapped).

void clear() {
pg_temp.clear();
}
void set(pg_t pgid, const mempool::osdmap::vector<int32_t>& v) {

This comment has been minimized.

@tchaikov

tchaikov May 26, 2017

Contributor

nit, could add a set(mempool::osdmap::vector<int32_t>&& v), so we can use the move ctor if v is a rvalue reference.

void set(pg_t pgid, const mempool::osdmap::vector<int32_t>& v) {
pg_temp[pgid] = v;
}
vector<int32_t> get(pg_t pgid) {

This comment has been minimized.

@tchaikov

tchaikov May 26, 2017

Contributor

should return a const ref.

void decode(bufferlist::iterator& p) {
data.clear();
map.clear();
int32_t n;

This comment has been minimized.

@tchaikov

tchaikov May 26, 2017

Contributor

i don't understand why we are encoding n as an uint32_t, while decoding it as an int32_t?

This comment has been minimized.

@tchaikov

tchaikov May 26, 2017

Contributor

will continue the review later on.

@liewegas

This comment has been minimized.

Member

liewegas commented May 26, 2017

http://pulpito.ceph.com/sage-2017-05-26_06:56:21-rados-wip-sage-testing---basic-smithi/
tests out ok, although i'm seeing a (possibly related) crash from mon on bigbang.

friend bool operator==(const PGTempMap& l, const PGTempMap& r) {
return
l.map.size() == r.map.size() &&
l.data.contents_equal(r.data);

This comment has been minimized.

@tchaikov

tchaikov May 26, 2017

Contributor

no need to check l.map.size() == r.map.size(), i think. l.data.contents_equal(r.data) implies l.map.size() == r.map.size().

::encode(v, data);
map[pgid] = (int32_t*)(data.back().end_c_str()) - (1 + v.size());
}
mempool::osdmap::vector<int32_t> get(pg_t pgid) {

This comment has been minimized.

@tchaikov

tchaikov May 26, 2017

Contributor

return const ref

This comment has been minimized.

@tchaikov

tchaikov Jun 1, 2017

Contributor

i was wrong. it's a local/temp var.

map.clear();
data.clear();
}
void set(pg_t pgid, const mempool::osdmap::vector<int32_t>& v) {

This comment has been minimized.

@tchaikov

tchaikov May 26, 2017

Contributor

could have a void set(pg_t pgid, mempool::osdmap::vector<int32_t>&& v) if possible.

liewegas added some commits May 24, 2017

osd/OSDMap: encapsulate pg_temp
Signed-off-by: Sage Weil <sage@redhat.com>
unittest_osdmap: parse env
Signed-off-by: Sage Weil <sage@redhat.com>
osd/OSDMap: more efficient PGMapTemp
Use a flat_map with pointers into a buffer with the actual data.  For a
decoded mapping, we have just two allocations (one for flat_map and one
for the encoded buffer).

This can get slow if you make lots of incremental changes after the fact
since flat_map is not efficient for modifications at large sizes.  :/

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

This comment has been minimized.

Member

liewegas commented May 26, 2017

passed a qa run, and looks ok on bigbang.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 27, 2017

@liewegas i think i've finished the review.

@liewegas

This comment has been minimized.

Member

liewegas commented May 30, 2017

oops i forget to push my update

@tchaikov tchaikov referenced this pull request May 31, 2017

Merged

mon,mgr: extricate PGmap from monitor #15073

11 of 11 tasks complete

@liewegas liewegas merged commit b19352e into ceph:master May 31, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@liewegas liewegas deleted the liewegas:wip-osdmap-pgtemp branch May 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment