[inductor] Add a subprocess-based parallel compile #126816

masnesral · 2024-05-21T22:20:14Z

Stack from ghstack (oldest at bottom):

Summary:
Adds a "safe" parallel compile implementation that a) Popens a sub-process with an entry point we control, and b) Uses a ProcessPoolExecutor in that sub-processes to perform parallel compiles. This change essentially squashes these two implementations from jansel, but removes the "thread-based" approach since benchmarking revealed that compile-time performance was poor compared to the existing impl:
#124682
#122941

This PR adds the implementation, but defaults to the existing "fork". I'll submit a separate change to enable.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang

Summary: Adds a "safe" parallel compile implementation that a) Popens a sub-process with an entry point we control, and b) Uses a ProcessPoolExecutor in that sub-processes to perform parallel compiles. This change essentially squashes these two implementations from jansel, but removes the "thread-based" approach since benchmarking revealed that compile-time performance was poor compared to the existing impl: #124682 #122941 This PR adds the implementation, but defaults to the existing "fork". I'll submit a separate change to enable. [ghstack-poisoned]

pytorch-bot · 2024-05-21T22:20:18Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/126816

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (3 Unrelated Failures)

As of commit ecde0bf with merge base 0ff2f8b ():

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

inductor / linux-jammy-cpu-py3.8-gcc11-inductor / test (inductor_torchbench_cpu_smoketest_perf, 1, 1, linux.24xl.spr-metal, unstable) (gh) (#126993)
Process completed with exit code 1.
linux-binary-manywheel / manywheel-py3_8-cuda12_1-test / test (gh) (#127288)
ImportError: libcudnn.so.9: cannot open shared object file: No such file or directory
linux-binary-manywheel / manywheel-py3_8-cuda12_4-test / test (gh) (#127289)
ImportError: libcudnn.so.9: cannot open shared object file: No such file or directory

This comment was automatically generated by Dr. CI and updates every 15 minutes.

masnesral · 2024-05-21T22:22:42Z

Benchmark results: https://hud.pytorch.org/benchmark/compilers?startTime=Tue%2C+14+May+2024+19%3A50%3A25+GMT&stopTime=Tue%2C+21+May+2024+19%3A50%3A25+GMT&granularity=hour&suite=torchbench&mode=training&dtype=amp&lBranch=gh%2Fmasnesral%2F61%2Fhead&lCommit=e5e2e5796b0ffab0922da6a7a714a26ae57ba513&rBranch=main&rCommit=53f73cdeb6fe53155f91064ca15b722e80dec2f3

masnesral · 2024-05-21T22:29:46Z

torch/_inductor/codecache.py

 if (
    os.environ.get("TORCH_TNT_IN_USE", "0") == "1"
    or os.environ.get("TORCH_WARM_POOL", "1") != "1"
+    # TODO(masnesral): warm pool is broken, need to fix it


@jansel, I commandeered this TODO you had originally, but I'm not sure there's anything to actually do here? We're calling _warm_process_pool() on the ProcessPoolExecutor owned by the subproc in SubprocMain.init(). Or did you have some other kind of warmup?

I was getting errors with this turned on. Does it pass for you?

Hmm ok. It's passing for me, but looks like something weird is happening -- extra subprocesses being spawned, probably from the subprocess itself.

jansel · 2024-05-22T04:15:41Z

torch/_inductor/codecache.py

 if (
    os.environ.get("TORCH_TNT_IN_USE", "0") == "1"
    or os.environ.get("TORCH_WARM_POOL", "1") != "1"
+    # TODO(masnesral): warm pool is broken, need to fix it


I was getting errors with this turned on. Does it pass for you?

Summary: Adds a "safe" parallel compile implementation that a) Popens a sub-process with an entry point we control, and b) Uses a ProcessPoolExecutor in that sub-processes to perform parallel compiles. This change essentially squashes these two implementations from jansel, but removes the "thread-based" approach since benchmarking revealed that compile-time performance was poor compared to the existing impl: #124682 #122941 This PR adds the implementation, but defaults to the existing "fork". I'll submit a separate change to enable. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

jamesjwu · 2024-05-27T17:58:23Z

torch/_inductor/compile_worker/__main__.py

+import sys
+import typing
+
+from torch._inductor.codecache import _set_triton_ptxas_path, caching_device_properties


_set_triton_ptxas_path isn't defined in torch._inductor.codecache, do you mean the one defined in compile_tasks.py?

Hmmm; I guess it works because codecache.py imports that function. But yeah, will fix

python torch/_inductor/compile_worker/main.py doesn't seem to run for me if you just call it directly, not sure if there's some special way to call it so it passes today

I'm perplexed. Seems to work fine for me:

$ python torch/_inductor/compile_worker/__main__.py --help usage: __main__.py [-h] [--workers WORKERS] [--parent PARENT] options: -h, --help show this help message and exit --workers WORKERS --parent PARENT

(but making change anyway)

eellison · 2024-05-23T02:36:33Z

torch/_inductor/codecache.py

+            # Wrapper around ProcessPoolExecutor forks in a new process we control
+            pool = SubprocPool(config.compile_threads)
+        else:
+            # ensure properties have been calculated before processes


nit: assert that the worker_start_method is in the other possible start_methods

eellison · 2024-05-28T17:43:25Z

torch/_inductor/compile_worker/subproc_pool.py

+    # Examples:
+    # A simple x + x + x script: 10ms seconds in the middle of the program, 2ms at startup
+    # tf_efficientnet_b0 benchmark: 50ms! in the middle of the program , 3ms at startup
+
+    # So we want to start the workers early when it is still cheap, and also to allow the workers to get
+    # ready before we have work for them.
+
+    # ProcessPoolExecutor also does not launch the workers until it finds a point when all the workers are idle.
+    # But if we waited until then fork time will be long and we will be waiting for the processes to initialize.


Are these comments still relevant ?

Lemme add a TODO and I'll benchmark

Summary: Adds a "safe" parallel compile implementation that a) Popens a sub-process with an entry point we control, and b) Uses a ProcessPoolExecutor in that sub-processes to perform parallel compiles. This change essentially squashes these two implementations from jansel, but removes the "thread-based" approach since benchmarking revealed that compile-time performance was poor compared to the existing impl: #124682 #122941 This PR adds the implementation, but defaults to the existing "fork". I'll submit a separate change to enable. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

masnesral · 2024-05-28T21:31:02Z

New benchmark result after properly warming the cache:

https://hud.pytorch.org/benchmark/compilers?startTime=Tue%2C%2021%20May%202024%2003%3A36%3A49%20GMT&stopTime=Tue%2C%2028%20May%202024%2003%3A36%3A49%20GMT&granularity=hour&suite=torchbench&mode=training&dtype=amp&lBranch=gh/masnesral/65/head&lCommit=4293f564e1bbd0724578cecc05e6159b4166ce93&rBranch=main&rCommit=b516de8cac9317155104d476056bae1d910a125c

masnesral · 2024-05-29T04:41:55Z

@pytorchbot merge

pytorchmergebot · 2024-05-29T04:44:28Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

Summary: Adds a "safe" parallel compile implementation that a) Popens a sub-process with an entry point we control, and b) Uses a ProcessPoolExecutor in that sub-processes to perform parallel compiles. This change essentially squashes these two implementations from jansel, but removes the "thread-based" approach since benchmarking revealed that compile-time performance was poor compared to the existing impl: pytorch#124682 pytorch#122941 This PR adds the implementation, but defaults to the existing "fork". I'll submit a separate change to enable. Pull Request resolved: pytorch#126816 Approved by: https://github.com/jansel

