[V1] Refactor parallel sampling support #13774

markmc · 2025-02-24T16:56:08Z

The initial implementation in #10980 went to great efforts to add parallel sampling as a wrapper at the highest layer of abstraction possible. This resulted in a lot of tricky code to post-process RequestOutputs to aggregate them where necessary.

Instead, it probably makes sense to implement parallel sampling at the layer that actually creates RequestOutput objects - i.e. in OutputProcessor

To do this, we simply need to allow for fanning out child requests in LLMEngine.add_request(), passing details of the fan-out to OutputProcessor.

This adds some overhead to the n=1 case (see SingularSamplingRequest) in return for significantly less overhead and complication in the parallel sampling case.

github-actions · 2025-02-24T16:56:23Z

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

mgoin · 2025-02-24T17:32:52Z

Nice work cleaning this up!

This adds some overhead to the n=1 case (see SingularSamplingRequest) in return for significantly less overhead and complication in the parallel sampling case.

We should verify that this overhead is negligible with a quick benchmark

njhill

Thanks @markmc, looks great and I agree doing the aggregation in the output processor is much nicer! Just some minor suggestions to simplify further.

vllm/v1/engine/output_processor.py

vllm/v1/engine/async_llm.py

mergify · 2025-02-25T10:08:16Z

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @markmc.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

vllm/v1/engine/output_processor.py

vllm/v1/engine/async_llm.py

vllm/v1/engine/output_processor.py

afeldman-nm · 2025-02-27T16:39:46Z

Initial benchmarking results with n=1 (averaged over 3x runs) - it appears that there is not a performance regression. I may try a serving benchmark later.

main:

Throughput: 41.7 rps
P99 LLMEngine average execution time: 2.50 sec

parallel-sampling-refactor:

Throughput (rps): 42.0 rps
P99 LLMEngine average execution time: 2.47 sec

Command to measure LLMEngine throughput:

VLLM_USE_V1=1 VLLM_ENABLE_V1_MULTIPROCESSING=1 python benchmarks/benchmark_throughput.py --model meta-llama/Llama-3.2-3B-Instruct --input-len 128 --output-len 512

Command to measure LLMEngine average execution time:

VLLM_USE_V1=1 VLLM_ENABLE_V1_MULTIPROCESSING=1 python benchmarks/benchmark_latency.py --model meta-llama/Llama-3.2-3B-Instruct --input-len 128 --output-len 512

afeldman-nm · 2025-02-27T19:06:27Z

Async serving benchmarking results with n=1 (averaged over 3x runs) - it appears that there is not a performance regression. Since serving benchmark results can have high variance on beaker, for each metric I also include (loosely speaking) the range of each metric, computed as "worst" minus "best" as a percentage of the "best".

main:

Throughput: 79.44 rps (Lowest-Highest %: -2.3%)
P99 TTFT: 3512.56 ms (Highest-Lowest %: 5.3%)
P99 TPOT: 110.70ms (Highest-Lowest %: 21.6%)
P99 ITL: 114.84ms (Highest-Lowest %: 3.0%)

parallel-sampling-refactor:

Throughput: 80.49 rps (Lowest-Highest %: -5.3%), +1% improvement over main
P99 TTFT: 3289.69 ms (Highest-Lowest %: 6.4%), -6% improvement over main
P99 TPOT: 98.53ms (Highest-Lowest %: 1.52%), -11% improvement over main
P99 ITL: 111.13ms (Highest-Lowest %: 0.96%), -3% improvement over main

Command to bring up vLLM v1 engine server:

VLLM_USE_V1=1 VLLM_ENABLE_V1_MULTIPROCESSING=1 vllm serve meta-llama/Llama-3.2-3B-Instruct --port 8091 --disable-log-requests --no-enable-prefix-caching

Benchmark command:

python benchmarks/benchmark_serving.py --model meta-llama/Llama-3.2-3B-Instruct --dataset-path ../sharegpt.json  --port 8091

afeldman-nm · 2025-02-27T19:09:55Z

Nice work cleaning this up!

This adds some overhead to the n=1 case (see SingularSamplingRequest) in return for significantly less overhead and complication in the parallel sampling case.

We should verify that this overhead is negligible with a quick benchmark

Based on the benchmarking results above, there does not appear to be a perf regression.

vllm/v1/engine/parallel_sampling.py

vllm/v1/engine/llm_engine.py

afeldman-nm

Thanks for the PR @markmc ! I left some feedback.

vllm/v1/engine/llm_engine.py

