You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "nchammas (via GitHub)" <gi...@apache.org> on 2024/01/11 04:15:23 UTC
[PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
nchammas opened a new pull request, #44680:
URL: https://github.com/apache/spark/pull/44680
### What changes were proposed in this pull request?
Upgrade to Sphinx 4.5.0, which is the [latest in the 4.x line][1] and includes the [fix for parallel builds on macOS][2].
Enable parallel Sphinx workers to build the Python API docs.
I experimented with a few different values, and `auto` seems to work best. Configuring 4 workers seems to yield the same improvement as `auto`, suggesting parallelization beyond that is ineffective due to some sort of resource contention. But I left it as `auto` since that's more dynamic and may work better on CI.
On my 16-core Intel workstation, the runtime of `make html` was cut by ~60%.
```sh
# `make html` @ master
real 43m51.167s
user 41m43.526s
sys 0m39.651s
# `make html` with parallel workers
real 17m8.424s
user 174m42.051s
sys 5m8.824s
```
[1]: https://www.sphinx-doc.org/en/master/changes.html#release-4-5-0-released-mar-28-2022
[2]: https://github.com/sphinx-doc/sphinx/pull/9793
### Why are the changes needed?
This saves developer time (and may also save CI time).
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
I manually built and reviewed the docs using:
```sh
SKIP_SCALADOC=1 SKIP_SQLDOC=1 SKIP_RDOC=1 time bundle exec jekyll build
```
### Was this patch authored or co-authored using generative AI tooling?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44680:
URL: https://github.com/apache/spark/pull/44680#issuecomment-1886254077
cc @itholic and @HyukjinKwon. This PR builds on #44012.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #44680:
URL: https://github.com/apache/spark/pull/44680#discussion_r1762087346
##########
python/docs/Makefile:
##########
@@ -16,7 +16,7 @@
# Minimal makefile for Sphinx documentation
# You can set these variables from the command line.
-SPHINXOPTS ?= "-W"
+SPHINXOPTS ?= "-W" "-j" "auto"
Review Comment:
Unfortunately, this breaks Python API doc generation in many core machines because this means the number of parallel `SparkSubmit` invocation of PySpark. In addition, given that each `PySpark` currently is launched with `local[*]`, this ends up `N * N` `pyspark.daemon`.
As of today, this setting seems to work on low-core machine like `GitHub Action` runners (4 cores). This breaks `Python` documentations build even on M3 Max environment and this is worse on large EC2 machines (c7i.24xlarge). You can see the failure locally like this.
```
$ build/sbt package -Phive-thriftserver
$ cd python/docs
$ make html
...
java.lang.OutOfMemoryError: Java heap space
...
24/09/16 14:09:55 WARN PythonRunner: Incomplete task 7.0 in stage 30 (TID 177) interrupted: Attempting to kill Python Worker
...
make: *** [html] Error 2
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #44680:
URL: https://github.com/apache/spark/pull/44680#discussion_r1762087346
##########
python/docs/Makefile:
##########
@@ -16,7 +16,7 @@
# Minimal makefile for Sphinx documentation
# You can set these variables from the command line.
-SPHINXOPTS ?= "-W"
+SPHINXOPTS ?= "-W" "-j" "auto"
Review Comment:
Unfortunately, this breaks Python API docs in many core machines because this means the number of parallel `SparkSubmit` invocation of PySpark. In addition, given that each `PySpark` currently is launched with `local[*]`, this ends up `N * N` `pyspark.daemon`.
In short, this setting is only valid on low-core machine like `GitHub Action` runners (4 cores). This breaks `Python` documentations build even on M3 Max environment and this is worse on large EC2 machines. You can see the failure locally like this.
```
$ build/sbt package -Phive-thriftserver
$ cd python/docs
$ make html
...
java.lang.OutOfMemoryError: Java heap space
...
24/09/16 14:09:55 WARN PythonRunner: Incomplete task 7.0 in stage 30 (TID 177) interrupted: Attempting to kill Python Worker
...
make: *** [html] Error 2
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #44680:
URL: https://github.com/apache/spark/pull/44680#issuecomment-1886565032
Merged to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #44680:
URL: https://github.com/apache/spark/pull/44680#discussion_r1762087346
##########
python/docs/Makefile:
##########
@@ -16,7 +16,7 @@
# Minimal makefile for Sphinx documentation
# You can set these variables from the command line.
-SPHINXOPTS ?= "-W"
+SPHINXOPTS ?= "-W" "-j" "auto"
Review Comment:
Unfortunately, this breaks Python API docs in many core machines because this means the number of parallel `SparkSubmit` invocation of PySpark. In addition, given that each `PySpark` currently is launched with `local[*]`, this ends up `N * N` `pyspark.daemon`.
As of today, this setting seems to work on low-core machine like `GitHub Action` runners (4 cores). This breaks `Python` documentations build even on M3 Max environment and this is worse on large EC2 machines. You can see the failure locally like this.
```
$ build/sbt package -Phive-thriftserver
$ cd python/docs
$ make html
...
java.lang.OutOfMemoryError: Java heap space
...
24/09/16 14:09:55 WARN PythonRunner: Incomplete task 7.0 in stage 30 (TID 177) interrupted: Attempting to kill Python Worker
...
make: *** [html] Error 2
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #44680: [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs
URL: https://github.com/apache/spark/pull/44680
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44680:
URL: https://github.com/apache/spark/pull/44680#issuecomment-2354228907
Here is a PR with new default setting while keeping the speed of GitHub Action CI. The users also override this via `SPHINXOPT`.
- #48129
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #44680:
URL: https://github.com/apache/spark/pull/44680#discussion_r1762087346
##########
python/docs/Makefile:
##########
@@ -16,7 +16,7 @@
# Minimal makefile for Sphinx documentation
# You can set these variables from the command line.
-SPHINXOPTS ?= "-W"
+SPHINXOPTS ?= "-W" "-j" "auto"
Review Comment:
Unfortunately, this breaks Python API docs in many core machines because this means the number of parallel `SparkSubmit` invocation of PySpark. In addition, given that each `PySpark` currently is launched with `local[*]`, this ends up `N * N` `pyspark.daemon`.
As of today, this setting seems to work on low-core machine like `GitHub Action` runners (4 cores). This breaks `Python` documentations build even on M3 Max environment and this is worse on large EC2 machines (c7i.24xlarge). You can see the failure locally like this.
```
$ build/sbt package -Phive-thriftserver
$ cd python/docs
$ make html
...
java.lang.OutOfMemoryError: Java heap space
...
24/09/16 14:09:55 WARN PythonRunner: Incomplete task 7.0 in stage 30 (TID 177) interrupted: Attempting to kill Python Worker
...
make: *** [html] Error 2
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46668][DOCS] Parallelize Sphinx build of Python API docs [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on code in PR #44680:
URL: https://github.com/apache/spark/pull/44680#discussion_r1762430076
##########
python/docs/Makefile:
##########
@@ -16,7 +16,7 @@
# Minimal makefile for Sphinx documentation
# You can set these variables from the command line.
-SPHINXOPTS ?= "-W"
+SPHINXOPTS ?= "-W" "-j" "auto"
Review Comment:
> As of today, this setting seems to work on low-core machine like `GitHub Action` runners (4 cores).
It seems something about the build has changed since this PR was merged in ~8 months ago. As stated in the PR description, I tested `auto` on a 16-core Intel and it worked fine. And I remember running the build many times to get a representative runtime to report.
> this ends up `N * N` `pyspark.daemon`
Perhaps this was not the case back in January, for some reason. 🤷♂️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org