[Core] Multiprocessing executor for single-node multi-GPU deployment #3466

njhill · 2024-03-18T16:00:26Z

ray is a powerful platform for general purpose distributed computing but potentially overkill for the specific requirements of realtime synchronized inferencing between GPUs on a single node.

We would prefer to have a "lightweight" option without the ray dependency for non-ray cluster environments. This also helps with production security compliance.

With the changes in this PR, ray will continue to be used for parallel workers if it's installed, otherwise vanilla python multiprocessing is used. It can also be overridden with --no-worker-use-ray.

Worker processes are shut down when the LLMEngine is garbage collected.

This PR was co-authored by @sahilsuneja1.

This reworks the original PR #2898 to plug into the new distributed executor abstraction.

I've introduced a MultiGPUExecutor abstract superclass shared between the ray and vanilla multiprocessing implementations.

njhill · 2024-03-21T21:48:17Z

@zhuohan123 @simon-mo this one should be ready to go! 🙏

njhill · 2024-03-26T19:24:46Z

vllm/executor/ray_gpu_executor.py

@@ -322,19 +213,17 @@ def _run_workers(
                                       method)(*driver_args, **driver_kwargs)

        # Get the results of the ray workers.
-        if self.workers:


This if removed because workers should always be non-empty here (i.e. if TP > 1)

I believe this if is indeed redundant. However, workers can indeed be empty since the ray GPU executor can be run with TP=1

setup.py

simon-mo · 2024-03-29T00:09:41Z

tests/engine/test_local_worker.py

+    return prompt_token_ids
+
+
+@pytest.mark.skip("Requires multiple GPUs")


our CI can test 2 GPUs, just set the num_gpus: 2 in https://github.com/vllm-project/vllm/blob/main/.buildkite/test-pipeline.yaml

@simon-mo I've actually removed the new test now and just parameterized the existing distributed test to include ray and non-ray.

vllm/engine/llm_engine.py

simon-mo · 2024-03-29T00:17:06Z

vllm/engine/local_worker_utils.py

test some of the functionality from this file without LLM?

OK these tests added in local_worker_tests.py

simon-mo · 2024-03-29T00:17:21Z

vllm/executor/executor_base.py

deferring the review of the interface change to @zhuohan123

vllm/executor/multiproc_gpu_executor.py

njhill · 2024-03-30T01:05:57Z

Thanks for the review @simon-mo! I've addressed all of your comments

njhill · 2024-03-31T19:41:30Z

@zhuohan123 @simon-mo @WoosukKwon I just tried some cursory performance comparisons, wasn't expecting the difference to be so significant. Surprisingly Ray doesn't appear to give any latency benefits over single GPU for the config I tried.

Using 80GB A100s, with llama-2-7b openai completion API. Single request with 5 input tokens, 2000 generated tokens. I repeated each test request multiple times, results were very consistent.

	Time (sec)	Difference
TP=1	24.2	0
TP=2 using Ray (main or this PR)	24.2	0
TP=2 without Ray (this PR with `--no-worker-use-ray`)	17.0	-30%

ywang96 · 2024-03-31T20:53:51Z

