Skip to content

Commit f82ab1c

Browse files
MaxKellermannMaxKellermann
committed
mgr, mon, osdc: pass complex parameters by rvalue reference
A non-trivial type is always passed by reference, even if the parameter is declared as a value; in that case, the compiler must allocate stack space and move the value there, and then really passes a reference to this stack allocation. This only adds overhead for no reason. A side effect of this API change is that all callers that accidently created temporary copies are now revealed by throwing a compiler error. This commit fixes all callers and reduces a good amount of useless overhead by wrapping those parameters in std::move() instead of copying temporaries. More temporaries are avoided by emplacing the std::string&& into the std::vector. Signed-off-by: Max Kellermann <[email protected]>
1 parent 7cf33c4 commit f82ab1c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+541
-675
lines changed

‎src/common/io_exerciser/RadosIo.cc‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,25 @@ using ConsistencyChecker = ceph::consistency::ConsistencyChecker;
1919
namespace {
2020
template <typename S>
2121
int send_osd_command(int osd, S& s, librados::Rados& rados, const char* name,
22-
ceph::buffer::list& inbl, ceph::buffer::list* outbl,
22+
ceph::buffer::list&& inbl, ceph::buffer::list* outbl,
2323
Formatter* f) {
2424
encode_json(name, s, f);
2525

2626
std::ostringstream oss;
2727
f->flush(oss);
28-
int rc = rados.osd_command(osd, oss.str(), inbl, outbl, nullptr);
28+
int rc = rados.osd_command(osd, oss.str(), std::move(inbl), outbl, nullptr);
2929
return rc;
3030
}
3131

3232
template <typename S>
3333
int send_mon_command(S& s, librados::Rados& rados, const char* name,
34-
ceph::buffer::list& inbl, ceph::buffer::list* outbl,
34+
ceph::buffer::list&& inbl, ceph::buffer::list* outbl,
3535
Formatter* f) {
3636
encode_json(name, s, f);
3737

3838
std::ostringstream oss;
3939
f->flush(oss);
40-
int rc = rados.mon_command(oss.str(), inbl, outbl, nullptr);
40+
int rc = rados.mon_command(oss.str(), std::move(inbl), outbl, nullptr);
4141
return rc;
4242
}
4343
} // namespace
@@ -377,14 +377,14 @@ void RadosIo::applyReadWriteOp(IoOp& op) {
377377
}
378378