vllm/v1/engine/async_llm.py

vllm/v1/engine/parallel_sampling.py

afeldman-nm · 2025-02-27T22:03:28Z

vllm/v1/engine/async_llm.py

+            request = self.processor.process_inputs(request_id, prompt, params,
+                                                    arrival_time, lora_request,
+                                                    trace_headers,
+                                                    prompt_adapter_request,
+                                                    priority)


Something I wondered - can we reuse the output of process_inputs over multiple child requests? Since I believe process_inputs operates solely on the prompt (or MM inputs or whatever.)

This could be tricky for multimodal so I don't know if it is in-scope for this PR.

Yeah, I did wonder that too, but didn't dig into it. Seem quite orthogonal to this PR though

Yes agree. Actually we can move this outside of the loop I think, and just update the request_id and sampling_params of the request inside the loop.

Doesn't need to hold up this PR but would be quite a simple change I think.

vllm/v1/engine/output_processor.py

vllm/v1/engine/parallel_sampling.py

vllm/v1/engine/output_processor.py

njhill

Thanks @markmc looks great, just a couple more small things.

vllm/v1/engine/llm_engine.py

njhill · 2025-02-28T16:19:06Z

vllm/v1/engine/parallel_sampling.py

-        gens.append(gen)
+        # If not streaming, aggregate until all child requests complete
+        if (final_only and len(request_output.outputs) != self.n):
+            self.output_aggregator = request_output


It would make more sense to me for this assignment to be in the other if above.

But you don't want to set it if final_only == False ...

Ah right, ok ignore my comment then! I'm not sure why I find it slightly confusing to overwrite this for the intermediate outputs.

vllm/v1/engine/parallel_sampling.py

njhill · 2025-02-28T16:44:39Z

vllm/v1/engine/async_llm.py

+            request = self.processor.process_inputs(request_id, prompt, params,
+                                                    arrival_time, lora_request,
+                                                    trace_headers,
+                                                    prompt_adapter_request,
+                                                    priority)


Yes agree. Actually we can move this outside of the loop I think, and just update the request_id and sampling_params of the request inside the loop.

Doesn't need to hold up this PR but would be quite a simple change I think.

vllm/v1/engine/output_processor.py

njhill

LGTM

The initial implementation went to great efforts to add parallel sampling as a wrapper at the highest layer of abstraction possible. This resulted in a lot of tricky code to post-process RequestOutputs to aggregate them where necessary. Instead, it probably makes sense to implement parallel sampling at the layer that actually creates RequestOutput objects - i.e. in OutputProcessor. To do this, we simply need to allow for fanning out child requests in LLMEngine.add_request(), passing details of the fan-out to OutputProcessor. This adds some overhead to the n=1 case in return for significantly less overhead and complication in the parallel sampling case. Signed-off-by: Mark McLoughlin <markmc@redhat.com>

Add default values for OutputProcessor.add_request() parent_req and index parameters to fix tests. These are the values used in the n==1 case. Signed-off-by: Mark McLoughlin <markmc@redhat.com>

markmc · 2025-03-03T11:08:30Z

Lint and deploy minio setup failing with:

make_bucket failed: s3://testbucket Could not connect to the endpoint URL: "http://127.0.0.1:9000/testbucket"

As per the original V1 parallel sampling code. Signed-off-by: Mark McLoughlin <markmc@redhat.com>

Signed-off-by: Mark McLoughlin <markmc@redhat.com>

markmc requested review from WoosukKwon, robertgshaw2-redhat, njhill, ywang96, comaniac and alexm-redhat as code owners February 24, 2025 16:56

mergify bot added the v1 label Feb 24, 2025

njhill reviewed Feb 24, 2025

View reviewed changes

mergify bot added the needs-rebase label Feb 25, 2025

markmc closed this Feb 25, 2025

markmc force-pushed the parallel-sampling-refactor branch from 1aed2ed to 75e9d49 Compare February 25, 2025 10:34

markmc reopened this Feb 25, 2025

mergify bot removed the needs-rebase label Feb 25, 2025

njhill reviewed Feb 25, 2025

View reviewed changes

vllm/v1/engine/output_processor.py Outdated Show resolved Hide resolved

vllm/v1/engine/output_processor.py Outdated Show resolved Hide resolved

vllm/v1/engine/output_processor.py Outdated Show resolved Hide resolved

vllm/v1/engine/async_llm.py Outdated Show resolved Hide resolved

afeldman-nm reviewed Feb 25, 2025