PaliC · 2024-05-31T12:34:39Z

@pytorchbot revert -m "causing internal regression" -c "ghfirst"

pytorchmergebot · 2024-05-31T12:36:20Z

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot · 2024-05-31T12:36:31Z

Reverting PR 126816 failed

Reason: Command git -C /home/runner/work/pytorch/pytorch revert --no-edit 6c81856dcaab477e748aeda4c575b89f9e4dc4dd returned non-zero exit code 1

Auto-merging torch/_inductor/codecache.py
CONFLICT (content): Merge conflict in torch/_inductor/codecache.py
CONFLICT (modify/delete): torch/_inductor/compile_worker/__main__.py deleted in parent of 6c81856dcaa ([inductor] Add a subprocess-based parallel compile (#126816)) and modified in HEAD.  Version HEAD of torch/_inductor/compile_worker/__main__.py left in tree.
error: could not revert 6c81856dcaa... [inductor] Add a subprocess-based parallel compile (#126816)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git revert --continue".
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
hint: Disable this message with "git config advice.mergeConflict false"

Details for Dev Infra team

Raised by workflow job

pytorch-bot bot added ciflow/inductor module: inductor labels May 21, 2024

masnesral mentioned this pull request May 21, 2024

[inductor] Enable subprocess-based parallel compile as the default #126817

Closed

masnesral added the topic: not user facing topic category label May 21, 2024

masnesral requested review from eellison and jansel May 21, 2024 22:23

masnesral commented May 21, 2024

View reviewed changes

jansel approved these changes May 22, 2024

View reviewed changes

jamesjwu mentioned this pull request May 27, 2024

[easy?] Move AsyncCompile to a different file #127235

Closed

jamesjwu reviewed May 27, 2024

View reviewed changes

eellison reviewed May 28, 2024

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label May 29, 2024

pytorchmergebot added the merging label May 29, 2024

pytorchmergebot added the Merged label May 29, 2024

pytorchmergebot closed this in 6c81856 May 29, 2024

pytorchmergebot removed the merging label May 29, 2024

masnesral mentioned this pull request May 29, 2024

[inductor] Enable subprocess-based parallel compile internally #127401

Closed

github-actions bot deleted the gh/masnesral/62/head branch July 1, 2024 02:00

peterbell10 mentioned this pull request Nov 4, 2024

Don't use fork method if it's not available on the platform triton-lang/triton#5051

Merged

[inductor] Add a subprocess-based parallel compile #126816

[inductor] Add a subprocess-based parallel compile #126816

Uh oh!

Conversation

masnesral commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/126816

✅ You can merge normally! (3 Unrelated Failures)

Uh oh!

masnesral commented May 21, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

masnesral commented May 28, 2024

Uh oh!

masnesral commented May 29, 2024

Uh oh!

pytorchmergebot commented May 29, 2024

Merge started

Uh oh!

PaliC commented May 31, 2024

Uh oh!

pytorchmergebot commented May 31, 2024

Uh oh!

pytorchmergebot commented May 31, 2024

Reverting PR 126816 failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

masnesral commented May 21, 2024 •

edited

Loading

pytorch-bot bot commented May 21, 2024 •

edited

Loading