@njhill Wow! 30% is quite a bit (albeit serving llama-7b over 2 A100-80G probably doesn't really make sense in practice).

I will do some testing on this branch on parallel with serving benchmark and report back as well

njhill · 2024-03-31T21:19:48Z

Thanks @ywang96, yes I'm sure this will be smaller in relative terms for larger models. But not bad given performance improvement was not the purpose of this PR.

ywang96 · 2024-04-01T00:44:16Z

@njhill I did some preliminary testing on H100 TP2 with mistralai/Mistral-7B-Instruct-v0.1 and there's definitly some speedup (not as much as 30% since this is running on sharegpt).

Server launch command:

python -m vllm.entrypoints.openai.api_server \
        --model mistralai/Mistral-7B-Instruct-v0.1 \
        --swap-space 16 \
        --disable-log-requests \
        --tensor-parallel-size 2 \
        --no-worker-use-ray #comment if use ray

Benchmark command:

python benchmarks/benchmark_serving.py \
        --backend vllm \
        --model mistralai/Mistral-7B-Instruct-v0.1 \
        --dataset-name sharegpt \
        --dataset-path ShareGPT_V3_unfiltered_cleaned_split.json \
        --request-rate 1 \
        --num-prompts 100

With Ray workers:

---------------Time to First Token----------------
Mean TTFT (ms):                          25.42     
Median TTFT (ms):                        31.51     
P99 TTFT (ms):                           49.27     
-----Time per Output Token (excl. 1st token)------
Mean TPOT (ms):                          8.21      
Median TPOT (ms):                        8.20      
P99 TPOT (ms):                           10.59

This PR:

---------------Time to First Token----------------
Mean TTFT (ms):                          22.31     
Median TTFT (ms):                        27.61     
P99 TTFT (ms):                           36.08     
-----Time per Output Token (excl. 1st token)------
Mean TPOT (ms):                          7.75      
Median TPOT (ms):                        7.79      
P99 TPOT (ms):                           9.59

njhill · 2024-04-01T00:50:54Z

Thanks @ywang96, that's great! 5-6% lower TPOT still nice to have! I am doing some spot tests on TP=4 llama-70b too.

njhill · 2024-04-01T00:58:49Z

For llama-2-70b with single request 5 input / 1000 output the times I got are 32.3 before, 30.8 after i.e. 4-5% speedup.

ywang96 · 2024-04-01T01:06:01Z

For llama-2-70b with single request 5 input / 1000 output the times I got are 32.3 before, 30.8 after i.e. 4-5% speedup.

I will test on A100-80G with Mixtral TP4 and TP8 just to see if 4-5% is likely the average speedup we get in general.

rkooo567

I see. If the mp based approach is faster, it makes sense to change the default I think. OOC, what's the delta between this vs ray default (for a single node case) except the performance? I assume it supports logging prefix, so maybe just the debugger and the ray dashboard?

rkooo567 · 2024-04-15T15:44:48Z

vllm/engine/local_worker_utils.py

@@ -0,0 +1,242 @@
+import asyncio


QQ: this file is probably very specific to mp based executor. Should we create executor/util and move it to there?

@rkooo567 yes maybe the naming is bad but that's what it's meant for, equivalent to ray_utils.py which is in the same place.

+1 Let's probably move this file to executor/? And also move ray_utils to executor/?

ray is a powerful platform for general purpose distributed computing but potentially overkill for the specific requirements of realtime synchronized inferencing between GPUs on a single node. We would prefer to have a "lightweight" option without the ray dependency for non-ray cluster environments. This also helps with production security compliance. With the changes in this PR, ray will continue to be used for parallel workers if it's installed, otherwise vanilla python multiprocessing is used. It can also be overridden with --no-worker-use-ray. Worker processes are shut down when the LLMEngine is garbage collected. Co-authored-by: Sahil Suneja <[email protected]>

Instead of adding equivalent new test

Useful for unit tests

mpjlu · 2024-04-16T08:51:12Z

I tested baichuan 13B TP=2 with/without cudagraph on A100, the performance of the PR and main branch is the same. Do you test 13B models?

njhill · 2024-04-18T22:20:46Z

what's the delta between this vs ray default (for a single node case) except the performance? I assume it supports logging prefix, so maybe just the debugger and the ray dashboard?

@rkooo567 yes I think that's all correct.

youkaichao · 2024-04-22T19:47:19Z

My suggestions:

try if torch rpc works and if it has benefit. that modification should be small, and the usage is quite similar to ray, while the underlying implementation is multiprocessing.
tear down the PR in small sizes, so that we can digest it step-by-step.

zhuohan123

Sorry for the delay Nick! Please find my comments below. I think in general this is a good feature that we should have. However, I don't quite understand what's going on in vllm/engine/local_worker_utils.py. Let's chat offline about this.

zhuohan123 · 2024-03-24T23:05:05Z

setup.py

+def get_ray_requirement() -> Optional[Dict[str, List[str]]]:
+    if _is_neuron():
+        return None
+    return {"ray": ["ray >= 2.9"]}


We have different requirements.txt for different backends. Is this function necessary?

This is to make ray an optional extra, independent of the backend. Ray is currently supported for some backends and not others, and if it is supported the version requirement can be different, hence the if's here.

zhuohan123 · 2024-03-24T23:06:55Z

tests/engine/test_local_worker.py

+    return prompt_token_ids
+
+
+@pytest.mark.skip("Requires multiple GPUs")


You can add a condition like

@pytest.mark.skipif(torch.cuda.device_count() < 2, reason="Need at least 2 GPUs to run the test.")

and put this test in distributed test?

@zhuohan123 I think somehow you weren't reviewing the latest version of the PR, but the last time this branch was updated was a week ago.

These tests no longer require GPUs, they test the mutliprocessing mechanics in isolation. The existing distributed test is additionally parameterized to test both ray and non-ray.

zhuohan123 · 2024-03-24T23:08:40Z

vllm/config.py

+        if self.worker_use_ray is None:
+            ray_found = importlib.util.find_spec("ray") is not None
+            self.worker_use_ray = ray_found and self.world_size > 1
+


Can we check whether ray is successfully imported in vllm/engine/ray_utils.py?

zhuohan123 · 2024-04-23T02:45:47Z

vllm/engine/local_worker_utils.py

+# Use dedicated multiprocess context for workers.
+# Both spawn and fork work
+mp_method = os.getenv("MULTIPROC_METHOD", "fork")


Can we always use spawn here? I don't think there will be cases when fork will be better.

Sure. I think fork might be faster, I can test to see if it makes non-negligible difference.

zhuohan123 · 2024-04-23T02:48:22Z

vllm/engine/arg_utils.py

-                            'automatically set when using more than 1 GPU')
+        parser.add_argument(
+            '--worker-use-ray',
+            action=argparse.BooleanOptionalAction,


Why is this change needed?

@zhuohan123 it allows having a default to a boolean arg of None so that we can differentiate between not set and explicitly set to false.

It looks like this may have been introduced in python 3.9 though, so I guess may need to be changed anyhow.

zhuohan123 · 2024-04-23T02:50:14Z

vllm/engine/local_worker_utils.py

@@ -0,0 +1,242 @@
+import asyncio


+1 Let's probably move this file to executor/? And also move ray_utils to executor/?

zhuohan123 · 2024-04-23T03:02:05Z

vllm/executor/ray_gpu_executor.py

@@ -322,19 +213,17 @@ def _run_workers(
                                       method)(*driver_args, **driver_kwargs)

        # Get the results of the ray workers.
-        if self.workers:


I believe this if is indeed redundant. However, workers can indeed be empty since the ray GPU executor can be run with TP=1

zhuohan123 · 2024-04-23T03:03:12Z

vllm/executor/multi_gpu_executor.py

This refactor looks really good! Thanks for the work!

zhuohan123 · 2024-04-23T03:11:47Z

vllm/engine/local_worker_utils.py

+        self.result_handler.close()
+
+
+class LocalWorkerVllm(mp.Process):


Why do we choose to inherit mp.Process instead of just creating a mp.Process instance in the class? I'm not very familiar but is this a standard practice?

I actually didn't write the first iteration of this PR ... I think it's fairly standard but probably more common/clearer to wrap an instance instead, I'll change this.

zhuohan123 · 2024-04-23T03:27:46Z

vllm/engine/local_worker_utils.py

+        _add_prefix(sys.stdout, process_name, pid)
+        _add_prefix(sys.stderr, process_name, pid)
+
+        del self.tasks  # Not used in forked process


Why do we need to explicitly delete this field here? Can we just not include this field in __init__?

__init__ runs in the main/parent process (since that's where this class gets constructed), the object state will be copied to the forked/spawned process when start() is called. This task map is only used in the copy of this object in the main process - it's deleted here to make that clearer.

This will probably need to change though if we change to not subclass multiprocessing.Process.

@zhuohan123

Per @zhuohan123's request here: vllm-project#3466 (comment)

nivibilla · 2024-04-30T22:07:48Z

Just a note for this. I use Ray to do multi node batch inference with vllm. (On a 8x8*A10) And with models that fit in a single GPU it worked perfectly but trying to initialize tensor parallel models with ray, within a ray instance doesn't work. I think this solution is the only way to do multi node batch Inference with ray orchestrating the nodes. And the multiprocessing for tensor parallel 70b inside the worker node.

Thank you for this PR! Hope it gets merged soon.

jacobthebanana · 2024-05-08T15:40:03Z

My team at work has been looking for a way to do efficient autoregressive generation during LLM fine-tuning. We'd like to tap into the efficiency of vLLM, but so far haven't been able to run torch FSDP alongside vLLM on the same set of GPUs. The changes proposed in this pull request have resolved our issue. Thanks again Nick for the great work, and I'd love to see this pull request being merged very soon.

njhill · 2024-05-08T15:48:45Z

@nivibilla @jacobthebanana I had some interrupts in the last few days but will make sure this lands this week (not this PR but the ones that replaced it).

njhill · 2024-05-15T22:46:05Z

This was broken into smaller PRs which have now all been merged, see #4539.

njhill mentioned this pull request Mar 18, 2024

Make ray optional for single-node deployment #2898

Closed

njhill force-pushed the ray-optional2 branch 3 times, most recently from d55f701 to a1e1ede Compare March 20, 2024 20:25

njhill force-pushed the ray-optional2 branch from a1e1ede to cbd2002 Compare March 22, 2024 22:11

youkaichao mentioned this pull request Mar 23, 2024

[RFC]: Interface and Abstraction for Distributed Inference Environment #3587

Closed

zhuohan123 self-assigned this Mar 24, 2024

njhill force-pushed the ray-optional2 branch 2 times, most recently from f6f72a3 to 8a60fb8 Compare March 26, 2024 19:22

njhill commented Mar 26, 2024

View reviewed changes

njhill force-pushed the ray-optional2 branch 2 times, most recently from 7232261 to 1169213 Compare March 28, 2024 21:23

simon-mo reviewed Mar 29, 2024

View reviewed changes

njhill force-pushed the ray-optional2 branch 2 times, most recently from efd61b2 to fb15723 Compare March 29, 2024 17:22

chanh reviewed Mar 29, 2024

View reviewed changes

vllm/executor/multiproc_gpu_executor.py Show resolved Hide resolved

WoosukKwon mentioned this pull request Mar 29, 2024

[Core][ROCm][AMD] Add optional torchrun multi GPU executor #3691

Closed

njhill force-pushed the ray-optional2 branch from fb15723 to a85ef7e Compare March 30, 2024 01:04

njhill force-pushed the ray-optional2 branch from a85ef7e to a7ad3bb Compare March 30, 2024 05:19

njhill mentioned this pull request Mar 31, 2024

[Core] Eliminate parallel worker per-step task scheduling overhead #3763

Closed

youkaichao mentioned this pull request Mar 31, 2024

[WIP][Core] fully composible launcher/task/coordinator/communicator design and implementation #3762

Closed

rkooo567 reviewed Apr 15, 2024

View reviewed changes

njhill and others added 8 commits April 15, 2024 13:16

Use getattr to access model_executor in engine destructor

c0bad3d

Address a couple of review comments

d0a8709

Extend existing distributed correctness test

5e214a3

Instead of adding equivalent new test

Use factory for worker initialization

0fb0743

Useful for unit tests

Test local worker mechanics in isolation

e048001

Add pid prefix to process stdout/stderr instead of logger

1938c35

Update new chunked prefill distributed test to include non-Ray

56a1ad4

njhill force-pushed the ray-optional2 branch from eda5a68 to 56a1ad4 Compare April 15, 2024 20:20

njhill mentioned this pull request Apr 19, 2024

[Core] Some simplification of WorkerWrapper changes #4183

Merged

zhuohan123 reviewed Apr 23, 2024

View reviewed changes

zhuohan123 added the action-required label Apr 23, 2024

njhill mentioned this pull request Apr 25, 2024

[Core] Multiprocessing executor for single-node multi-GPU [1/2] #4345

Closed

njhill added a commit to njhill/vllm that referenced this pull request Apr 25, 2024

Move ray_utils.py from engine to executor package

3347604

Per @zhuohan123's request here: vllm-project#3466 (comment)

njhill mentioned this pull request Apr 25, 2024

[Core] Move ray_utils.py from engine to executor package #4347

Merged

nivibilla mentioned this pull request Apr 30, 2024

v0.4.2 Release Tracker #4505

Closed

This was referenced May 1, 2024

[Core] Add multiproc_worker_utils for multiprocessing-based workers #4357

Merged

[Core] Add MultiprocessingGPUExecutor #4539

Merged

jacobthebanana mentioned this pull request May 9, 2024

[Feature]: Allow LoRA adapters to be specified as in-memory dict of tensors #4068

Open

njhill closed this May 15, 2024

njhill deleted the ray-optional2 branch May 15, 2024 22:46

youkaichao mentioned this pull request Jun 19, 2024

[misc][distributed] use localhost for single-node #5619

Merged

1 task

		return prompt_token_ids


		@pytest.mark.skip("Requires multiple GPUs")

		self.result_handler.close()


		class LocalWorkerVllm(mp.Process):

[Core] Multiprocessing executor for single-node multi-GPU deployment #3466

[Core] Multiprocessing executor for single-node multi-GPU deployment #3466

Conversation

njhill commented Mar 18, 2024

njhill commented Mar 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njhill commented Mar 30, 2024

njhill commented Mar 31, 2024

ywang96 commented Mar 31, 2024

njhill commented Mar 31, 2024

ywang96 commented Apr 1, 2024 • edited Loading

njhill commented Apr 1, 2024

njhill commented Apr 1, 2024

ywang96 commented Apr 1, 2024 • edited Loading

rkooo567 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpjlu commented Apr 16, 2024

njhill commented Apr 18, 2024

youkaichao commented Apr 22, 2024

zhuohan123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nivibilla commented Apr 30, 2024 • edited Loading

jacobthebanana commented May 8, 2024

njhill commented May 8, 2024

njhill commented May 15, 2024

ywang96 commented Apr 1, 2024 •

edited

Loading

ywang96 commented Apr 1, 2024 •

edited

Loading

rkooo567 left a comment •

edited

Loading

nivibilla commented Apr 30, 2024 •

edited

Loading