View reviewed changes

vllm/v1/engine/output_processor.py Show resolved Hide resolved

markmc force-pushed the parallel-sampling-refactor branch from 0153a5b to 60fa08d Compare February 26, 2025 12:16

afeldman-nm reviewed Feb 27, 2025

View reviewed changes

vllm/v1/engine/parallel_sampling.py Outdated Show resolved Hide resolved

vllm/v1/engine/parallel_sampling.py Show resolved Hide resolved

vllm/v1/engine/llm_engine.py Outdated Show resolved Hide resolved

afeldman-nm suggested changes Feb 27, 2025

View reviewed changes

markmc force-pushed the parallel-sampling-refactor branch from 60fa08d to a65f0c9 Compare February 28, 2025 16:20

markmc mentioned this pull request Feb 28, 2025

[WIP][[V1][Metrics] Implement max_num_generation_tokens, request_params_n, and request_params_max_tokens metrics #14055

Merged

njhill reviewed Feb 28, 2025

View reviewed changes

njhill approved these changes Feb 28, 2025

View reviewed changes

robertgshaw2-redhat approved these changes Feb 28, 2025

View reviewed changes

robertgshaw2-redhat added the ready label Mar 2, 2025

markmc force-pushed the parallel-sampling-refactor branch from 044ad2f to 7736b2d Compare March 3, 2025 07:37

[V1][Parallel Sampling] Fix test_output_processor.py

Loading
Loading status checks…

1f41580

Add default values for OutputProcessor.add_request() parent_req and index parameters to fix tests. These are the values used in the n==1 case. Signed-off-by: Mark McLoughlin <markmc@redhat.com>

[V1][Parallel Sampling] Sort completion outputs by index

Loading
Loading status checks…

a482e3b

As per the original V1 parallel sampling code. Signed-off-by: Mark McLoughlin <markmc@redhat.com>

njhill merged commit 4167252 into vllm-project:main Mar 3, 2025
33 checks passed

Akshat-Tripathi pushed a commit to krai/vllm that referenced this pull request Mar 3, 2025

[V1] Refactor parallel sampling support (vllm-project#13774)

c6cc1f5

Signed-off-by: Mark McLoughlin <markmc@redhat.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

[V1] Refactor parallel sampling support #13774

[V1] Refactor parallel sampling support #13774

markmc commented Feb 24, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Feb 24, 2025

mgoin commented Feb 24, 2025

njhill left a comment

mergify bot commented Feb 25, 2025

afeldman-nm commented Feb 27, 2025 •

edited

Loading

afeldman-nm commented Feb 27, 2025 •

edited

Loading

afeldman-nm commented Feb 27, 2025

afeldman-nm left a comment

afeldman-nm Feb 27, 2025

markmc Feb 28, 2025

njhill Feb 28, 2025

njhill left a comment

njhill Feb 28, 2025

markmc Feb 28, 2025

njhill Feb 28, 2025

njhill Feb 28, 2025

njhill left a comment

markmc commented Mar 3, 2025

[V1] Refactor parallel sampling support #13774

[V1] Refactor parallel sampling support #13774

Conversation

markmc commented Feb 24, 2025 • edited by github-actions bot Loading

github-actions bot commented Feb 24, 2025

mgoin commented Feb 24, 2025

njhill left a comment

Choose a reason for hiding this comment

mergify bot commented Feb 25, 2025

afeldman-nm commented Feb 27, 2025 • edited Loading

afeldman-nm commented Feb 27, 2025 • edited Loading

afeldman-nm commented Feb 27, 2025

afeldman-nm left a comment

Choose a reason for hiding this comment

afeldman-nm Feb 27, 2025

Choose a reason for hiding this comment

markmc Feb 28, 2025

Choose a reason for hiding this comment

njhill Feb 28, 2025

Choose a reason for hiding this comment

njhill left a comment

Choose a reason for hiding this comment

njhill Feb 28, 2025

Choose a reason for hiding this comment

markmc Feb 28, 2025

Choose a reason for hiding this comment

njhill Feb 28, 2025

Choose a reason for hiding this comment

njhill Feb 28, 2025

Choose a reason for hiding this comment

njhill left a comment

Choose a reason for hiding this comment

markmc commented Mar 3, 2025

markmc commented Feb 24, 2025 •

edited by github-actions bot

Loading

afeldman-nm commented Feb 27, 2025 •

edited

Loading

afeldman-nm commented Feb 27, 2025 •

edited

Loading