Skip to content

Commit 0c0e25e

Browse files
excelle08meta-codesync[bot]
authored andcommitted
Move runFeatureExtraction off the dispatcher; enable for dlrm_mini
Summary: DLRMRequestHandler used to call runFeatureExtraction synchronously on the dispatcher (ThriftSrv.IO) thread, blocking the IO loop for the duration of feature extraction. Worse, feedsim_autoscale_dlrm_mini didn't pass --feature-extractors at all, so on the fixed-QPS variant the call was a no-op anyway and FeatureExtraction's CPU share dropped to ~0% (vs production's 30-35%). The recent higher-QPS sweeps showed BGM at 80% idle past saturation — there's plenty of CPU room to host real feature extraction work, and getting the share up is required to match the production multifeed_aggregator microarchitectural profile. This commit reshapes DLRMRequestHandler's pipeline: * runFeatureExtraction is dispatched async via the same RANKER -> GlobalCPUThread pattern that GetStoriesUncompressedRequestHandler already uses (folly::via(srvCPUThreadPool, lambda { return folly::via(cpuThreadPool, runFeatureExtraction); })). RANKER orchestrates; GlobalCPUThread runs the generated vc_* extractor functions. The dispatcher returns immediately. * Feature extraction starts at the head of the async chain in parallel with the DLRM inference future that was kicked off above. Both run on cpuThreadPool (folly::CPUThreadPoolExecutor is multi-threaded so the two stages overlap). After feature extraction completes, the chain waits for inference, then proceeds to simulateIoOrFanout / pointer chase / response generation. * feedsim_autoscale_dlrm_mini now passes --feature-extractors --feature-complexity=5 --num-stories=100 --extractors-per-story=50 by default. That's 5000 extractor calls per request, the count tuned during Phase 2 to match production's instruction mix. If the resulting CPU share doesn't reach the 30-35% target, the knobs are tunable per-run via -i. Differential Revision: D105119220
1 parent 7142fad commit 0c0e25e

2 files changed

Lines changed: 49 additions & 9 deletions

File tree

benchpress/config/jobs.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,10 @@
805805
- '--client-feature-seed={client_feature_seed}'
806806
- '--client-num-dense={client_num_dense}'
807807
- '--client-num-sparse={client_num_sparse}'
808+
- '--feature-extractors'
809+
- '--feature-complexity={feature_complexity}'
810+
- '--num-stories={num_stories}'
811+
- '--extractors-per-story={extractors_per_story}'
808812
- '-q {fixed_qps}'
809813
- '-d {fixed_qps_duration}'
810814
- '-w {warmup_time}'
@@ -831,6 +835,9 @@
831835
- 'client_feature_seed=42'
832836
- 'client_num_dense=13'
833837
- 'client_num_sparse=26'
838+
- 'feature_complexity=5'
839+
- 'num_stories=100'
840+
- 'extractors_per_story=50'
834841
- 'fixed_qps=100000'
835842
- 'fixed_qps_duration=10'
836843
- 'warmup_time=5'

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

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,10 +1192,10 @@ void DLRMRequestHandler(
11921192
}
11931193
}
11941194

1195-
// Run feature extraction if enabled (with story content if available)
1196-
runFeatureExtraction(this_thread, story_contents);
1197-
1198-
// Populate CopyContext structData from story content when available
1195+
// Populate CopyContext structData from story content when available.
1196+
// This is small (KB-scale memcpy + hashing) so it stays on the
1197+
// dispatcher; the heavy ranking-side feature extraction runs async
1198+
// below.
11991199
if (!story_contents.empty()) {
12001200
std::vector<float> story_dense;
12011201
std::vector<int64_t> story_sparse;
@@ -1254,10 +1254,17 @@ void DLRMRequestHandler(
12541254
? (num_io_stages * io_stage_latency_ms)
12551255
: io_latency_ms;
12561256

1257-
// Pipeline: DLRM inference -> I/O sleep (or RPC fanout) -> pointer
1258-
// chase -> generate+send response. Everything chains via futures so
1259-
// the handler thread returns immediately and the request is processed
1260-
// entirely off the dispatcher thread.
1257+
// Pipeline:
1258+
// - DLRM inference (already started above, runs on cpuThreadPool)
1259+
// - Feature extraction in parallel: orchestrated by RANKER, the
1260+
// actual extractor work delegated to GlobalCPUThread (matches the
1261+
// prod thread-pool layout: ranking orchestration on RANKER, heavy
1262+
// CPU extraction on GlobalCPUThread). Production multifeed
1263+
// aggregator spends ~30-35% of CPU here, so we keep it in the
1264+
// hot path.
1265+
// - collectAll the two, then proceed to I/O fanout, pointer chase,
1266+
// response generation. Everything chains via futures so the
1267+
// dispatcher thread returns immediately.
12611268
//
12621269
// Phase 5: when --rpc_dist_path is set, the I/O sleep is replaced by
12631270
// an outbound RPC fanout to mock_services on srEventBasePool (see
@@ -1268,7 +1275,33 @@ void DLRMRequestHandler(
12681275
// now redundant.
12691276
auto timekeeper = timekeeperPool->getTimekeeper();
12701277
ThreadData* this_thread_ptr = &this_thread;
1271-
std::move(inference_future)
1278+
auto cpuPool = this_thread.cpuThreadPool;
1279+
1280+
// Stage A: hop to RANKER, then delegate feature extraction to
1281+
// GlobalCPUThread. Production multifeed_aggregator spends ~30-35% of
1282+
// CPU here, so we keep it on the hot path. Runs in parallel with
1283+
// the DLRM inference that was already kicked off above (inference
1284+
// executes on cpuThreadPool too, but folly's CPUThreadPoolExecutor
1285+
// is multi-threaded so the two stages overlap).
1286+
// story_contents is captured by value (copy) so its lifetime
1287+
// survives into the deferred GlobalCPUThread task.
1288+
folly::via(srvCPUThreadPool.get(),
1289+
[this_thread_ptr, story_contents, cpuPool]() {
1290+
return folly::via(cpuPool.get(),
1291+
[this_thread_ptr, story_contents]() {
1292+
runFeatureExtraction(*this_thread_ptr, story_contents);
1293+
return folly::unit;
1294+
});
1295+
})
1296+
// Stage B: wait for DLRM inference to complete. inference_future
1297+
// was started before this chain so the wait is usually trivial
1298+
// (extraction is heavier). Hop onto ioThreadPool so the
1299+
// continuation that runs simulateIoOrFanout is on the IO loop.
1300+
.via(ioThreadPool.get())
1301+
.thenValue([inference_future = std::move(inference_future)](
1302+
folly::Unit) mutable {
1303+
return std::move(inference_future);
1304+
})
12721305
.via(ioThreadPool.get())
12731306
.thenValue([this_thread_ptr, total_io_latency_ms, timekeeper, ioThreadPool](
12741307
int prediction_result) {

0 commit comments

Comments
 (0)