You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by do...@apache.org on 2024/04/28 00:39:08 UTC
(spark) branch master updated: [SPARK-48021][ML][BUILD] Add `--add-modules=jdk.incubator.vector` to `JavaModuleOptions`
This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 64d321926bbc [SPARK-48021][ML][BUILD] Add `--add-modules=jdk.incubator.vector` to `JavaModuleOptions`
64d321926bbc is described below
commit 64d321926bbcede05d1c145405d503b3431f185b
Author: panbingkun <pa...@baidu.com>
AuthorDate: Sat Apr 27 17:38:55 2024 -0700
[SPARK-48021][ML][BUILD] Add `--add-modules=jdk.incubator.vector` to `JavaModuleOptions`
### What changes were proposed in this pull request?
The pr aims to:
- add `--add-modules=jdk.incubator.vector` to `JavaModuleOptions`
- remove `jdk.incubator.foreign` and `-Dforeign.restricted=warn` from `SparkBuild.scala`
### Why are the changes needed?
1.`jdk.incubator.vector`
First introduction: https://github.com/apache/spark/pull/30810
https://github.com/apache/spark/pull/30810/files#diff-6f545c33f2fcc975200bf208c900a600a593ce6b170180f81e2f93b3efb6cb3e
<img width="1045" alt="image" src="https://github.com/apache/spark/assets/15246973/6ac7919a-5d82-475c-b8a2-7d9de71acacc">
Why should we add `--add-modules=jdk.incubator.vector` to `JavaModuleOptions`,
Because when we only add `--add-modules=jdk.incubator.vector` to `SparkBuild.scala`, it will only take effect when compiling, as follows:
```
build/sbt "mllib-local/Test/runMain org.apache.spark.ml.linalg.BLASBenchmark"
...
```
<img width="619" alt="image" src="https://github.com/apache/spark/assets/15246973/54d5f55f-cefe-4126-b255-69488f8699a6">
However, when we use `spark-submit`, it is as follows:
```
./bin/spark-submit --class org.apache.spark.ml.linalg.BLASBenchmark /Users/panbingkun/Developer/spark/spark-community/mllib-local/target/scala-2.13/spark-mllib-local_2.13-4.0.0-SNAPSHOT-tests.jar
```
<img width="1399" alt="image" src="https://github.com/apache/spark/assets/15246973/8e02fa93-fef4-4cdc-96bd-908b3e9baea1">
Obviously, `--add-modules=jdk.incubator.vector` does not take effect in the `Spark runtime`, so I propose adding `--add-modules=jdk.incubator.vector` to the `JavaModuleOptions`(`Spark runtime options`) so that we can improve `performance` by using `hardware-accelerated BLAS operations` by default.
After this patch(add `--add-modules=jdk.incubator.vector` to the `JavaModuleOptions`), as follows:
<img width="1399" alt="image" src="https://github.com/apache/spark/assets/15246973/da7aa494-0d3c-4c60-9991-e7cd29a1cec5">
2.`jdk.incubator.foreign` and `-Dforeign.restricted=warn`
A.First introduction: https://github.com/apache/spark/pull/32253
https://github.com/apache/spark/pull/32253/files#diff-6f545c33f2fcc975200bf208c900a600a593ce6b170180f81e2f93b3efb6cb3e
<img width="1041" alt="image" src="https://github.com/apache/spark/assets/15246973/3f526019-c389-4e60-ab2a-7777f8e99cfb">
Use `dev.ludovic.netlib:blas:1.3.2`, the class `ForeignLinkerBLAS` uses `jdk.incubator.foreign.*` in this version, so we need to add `jdk.incubator.foreign` and `-Dforeign.restricted=warn` to `SparkBuild.scala`
https://github.com/apache/spark/pull/32253/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8
<img width="497" alt="image" src="https://github.com/apache/spark/assets/15246973/4fd35e96-0da2-4456-a3f6-6b57ad2e9b64">
https://github.com/luhenry/netlib/blob/v1.3.2/blas/src/main/java/dev/ludovic/netlib/blas/ForeignLinkerBLAS.java#L36
<img width="743" alt="image" src="https://github.com/apache/spark/assets/15246973/4b7e3bd1-4650-4c7d-bdb4-c1761d48d478">
However, with the iterative development of `dev.ludovic.netlib`, `ForeignLinkerBLAS` has experienced one `major` change, as following:
https://github.com/luhenry/netlib/commit/48e923c3e5e84560139eb25b3c9df9873c05e41d
<img width="452" alt="image" src="https://github.com/apache/spark/assets/15246973/7ba30b19-00c7-4cc4-bea7-a6ab4b326ad8">
From now on (V3.0.0), `jdk.incubator.foreign.*` will not be used in `dev.ludovic.netlib`
Currently, Spark has used the `dev.ludovic.netlib` of version `v3.0.3`. In this version, `ForeignLinkerBLAS` has be removed.
https://github.com/apache/spark/blob/master/pom.xml#L191
Double check (`jdk.incubator.foreign` cannot be found in the `netlib` source code):
<img width="674" alt="image" src="https://github.com/apache/spark/assets/15246973/5c6c6d73-6a5d-427a-9fb4-f626f02335ca">
So we can completely remove options `jdk.incubator.foreign` and `-Dforeign.restricted=warn`.
B.For JDK 21
(PS: This is to explain the historical reasons for the differences between the current code logic and the initial ones)
(Just because `Spark` made changes to support `JDK 21`)
https://issues.apache.org/jira/browse/SPARK-44088
<img width="1350" alt="image" src="https://github.com/apache/spark/assets/15246973/34e7e7e8-4e72-470e-abc0-d79406ad25e5">
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- Manually test
- Pass GA.
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #46246 from panbingkun/test_spark_build.
Authored-by: panbingkun <pa...@baidu.com>
Signed-off-by: Dongjoon Hyun <dh...@apple.com>
---
.../main/java/org/apache/spark/launcher/JavaModuleOptions.java | 1 +
project/SparkBuild.scala | 9 ++-------
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/launcher/src/main/java/org/apache/spark/launcher/JavaModuleOptions.java b/launcher/src/main/java/org/apache/spark/launcher/JavaModuleOptions.java
index dc5840185d62..7b23db052e8b 100644
--- a/launcher/src/main/java/org/apache/spark/launcher/JavaModuleOptions.java
+++ b/launcher/src/main/java/org/apache/spark/launcher/JavaModuleOptions.java
@@ -27,6 +27,7 @@ package org.apache.spark.launcher;
public class JavaModuleOptions {
private static final String[] DEFAULT_MODULE_OPTIONS = {
"-XX:+IgnoreUnrecognizedVMOptions",
+ "--add-modules=jdk.incubator.vector",
"--add-opens=java.base/java.lang=ALL-UNNAMED",
"--add-opens=java.base/java.lang.invoke=ALL-UNNAMED",
"--add-opens=java.base/java.lang.reflect=ALL-UNNAMED",
diff --git a/project/SparkBuild.scala b/project/SparkBuild.scala
index 1bcc9c893393..9d2ee6077d11 100644
--- a/project/SparkBuild.scala
+++ b/project/SparkBuild.scala
@@ -294,13 +294,8 @@ object SparkBuild extends PomBuild {
publishLocal := Seq((MavenCompile / publishLocal), (SbtCompile / publishLocal)).dependOn.value,
javaOptions ++= {
- val versionParts = System.getProperty("java.version").split("[+.\\-]+", 3)
- val major = versionParts(0).toInt
- if (major >= 21) {
- Seq("--add-modules=jdk.incubator.vector", "-Dforeign.restricted=warn")
- } else {
- Seq("--add-modules=jdk.incubator.vector,jdk.incubator.foreign", "-Dforeign.restricted=warn")
- }
+ // for `dev.ludovic.netlib.blas` which implements such hardware-accelerated BLAS operations
+ Seq("--add-modules=jdk.incubator.vector")
},
(Compile / doc / javacOptions) ++= {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org