[PerfFix] Avoid separate thread for MP executor shm spin

[njhill]

#26866 included a change to MultiprocExecutor to always busy-wait on the shm message queue from a separate thread, even when async scheduling and pipeline parallel aren't used.

This appears to have drastic performance consequences, possibly due to CPU contention / context switches of the spin with concurrent work happening in the scheduler process (e.g. serialization/deserialization).

This PR reworks the Future mechanism to busy-wait on the queue from the main thread while it is blocking on Future.get_result().

Apart from addressing the non-async performance regression, this may hopefully further improve async scheduling and pipeline parallel performance.

Note: The Executor collective_rpc method with non_block=True has been changed to return a future that resolves to a list rather than a list of futures.

Benchmark

vllm bench latency --model meta-llama/Llama-3.1-8B-Instruct --batch-size 256 --input-len 2048 --output-len 2048 -tp 4 --num-iters-warmup 1 --num-iters 3

Pre-#26866

Avg latency: 41.68921127310023 seconds
10% percentile latency: 41.66592846997082 seconds
25% percentile latency: 41.66958249360323 seconds
50% percentile latency: 41.675672532990575 seconds
75% percentile latency: 41.7020706825424 seconds
90% percentile latency: 41.71790957227349 seconds
99% percentile latency: 41.72741290611215 seconds

Post-#26866

Avg latency: 47.77594091727709 seconds
10% percentile latency: 47.59141381587833 seconds
25% percentile latency: 47.68351502344012 seconds
50% percentile latency: 47.83701703604311 seconds
75% percentile latency: 47.89890487049706 seconds
90% percentile latency: 47.936037571169436 seconds
99% percentile latency: 47.95831719157286 seconds

After this PR:

Avg latency: 40.96941473521292 seconds
10% percentile latency: 40.95436927340925 seconds
25% percentile latency: 40.960011321585625 seconds
50% percentile latency: 40.96941473521292 seconds
75% percentile latency: 40.97881814884022 seconds
90% percentile latency: 40.984460197016595 seconds
99% percentile latency: 40.98784542592242 seconds

[gemini-code-assist]

Code Review

This pull request refactors the MultiprocExecutor to avoid using a separate thread for handling asynchronous results, aiming to fix a performance regression. The approach is to use a queue of futures on the main thread, processing them when a result is requested or a blocking call is made. While the overall direction is sound and simplifies the logic by removing the ThreadPoolExecutor, I've identified a few critical issues in the implementation concerning execution order and API consistency, particularly in the MultiprocExecutor and RayExecutor.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[njhill]

The remaining failing tests appear to unrelated and also failing elsewhere.

[mgoin]

This looks reasonable to me although I'm not the right person to know this area. Approving for now to unblock release issues.
Can you add a benchmark result to show how this resolves the perf regression?

[njhill]

Thanks @mgoin, I've added the benchmark results above.