Skip to content

Commit

Permalink
Merge pull request #18387 from tchaikov/wip-dmclock-client-info-ptr
Browse files Browse the repository at this point in the history 
osd,dmclock: use pointer to ClientInfo instead of a copy of it

Reviewed-by: J. Eric Ivancich <ivancich@redhat.com>
  • Loading branch information
@tchaikov
tchaikov committed Oct 20, 2017
2 parents e956c9a + 22aff9e commit d7244dc
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 72 deletions.
4 changes: 2 additions & 2 deletions src/dmclock/sim/src/test_dmclock_main.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ int main(int argc, char* argv[]) {
return i;
};

auto client_info_f = [=](const ClientId& c) -> test::dmc::ClientInfo {
return client_info[ret_client_group_f(c)];
auto client_info_f = [=](const ClientId& c) -> const test::dmc::ClientInfo* {
return &client_info[ret_client_group_f(c)];
};

auto client_disp_filter = [=] (const ClientId& i) -> bool {
Expand Down
26 changes: 16 additions & 10 deletions src/dmclock/src/dmclock_server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,14 +305,14 @@ namespace crimson {

public:

ClientInfo info;
const ClientInfo* info;
bool idle;
Counter last_tick;
uint32_t cur_rho;
uint32_t cur_delta;

ClientRec(C _client,
const ClientInfo& _info,
const ClientInfo* _info,
Counter current_tick) :
client(_client),
prev_tag(0.0, 0.0, 0.0, TimeZero),
Expand Down Expand Up @@ -474,7 +474,7 @@ namespace crimson {


// a function that can be called to look up client information
using ClientInfoFunc = std::function<ClientInfo(const C&)>;
using ClientInfoFunc = std::function<const ClientInfo*(const C&)>;


bool empty() const {
Expand Down Expand Up @@ -788,7 +788,7 @@ namespace crimson {
}


inline const ClientInfo get_cli_info(ClientRec& client) const {
inline const ClientInfo* get_cli_info(ClientRec& client) const {
if (is_dynamic_cli_info_f) {
client.info = client_info_f(client.client);
}
Expand All @@ -812,7 +812,7 @@ namespace crimson {
if (client_map.end() != client_it) {
temp_client = &(*client_it->second); // address of obj of shared_ptr
} else {
ClientInfo info = client_info_f(client_id);
const ClientInfo* info = client_info_f(client_id);
ClientRecRef client_rec =
std::make_shared<ClientRec>(client_id, info, tick);
resv_heap.push(client_rec);
Expand Down Expand Up @@ -882,8 +882,10 @@ namespace crimson {
RequestTag tag(0, 0, 0, time);

if (!client.has_request()) {
const ClientInfo* client_info = get_cli_info(client);
assert(client_info);
tag = RequestTag(client.get_req_tag(),
get_cli_info(client),
*client_info,
req_params,
time,
cost,
Expand All @@ -893,8 +895,10 @@ namespace crimson {
client.update_req_tag(tag, tick);
}
#else
const ClientInfo* client_info = get_cli_info(client);
assert(client_info);
RequestTag tag(client.get_req_tag(),
get_cli_info(client),
*client_info,
req_params,
time,
cost,
Expand Down Expand Up @@ -949,7 +953,9 @@ namespace crimson {
#ifndef DO_NOT_DELAY_TAG_CALC
if (top.has_request()) {
ClientReq& next_first = top.next_request();
next_first.tag = RequestTag(tag, get_cli_info(top),
const ClientInfo* client_info = get_cli_info(top);
assert(client_info);
next_first.tag = RequestTag(tag, *client_info,
top.cur_delta, top.cur_rho,
next_first.tag.arrival,
0.0, anticipation_timeout);
Expand All @@ -974,15 +980,15 @@ namespace crimson {
// data_mtx should be held when called
void reduce_reservation_tags(ClientRec& client) {
for (auto& r : client.requests) {
r.tag.reservation -= client.info.reservation_inv;
r.tag.reservation -= client.info->reservation_inv;

#ifndef DO_NOT_DELAY_TAG_CALC
// reduce only for front tag. because next tags' value are invalid
break;
#endif
}
// don't forget to update previous tag
client.prev_tag.reservation -= client.info.reservation_inv;
client.prev_tag.reservation -= client.info->reservation_inv;
resv_heap.promote(client);
}

Expand Down
88 changes: 46 additions & 42 deletions src/dmclock/test/test_dmclock_server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ namespace crimson {
dmc::ClientInfo ci1(reservation, weight, 0.0);
dmc::ClientInfo ci2(reservation, weight, 1.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
if (client1 == c) return ci1;
else if (client2 == c) return ci2;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
if (client1 == c) return &ci1;
else if (client2 == c) return &ci2;
else {
ADD_FAILURE() << "got request from neither of two clients";
return ci1; // must return
return nullptr;
}
};

Expand Down Expand Up @@ -91,7 +91,9 @@ namespace crimson {
double reservation = 100.0;

dmc::ClientInfo ci(reservation, 1.0, 0.0);
auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo { return ci; };
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &ci;
};
auto server_ready_f = [] () -> bool { return true; };
auto submit_req_f = [] (const ClientId& c,
std::unique_ptr<Request> req,
Expand Down Expand Up @@ -186,7 +188,9 @@ namespace crimson {
dmc::ClientInfo ci(1.0, 0.0, 0.0);
Queue pq;

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo { return ci; };
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &ci;
};
auto server_ready_f = [] () -> bool { return true; };
auto submit_req_f = [&] (const ClientId& c,
std::unique_ptr<Request> req,
Expand Down Expand Up @@ -241,8 +245,8 @@ namespace crimson {

dmc::ClientInfo info1(0.0, 1.0, 0.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
return info1;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &info1;
};

Queue pq(client_info_f, true);
Expand Down Expand Up @@ -309,8 +313,8 @@ namespace crimson {

dmc::ClientInfo info1(0.0, 1.0, 0.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
return info1;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &info1;
};

Queue pq(client_info_f, true);
Expand Down Expand Up @@ -391,8 +395,8 @@ namespace crimson {

dmc::ClientInfo info1(0.0, 1.0, 0.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
return info1;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &info1;
};

Queue pq(client_info_f, true);
Expand Down Expand Up @@ -473,8 +477,8 @@ namespace crimson {

dmc::ClientInfo info1(0.0, 1.0, 0.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
return info1;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &info1;
};

Queue pq(client_info_f, true);
Expand Down Expand Up @@ -542,12 +546,12 @@ namespace crimson {

QueueRef pq;

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
if (client1 == c) return info1;
else if (client2 == c) return info2;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
if (client1 == c) return &info1;
else if (client2 == c) return &info2;
else {
ADD_FAILURE() << "client info looked up for non-existant client";
return info1;
return nullptr;
}
};

Expand Down Expand Up @@ -595,12 +599,12 @@ namespace crimson {
dmc::ClientInfo info1(2.0, 0.0, 0.0);
dmc::ClientInfo info2(1.0, 0.0, 0.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
if (client1 == c) return info1;
else if (client2 == c) return info2;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
if (client1 == c) return &info1;
else if (client2 == c) return &info2;
else {
ADD_FAILURE() << "client info looked up for non-existant client";
return info1;
return nullptr;
}
};

Expand Down Expand Up @@ -652,12 +656,12 @@ namespace crimson {

QueueRef pq;

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
if (client1 == c) return info1;
else if (client2 == c) return info2;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
if (client1 == c) return &info1;
else if (client2 == c) return &info2;
else {
ADD_FAILURE() << "client info looked up for non-existant client";
return info1;
return nullptr;
}
};

Expand Down Expand Up @@ -749,12 +753,12 @@ namespace crimson {

QueueRef pq;

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
if (client1 == c) return info1[cli_info_group];
else if (client2 == c) return info2[cli_info_group];
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
if (client1 == c) return &info1[cli_info_group];
else if (client2 == c) return &info2[cli_info_group];
else {
ADD_FAILURE() << "client info looked up for non-existant client";
return info1[0];
return nullptr;
}
};

Expand Down Expand Up @@ -838,12 +842,12 @@ namespace crimson {
dmc::ClientInfo info1(1.0, 0.0, 0.0);
dmc::ClientInfo info2(1.0, 0.0, 0.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
if (client1 == c) return info1;
else if (client2 == c) return info2;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
if (client1 == c) return &info1;
else if (client2 == c) return &info2;
else {
ADD_FAILURE() << "client info looked up for non-existant client";
return info1;
return nullptr;
}
};

Expand Down Expand Up @@ -898,8 +902,8 @@ namespace crimson {

dmc::ClientInfo info(1.0, 1.0, 1.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
return info;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &info;
};

QueueRef pq(new Queue(client_info_f, false));
Expand All @@ -925,8 +929,8 @@ namespace crimson {

dmc::ClientInfo info(1.0, 0.0, 1.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
return info;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &info;
};

QueueRef pq(new Queue(client_info_f, false));
Expand Down Expand Up @@ -956,8 +960,8 @@ namespace crimson {

dmc::ClientInfo info(0.0, 1.0, 1.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
return info;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &info;
};

QueueRef pq(new Queue(client_info_f, true));
Expand Down Expand Up @@ -987,8 +991,8 @@ namespace crimson {

dmc::ClientInfo info(1.0, 0.0, 1.0);

auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo {
return info;
auto client_info_f = [&] (ClientId c) -> const dmc::ClientInfo* {
return &info;
};

QueueRef pq(new Queue(client_info_f, true));
Expand Down
14 changes: 7 additions & 7 deletions src/osd/mClockClientQueue.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,24 +58,24 @@ namespace ceph {
}


dmc::ClientInfo
const dmc::ClientInfo*
mClockClientQueue::op_class_client_info_f(
const mClockClientQueue::InnerClient& client)
{
switch(client.second) {
case osd_op_type_t::client_op:
return mclock_op_tags->client_op;
return &mclock_op_tags->client_op;
case osd_op_type_t::osd_subop:
return mclock_op_tags->osd_subop;
return &mclock_op_tags->osd_subop;
case osd_op_type_t::bg_snaptrim:
return mclock_op_tags->snaptrim;
return &mclock_op_tags->snaptrim;
case osd_op_type_t::bg_recovery:
return mclock_op_tags->recov;
return &mclock_op_tags->recov;
case osd_op_type_t::bg_scrub:
return mclock_op_tags->scrub;
return &mclock_op_tags->scrub;
default:
assert(0);
return dmc::ClientInfo(-1, -1, -1);
return nullptr;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/osd/mClockClientQueue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ namespace ceph {

mClockClientQueue(CephContext *cct);

static crimson::dmclock::ClientInfo
static const crimson::dmclock::ClientInfo*
op_class_client_info_f(const InnerClient& client);

inline unsigned length() const override final {
Expand Down
14 changes: 7 additions & 7 deletions src/osd/mClockOpClassQueue.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,22 @@ namespace ceph {
}


dmc::ClientInfo
const dmc::ClientInfo*
mClockOpClassQueue::op_class_client_info_f(const osd_op_type_t& op_type) {
switch(op_type) {
case osd_op_type_t::client_op:
return mclock_op_tags->client_op;
return &mclock_op_tags->client_op;
case osd_op_type_t::osd_subop:
return mclock_op_tags->osd_subop;
return &mclock_op_tags->osd_subop;
case osd_op_type_t::bg_snaptrim:
return mclock_op_tags->snaptrim;
return &mclock_op_tags->snaptrim;
case osd_op_type_t::bg_recovery:
return mclock_op_tags->recov;
return &mclock_op_tags->recov;
case osd_op_type_t::bg_scrub:
return mclock_op_tags->scrub;
return &mclock_op_tags->scrub;
default:
assert(0);
return dmc::ClientInfo(-1, -1, -1);
return nullptr;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/osd/mClockOpClassQueue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace ceph {

mClockOpClassQueue(CephContext *cct);

static crimson::dmclock::ClientInfo
static const crimson::dmclock::ClientInfo*
op_class_client_info_f(const osd_op_type_t& op_type);

inline unsigned length() const override final {
Expand Down

0 comments on commit d7244dc

Please sign in to comment.