Skip to content

Commit bd70802

Browse files
excelle08meta-codesync[bot]
authored andcommitted
Round-robin outbound RPCs across one MockServicesClient per SREventBase
Summary: Each LeafNodeRank dispatcher thread now holds one MockServicesClient per SREventBase (was: one client total, pinned to a single EB). issueOutboundFanout round-robins RPCs across the vector via an atomic counter, so outbound fanout spreads across every EB in the pool instead of funneling through a single Rocket channel. Experimental patch to test the hypothesis that the single-channel-per-dispatcher design is the root cause of the Grace 380ms dispatch_per_rpc vs 4ms mock_handler_actual gap and BGM's low steady-state CPU utilization. If dispatch_per_rpc collapses toward mock_handler_actual after this change, the bottleneck is confirmed as per-channel serialization, not thread count. Reviewed By: YifanYuan3 Differential Revision: D105903218
1 parent 1450524 commit bd70802

1 file changed

Lines changed: 74 additions & 44 deletions

File tree

packages/feedsim/third_party/src/workloads/ranking/LeafNodeRank.cc

Lines changed: 74 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
#include <algorithm>
16+
#include <atomic>
1617
#include <chrono>
1718
#include <cmath>
1819
#include <cstdint>
@@ -150,16 +151,25 @@ struct ThreadData {
150151
std::shared_ptr<ranking::TimekeeperPool> timekeeperPool;
151152

152153
// Phase 5: outbound RPC fanout to mock_services. Populated only when
153-
// --rpc_dist_path is set; nullptr otherwise (in which case the legacy
154+
// --rpc_dist_path is set; empty otherwise (in which case the legacy
154155
// folly::futures::sleep path is used).
155156
//
156-
// mock_client is per-thread because each MockServiceAsyncClient is
157-
// pinned to one folly::EventBase (see MockServicesClient.h). registry
158-
// and silesia are shared (read-only after load) — held here as raw
159-
// pointers to globals to avoid shared_ptr churn on the request path.
157+
// mock_clients holds one MockServicesClient per SREventBase in the
158+
// pool; each MockServiceAsyncClient is pinned to one folly::EventBase
159+
// (see MockServicesClient.h), so to fan an outbound RPC across all
160+
// EBs we need one client per EB. issueOutboundFanout round-robins
161+
// across these via next_mock_client_idx. This addresses the
162+
// single-channel serialization where the prior single-client-per-
163+
// dispatcher design funneled all outbound RPCs through one Rocket
164+
// channel on one EB, producing dispatch_per_rpc ≫ mock_handler_actual
165+
// even with folly::window(K=16) bounding per-fanout concurrency.
166+
// registry and silesia are shared (read-only after load) — held here
167+
// as raw pointers to globals to avoid shared_ptr churn on the
168+
// request path.
160169
ranking::RpcDistRegistry* rpc_registry = nullptr;
161170
ranking::SilesiaLoader* rpc_silesia = nullptr;
162-
std::unique_ptr<ranking::MockServicesClient> mock_client;
171+
std::vector<std::unique_ptr<ranking::MockServicesClient>> mock_clients;
172+
std::atomic<uint64_t> next_mock_client_idx{0};
163173
std::mt19937 rpc_rng;
164174
// rpc_rng is sampled from issueOutboundFanout which runs inside .thenValue
165175
// continuations on the multi-threaded ioThreadPool — multiple concurrent
@@ -293,8 +303,8 @@ void ThreadStartup(
293303

294304
// Phase 5: populate per-thread RPC fanout state. registry / silesia
295305
// pointers are global (initialized in main() if --rpc_dist_path was
296-
// set); mock_client is constructed lazily here so it lives on a
297-
// thread from the SREventBase pool. Seed the fanout RNG with
306+
// set); mock_clients are constructed lazily here so they live on
307+
// threads from the SREventBase pool. Seed the fanout RNG with
298308
// hardware_destructive seed mixing so each thread gets independent
299309
// sample sequences without sharing the std::default_random_engine
300310
// used elsewhere in this struct.
@@ -303,21 +313,26 @@ void ThreadStartup(
303313
this_thread.rpc_rng.seed(
304314
std::random_device{}() ^ static_cast<unsigned>(thread_id + 1));
305315
if (this_thread.rpc_registry != nullptr && srEventBasePool != nullptr) {
306-
auto* evb = srEventBasePool->getEventBase();
307-
try {
308-
this_thread.mock_client =
309-
std::make_unique<ranking::MockServicesClient>(
310-
evb,
311-
args.mock_services_host_arg,
312-
static_cast<uint16_t>(args.mock_services_port_arg));
313-
} catch (const std::exception& e) {
314-
std::cerr << "Failed to connect to mock_services on "
315-
<< args.mock_services_host_arg << ":"
316-
<< args.mock_services_port_arg
317-
<< " (thread " << thread_id << "): " << e.what()
318-
<< ". Falling back to legacy folly::futures::sleep path."
319-
<< std::endl;
320-
this_thread.mock_client.reset();
316+
auto evbs = srEventBasePool->getAllEventBases();
317+
this_thread.mock_clients.reserve(evbs.size());
318+
for (size_t i = 0; i < evbs.size(); ++i) {
319+
try {
320+
this_thread.mock_clients.push_back(
321+
std::make_unique<ranking::MockServicesClient>(
322+
evbs[i].get(),
323+
args.mock_services_host_arg,
324+
static_cast<uint16_t>(args.mock_services_port_arg)));
325+
} catch (const std::exception& e) {
326+
std::cerr << "Failed to connect to mock_services on "
327+
<< args.mock_services_host_arg << ":"
328+
<< args.mock_services_port_arg
329+
<< " (thread " << thread_id << ", evb " << i << "): "
330+
<< e.what()
331+
<< ". Falling back to legacy folly::futures::sleep path."
332+
<< std::endl;
333+
this_thread.mock_clients.clear();
334+
break;
335+
}
321336
}
322337
}
323338

@@ -435,8 +450,8 @@ void ThreadStartup(
435450

436451
// Phase 5: populate per-thread RPC fanout state. registry / silesia
437452
// pointers are global (initialized in main() if --rpc_dist_path was
438-
// set); mock_client is constructed lazily here so it lives on a
439-
// thread from the SREventBase pool. Seed the fanout RNG with
453+
// set); mock_clients are constructed lazily here so they live on
454+
// threads from the SREventBase pool. Seed the fanout RNG with
440455
// hardware_destructive seed mixing so each thread gets independent
441456
// sample sequences without sharing the std::default_random_engine
442457
// used elsewhere in this struct.
@@ -445,21 +460,26 @@ void ThreadStartup(
445460
this_thread.rpc_rng.seed(
446461
std::random_device{}() ^ static_cast<unsigned>(thread_id + 1));
447462
if (this_thread.rpc_registry != nullptr && srEventBasePool != nullptr) {
448-
auto* evb = srEventBasePool->getEventBase();
449-
try {
450-
this_thread.mock_client =
451-
std::make_unique<ranking::MockServicesClient>(
452-
evb,
453-
args.mock_services_host_arg,
454-
static_cast<uint16_t>(args.mock_services_port_arg));
455-
} catch (const std::exception& e) {
456-
std::cerr << "Failed to connect to mock_services on "
457-
<< args.mock_services_host_arg << ":"
458-
<< args.mock_services_port_arg
459-
<< " (thread " << thread_id << "): " << e.what()
460-
<< ". Falling back to legacy folly::futures::sleep path."
461-
<< std::endl;
462-
this_thread.mock_client.reset();
463+
auto evbs = srEventBasePool->getAllEventBases();
464+
this_thread.mock_clients.reserve(evbs.size());
465+
for (size_t i = 0; i < evbs.size(); ++i) {
466+
try {
467+
this_thread.mock_clients.push_back(
468+
std::make_unique<ranking::MockServicesClient>(
469+
evbs[i].get(),
470+
args.mock_services_host_arg,
471+
static_cast<uint16_t>(args.mock_services_port_arg)));
472+
} catch (const std::exception& e) {
473+
std::cerr << "Failed to connect to mock_services on "
474+
<< args.mock_services_host_arg << ":"
475+
<< args.mock_services_port_arg
476+
<< " (thread " << thread_id << ", evb " << i << "): "
477+
<< e.what()
478+
<< ". Falling back to legacy folly::futures::sleep path."
479+
<< std::endl;
480+
this_thread.mock_clients.clear();
481+
break;
482+
}
463483
}
464484
}
465485
unsigned noderank_seed;
@@ -741,7 +761,7 @@ struct FanoutSpec {
741761

742762
static folly::Future<int> issueOutboundFanout(
743763
ThreadData& td, double scale) {
744-
if (td.mock_client == nullptr || td.rpc_registry == nullptr) {
764+
if (td.mock_clients.empty() || td.rpc_registry == nullptr) {
745765
// Defensive: caller should have checked --rpc_dist_path.
746766
return folly::makeFuture<int>(0);
747767
}
@@ -793,18 +813,28 @@ static folly::Future<int> issueOutboundFanout(
793813
}
794814

795815
uint64_t fanout_start_us = feedsim::nowUs();
796-
auto* mock_client = td.mock_client.get();
816+
auto* mock_clients_ptr = &td.mock_clients;
817+
auto* rr_idx_ptr = &td.next_mock_client_idx;
797818
auto* srEvbPool = td.srEventBasePool.get();
798819

799820
// folly::window(executor, items, fn, K): issues fn(item) for the
800821
// first K items; as each returned Future completes, issues the next
801822
// item's fn(). Bounded concurrency K avoids burst-queueing the
802823
// entire fanout onto one EB at once.
824+
//
825+
// Per RPC, round-robin across mock_clients (one per SREventBase) so
826+
// outbound RPCs spread across every EB in the pool instead of
827+
// funneling through a single per-dispatcher Rocket channel. Relaxed
828+
// memory order is fine — we only need monotonic forward progress for
829+
// uniform distribution, no happens-before with other state.
803830
auto futs = folly::window(
804831
srEvbPool,
805832
std::move(specs),
806-
[mock_client, srEvbPool](FanoutSpec spec) {
833+
[mock_clients_ptr, rr_idx_ptr, srEvbPool](FanoutSpec spec) {
807834
uint64_t dispatch_start_us = feedsim::nowUs();
835+
size_t idx = rr_idx_ptr->fetch_add(1, std::memory_order_relaxed) %
836+
mock_clients_ptr->size();
837+
auto* mock_client = (*mock_clients_ptr)[idx].get();
808838
return mock_client
809839
->dispatchByEnum(spec.method, spec.req_payload, spec.lat_us)
810840
.via(srEvbPool)
@@ -847,7 +877,7 @@ static folly::Future<folly::Unit> simulateIoOrFanout(
847877
int io_latency_ms,
848878
folly::Timekeeper* tk,
849879
folly::Executor* via_executor) {
850-
if (td.mock_client != nullptr && td.rpc_registry != nullptr) {
880+
if (!td.mock_clients.empty() && td.rpc_registry != nullptr) {
851881
return issueOutboundFanout(td, args.rpc_fanout_scale_arg)
852882
.via(via_executor)
853883
.thenValue([](int /*completed*/) { return folly::unit; });

0 commit comments

Comments
 (0)