379379
void RadosIo::applyInjectOp(IoOp& op) {
380-
bufferlist osdmap_inbl, inject_inbl, osdmap_outbl, inject_outbl;
380+
bufferlist osdmap_outbl, inject_outbl;
381381
auto formatter = std::make_unique<JSONFormatter>(false);
382382

383383
int osd = -1;
384384
std::vector<int> shard_order;
385385

386386
ceph::messaging::osd::OSDMapRequest osdMapRequest{pool, get_primary_oid(), ""};
387-
int rc = send_mon_command(osdMapRequest, rados, "OSDMapRequest", osdmap_inbl,
387+
int rc = send_mon_command(osdMapRequest, rados, "OSDMapRequest", {},
388388
&osdmap_outbl, formatter.get());
389389
ceph_assert(rc == 0);
390390

@@ -407,7 +407,7 @@ void RadosIo::applyInjectOp(IoOp& op) {
407407
injectErrorRequest{pool, primary_oid, errorOp.shard,
408408
errorOp.type, errorOp.when, errorOp.duration};
409409
int rc = send_osd_command(osd, injectErrorRequest, rados,
410-
"InjectECErrorRequest", inject_inbl,
410+
"InjectECErrorRequest", {},
411411
&inject_outbl, formatter.get());
412412
ceph_assert(rc == 0);
413413
} else if (errorOp.type == 1) {
@@ -416,7 +416,7 @@ void RadosIo::applyInjectOp(IoOp& op) {
416416
injectErrorRequest{pool, primary_oid, errorOp.shard,
417417
errorOp.type, errorOp.when, errorOp.duration};
418418
int rc = send_osd_command(osd, injectErrorRequest, rados,
419-
"InjectECErrorRequest", inject_inbl,
419+
"InjectECErrorRequest", {},
420420
&inject_outbl, formatter.get());
421421
ceph_assert(rc == 0);
422422
} else {
@@ -433,15 +433,15 @@ void RadosIo::applyInjectOp(IoOp& op) {
433433
injectErrorRequest{pool, primary_oid, errorOp.shard,
434434
errorOp.type, errorOp.when, errorOp.duration};
435435
int rc = send_osd_command(osd, injectErrorRequest, rados,
436-
"InjectECErrorRequest", inject_inbl,
436+
"InjectECErrorRequest", {},
437437
&inject_outbl, formatter.get());
438438
ceph_assert(rc == 0);
439439
} else if (errorOp.type == 3) {
440440
ceph::messaging::osd::InjectECErrorRequest<InjectOpType::WriteOSDAbort>
441441
injectErrorRequest{pool, primary_oid, errorOp.shard,
442442
errorOp.type, errorOp.when, errorOp.duration};
443443
int rc = send_osd_command(osd, injectErrorRequest, rados,
444-
"InjectECErrorRequest", inject_inbl,
444+
"InjectECErrorRequest", {},
445445
&inject_outbl, formatter.get());
446446
ceph_assert(rc == 0);
447447

@@ -462,15 +462,15 @@ void RadosIo::applyInjectOp(IoOp& op) {
462462
ceph::messaging::osd::InjectECClearErrorRequest<InjectOpType::ReadEIO>
463463
clearErrorInject{pool, primary_oid, errorOp.shard, errorOp.type};
464464
int rc = send_osd_command(osd, clearErrorInject, rados,
465-
"InjectECClearErrorRequest", inject_inbl,
465+
"InjectECClearErrorRequest", {},
466466
&inject_outbl, formatter.get());
467467
ceph_assert(rc == 0);
468468
} else if (errorOp.type == 1) {
469469
ceph::messaging::osd::InjectECClearErrorRequest<
470470
InjectOpType::ReadMissingShard>
471471
clearErrorInject{pool, primary_oid, errorOp.shard, errorOp.type};
472472
int rc = send_osd_command(osd, clearErrorInject, rados,
473-
"InjectECClearErrorRequest", inject_inbl,
473+
"InjectECClearErrorRequest", {},
474474
&inject_outbl, formatter.get());
475475
ceph_assert(rc == 0);
476476
} else {
@@ -488,15 +488,15 @@ void RadosIo::applyInjectOp(IoOp& op) {
488488
InjectOpType::WriteFailAndRollback>
489489
clearErrorInject{pool, primary_oid, errorOp.shard, errorOp.type};
490490
int rc = send_osd_command(osd, clearErrorInject, rados,
491-
"InjectECClearErrorRequest", inject_inbl,
491+
"InjectECClearErrorRequest", {},
492492
&inject_outbl, formatter.get());
493493
ceph_assert(rc == 0);
494494
} else if (errorOp.type == 3) {
495495
ceph::messaging::osd::InjectECClearErrorRequest<
496496
InjectOpType::WriteOSDAbort>
497497
clearErrorInject{pool, primary_oid, errorOp.shard, errorOp.type};
498498
int rc = send_osd_command(osd, clearErrorInject, rados,
499-
"InjectECClearErrorRequest", inject_inbl,
499+
"InjectECClearErrorRequest", {},
500500
&inject_outbl, formatter.get());
501501
ceph_assert(rc == 0);
502502
} else {

‎src/erasure-code/consistency/RadosCommands.cc‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ int RadosCommands::get_primary_osd(const std::string& pool_name,
2929
std::ostringstream oss;
3030
formatter.get()->flush(oss);
3131

32-
ceph::bufferlist inbl, outbl;
33-
int rc = rados.mon_command(oss.str(), inbl, &outbl, nullptr);
32+
ceph::bufferlist outbl;
33+
int rc = rados.mon_command(oss.str(), {}, &outbl, nullptr);
3434
ceph_assert(rc == 0);
3535

3636
JSONParser p;
@@ -60,8 +60,8 @@ bool RadosCommands::get_pool_allow_ec_optimizations(const std::string& pool_name
6060
std::ostringstream oss;
6161
formatter.get()->flush(oss);
6262

63-
ceph::bufferlist inbl, outbl;
64-
int rc = rados.mon_command(oss.str(), inbl, &outbl, nullptr);
63+
ceph::bufferlist outbl;
64+
int rc = rados.mon_command(oss.str(), {}, &outbl, nullptr);
6565
ceph_assert(rc == 0);
6666

6767
JSONParser p;
@@ -89,8 +89,8 @@ std::string RadosCommands::get_pool_ec_profile_name(const std::string& pool_name
8989
std::ostringstream oss;
9090
formatter.get()->flush(oss);
9191

92-
ceph::bufferlist inbl, outbl;
93-
int rc = rados.mon_command(oss.str(), inbl, &outbl, nullptr);
92+
ceph::bufferlist outbl;
93+
int rc = rados.mon_command(oss.str(), {}, &outbl, nullptr);
9494
ceph_assert(rc == 0);
9595

9696
JSONParser p;
@@ -123,8 +123,8 @@ ceph::ErasureCodeProfile RadosCommands::get_ec_profile_for_pool(const std::strin
123123
std::ostringstream oss;
124124
formatter.get()->flush(oss);
125125

126-
ceph::bufferlist inbl, outbl;
127-
int rc = rados.mon_command(oss.str(), inbl, &outbl, nullptr);
126+
ceph::bufferlist outbl;
127+
int rc = rados.mon_command(oss.str(), {}, &outbl, nullptr);
128128
ceph_assert(rc == 0);
129129

130130
// Parse the string output into an ErasureCodeProfile
@@ -170,8 +170,8 @@ void RadosCommands::inject_parity_read_on_primary_osd(const std::string& pool_na
170170
std::ostringstream oss;
171171
formatter.get()->flush(oss);
172172

173-
ceph::bufferlist inbl, outbl;
174-
int rc = rados.osd_command(primary_osd, oss.str(), inbl, &outbl, nullptr);
173+
ceph::bufferlist outbl;
174+
int rc = rados.osd_command(primary_osd, oss.str(), {}, &outbl, nullptr);
175175
ceph_assert(rc == 0);
176176
}
177177

@@ -192,7 +192,7 @@ void RadosCommands::inject_clear_parity_read_on_primary_osd(const std::string& p
192192
std::ostringstream oss;
193193
formatter.get()->flush(oss);
194194

195-
ceph::bufferlist inbl, outbl;
196-
int rc = rados.osd_command(primary_osd, oss.str(), inbl, &outbl, nullptr);
195+
ceph::bufferlist outbl;
196+
int rc = rados.osd_command(primary_osd, oss.str(), {}, &outbl, nullptr);
197197
ceph_assert(rc == 0);
198-
}
198+
}

‎src/include/neorados/RADOS.hpp‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1736,8 +1736,8 @@ class RADOS final
17361736
std::string, ceph::buffer::list);
17371737
using CommandComp = boost::asio::any_completion_handler<CommandSig>;
17381738
template<boost::asio::completion_token_for<CommandSig> CompletionToken>
1739-
auto osd_command(int osd, std::vector<std::string> cmd,
1740-
ceph::buffer::list in, CompletionToken&& token) {
1739+
auto osd_command(int osd, std::vector<std::string>&& cmd,
1740+
ceph::buffer::list&& in, CompletionToken&& token) {
17411741
auto consigned = consign(std::forward<CompletionToken>(token));
17421742
return boost::asio::async_initiate<decltype(consigned), CommandSig>(
17431743
[osd, this](auto&& handler, std::vector<std::string> cmd,
@@ -1747,8 +1747,8 @@ class RADOS final
17471747
}, consigned, std::move(cmd), std::move(in));
17481748
}
17491749
template<boost::asio::completion_token_for<CommandSig> CompletionToken>
1750-
auto pg_command(PG pg, std::vector<std::string> cmd,
1751-
ceph::buffer::list in, CompletionToken&& token) {
1750+
auto pg_command(PG pg, std::vector<std::string>&& cmd,
1751+
ceph::buffer::list&& in, CompletionToken&& token) {
17521752
auto consigned = consign(std::forward<CompletionToken>(token));
17531753
return boost::asio::async_initiate<decltype(consigned), CommandSig>(
17541754
[this](auto&& handler, PG pg, std::vector<std::string> cmd,
@@ -1759,8 +1759,8 @@ class RADOS final
17591759
}
17601760

17611761
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
1762-
auto mon_command(std::vector<std::string> command,
1763-
ceph::buffer::list bl,
1762+
auto mon_command(std::vector<std::string>&& command,
1763+
ceph::buffer::list&& bl,
17641764
std::string* outs, ceph::buffer::list* outbl,
17651765
CompletionToken&& token) {
17661766
auto consigned = consign(std::forward<CompletionToken>(token));
@@ -1881,13 +1881,13 @@ class RADOS final
18811881
Cursor end, std::uint32_t max,
18821882
ceph::buffer::list filter,
18831883
EnumerateComp c);
1884-
void osd_command_(int osd, std::vector<std::string> cmd,
1885-
ceph::buffer::list in, CommandComp c);
1886-
void pg_command_(PG pg, std::vector<std::string> cmd,
1887-
ceph::buffer::list in, CommandComp c);
1884+
void osd_command_(int osd, std::vector<std::string>&& cmd,
1885+
ceph::buffer::list&& in, CommandComp c);
1886+
void pg_command_(PG pg, std::vector<std::string>&& cmd,
1887+
ceph::buffer::list&& in, CommandComp c);
18881888

1889-
void mon_command_(std::vector<std::string> command,
1890-
ceph::buffer::list bl,
1889+
void mon_command_(std::vector<std::string>&& command,
1890+
ceph::buffer::list&& bl,
18911891
std::string* outs, ceph::buffer::list* outbl,
18921892
SimpleOpComp c);
18931893

‎src/include/rados/librados.hpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,13 +1473,13 @@ inline namespace v14_2_0 {
14731473
int get_min_compatible_client(int8_t* min_compat_client,
14741474
int8_t* require_min_compat_client);
14751475

1476-
int mon_command(std::string cmd, const bufferlist& inbl,
1476+
int mon_command(std::string&& cmd, bufferlist&& inbl,
14771477
bufferlist *outbl, std::string *outs);
1478-
int mgr_command(std::string cmd, const bufferlist& inbl,
1478+
int mgr_command(std::string&& cmd, bufferlist&& inbl,
14791479
bufferlist *outbl, std::string *outs);
1480-
int osd_command(int osdid, std::string cmd, const bufferlist& inbl,
1480+
int osd_command(int osdid, std::string&& cmd, bufferlist&& inbl,
14811481
bufferlist *outbl, std::string *outs);
1482-
int pg_command(const char *pgstr, std::string cmd, const bufferlist& inbl,
1482+
int pg_command(const char *pgstr, std::string&& cmd, bufferlist&& inbl,
14831483
bufferlist *outbl, std::string *outs);
14841484

14851485
int ioctx_create(const char *name, IoCtx &pioctx);

‎src/librados/IoCtxImpl.cc‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2107,10 +2107,7 @@ void librados::IoCtxImpl::application_enable_async(const std::string& app_name,
21072107
}
21082108
cmd << "}";
21092109

2110-
std::vector<std::string> cmds;
2111-
cmds.push_back(cmd.str());
2112-
bufferlist inbl;
2113-
client->mon_command_async(cmds, inbl, nullptr, nullptr,
2110+
client->mon_command_async({cmd.str()}, {}, nullptr, nullptr,
21142111
make_lambda_context(CB_PoolAsync_Safe(c)));
21152112
}
21162113

@@ -2174,10 +2171,7 @@ int librados::IoCtxImpl::application_metadata_set(const std::string& app_name,
21742171
<< "\"value\":\"" << value << "\""
21752172
<< "}";
21762173

2177-
std::vector<std::string> cmds;
2178-
cmds.push_back(cmd.str());
2179-
bufferlist inbl;
2180-
int r = client->mon_command(cmds, inbl, nullptr, nullptr);
2174+
int r = client->mon_command({cmd.str()}, {}, nullptr, nullptr);
21812175
if (r < 0) {
21822176
return r;
21832177
}
@@ -2197,10 +2191,7 @@ int librados::IoCtxImpl::application_metadata_remove(const std::string& app_name
21972191
<< "\"key\":\"" << key << "\""
21982192
<< "}";
21992193

2200-
std::vector<std::string> cmds;
2201-
cmds.push_back(cmd.str());
2202-
bufferlist inbl;
2203-
int r = client->mon_command(cmds, inbl, nullptr, nullptr);
2194+
int r = client->mon_command({cmd.str()}, {}, nullptr, nullptr);
22042195
if (r < 0) {
22052196
return r;
22062197
}

0 commit comments

Comments
 (0)