You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2025/10/20 10:11:06 UTC
[PR] [SPARK-53955][CORE] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
pan3793 opened a new pull request, #52665:
URL: https://github.com/apache/spark/pull/52665
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'common/utils/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Prefer to detect Java Home from env `JAVA_HOME` on finding `jmap`, then System Properties `java.home`
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
https://stackoverflow.com/questions/45441516/difference-between-java-home-and-java-home
> `JAVA_HOME` points to the JDK installation path given by the Environment Variable.
>
> But `java.home` points to the JRE installation path.
Somehow, it returns the same value in JDK 11+ (according to the following article, users are still able to create a JRE by themselves?), but it returns a different value in prior JDK 8
https://adoptium.net/news/2021/12/eclipse-temurin-jres-are-back
```
➜ spark-3.5.7-bin-hadoop3 bin/spark-shell
Setting default log level to "WARN".
To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
25/10/20 17:59:07 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
Spark context Web UI available at http://HIH-D-25944Z:4040
Spark context available as 'sc' (master = local[*], app id = local-1760954347801).
Spark session available as 'spark'.
Welcome to
____ __
/ __/__ ___ _____/ /__
_\ \/ _ \/ _ `/ __/ '_/
/___/ .__/\_,_/_/ /_/\_\ version 3.5.7
/_/
Using Scala version 2.12.18 (OpenJDK 64-Bit Server VM, Java 17.0.15)
Type in expressions to have them evaluated.
Type :help for more information.
scala> System.getProperty("java.home")
res0: String = /home/chengpan/.sdkman/candidates/java/17.0.15-zulu
scala> System.getenv("JAVA_HOME")
res1: String = /home/chengpan/.sdkman/candidates/java/17.0.15-zulu
```
```
➜ spark-3.5.7-bin-hadoop3 bin/spark-shell
Setting default log level to "WARN".
To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
25/10/20 17:59:25 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
Spark context Web UI available at http://HIH-D-25944Z:4040
Spark context available as 'sc' (master = local[*], app id = local-1760954366052).
Spark session available as 'spark'.
Welcome to
____ __
/ __/__ ___ _____/ /__
_\ \/ _ \/ _ `/ __/ '_/
/___/ .__/\_,_/_/ /_/\_\ version 3.5.7
/_/
Using Scala version 2.12.18 (OpenJDK 64-Bit Server VM, Java 1.8.0_432)
Type in expressions to have them evaluated.
Type :help for more information.
scala> System.getProperty("java.home")
res0: String = /home/chengpan/.sdkman/candidates/java/8.0.432.fx-zulu/jre
scala> System.getenv("JAVA_HOME")
res1: String = /home/chengpan/.sdkman/candidates/java/8.0.432.fx-zulu
```
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as new features, bug fixes, or other behavior changes. Documentation-only updates are not considered user-facing changes.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
Manually verified.
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
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-53955][CORE] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3421931264
I agree with this change as we can see similar code in `SparkBuild.scala`
We have also see `sys.props("java.home")` in [AppStatusListenerSuite.scala](https://github.com/apache/spark/blob/2bb73fbdeb19f0a972786d3ea33d3263bf84ab66/core/src/test/scala/org/apache/spark/status/AppStatusListenerSuite.scala#L76).
Should we change it as well?
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3427324230
Thank you for rebasing, @pan3793 .
So, the remaining topic is the following previous comment about `incompatibility`. Could you revise the PR title by focusing on the case where we need to use `System.getenv("JAVA_HOME")` instead of `System.getProperty("java.home")`?
> Also, there is no compatibility gaurantees that I am aware of between different versions of jdk and jmap (for example, jdk11 jmap against jdk17 or vice versa) - if I missed any, please do let me know !
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap for JDK 8 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3461622919
Merged into branch-3.5. Thanks @pan3793 @dongjoon-hyun and @sarutak
--
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-53955][CORE] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #52665:
URL: https://github.com/apache/spark/pull/52665#discussion_r2444926039
##########
core/src/main/scala/org/apache/spark/util/Utils.scala:
##########
@@ -2112,7 +2112,8 @@ private[spark] object Utils
/** Return a heap dump. Used to capture dumps for the web UI */
def getHeapHistogram(): Array[String] = {
val pid = String.valueOf(ProcessHandle.current().pid())
- val jmap = System.getProperty("java.home") + "/bin/jmap"
+ val jmap = Option(System.getenv("JAVA_HOME"))
Review Comment:
@sarutak the suggestted approach is simpler but will load all env vars/properties into a new map, which say, is not as efficient as Java's method.
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3429358715
I must be clear on it. I already got your point about that but what I asked in the above is different. What I asked is more about the previous question when the different JDK versions occurs between `System.getenv("JAVA_HOME")` and `System.getProperty("java.home")`.
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap for JDK 8 [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3626784001
@mridulm Spark should correctly resolve `JAVA_HOME` env var or fail fast when launching by `spark-submit` or other shell scripts, but you are right, it's still possible that the user uses Spark as a library and creates `SparkContext` from their Java process.
How do you think about code below for Spark 3.5?
```scala
private lazy val jmapCmd: Option[String] = {
def jmapExists(maybeJavaHome: String): Boolean =
Files.exists(Paths.get(maybeJavaHome, "/bin/jmap"))
Option(System.getenv("JAVA_HOME")).filter(jmapExists)
.orElse(Option(System.getProperty("java.home")).filter(jmapExists))
.map(javaHome => javaHome + "/bin/jmap")
}
/** Return a heap dump. Used to capture dumps for the web UI */
def getHeapHistogram(): Array[String] = jmapCmd.map { jmap =>
// From Java 9+, we can use 'ProcessHandle.current().pid()'
val pid = getProcessName().split("@").head
val builder = new ProcessBuilder(jmap, "-histo:live", pid)
val p = builder.start()
val rows = ArrayBuffer.empty[String]
Utils.tryWithResource(new BufferedReader(new InputStreamReader(p.getInputStream()))) { r =>
var line = ""
while (line != null) {
if (line.nonEmpty) rows += line
line = r.readLine()
}
}
rows.toArray
}.getOrElse {
logWarning("Return empty heap histogram due to jcmd is unavailable.")
Array.empty
}
```
Additionally, I found that `jmap` is still possible to be missing after Java 17, for example,
```
$ docker run -it ubuntu:24.04 bash
# apt update
# apt install openjdk-25-jre-headless
# ll /usr/lib/jvm/java-25-openjdk-arm64/bin/
total 280
drwxr-xr-x 2 root root 4096 Dec 8 12:30 ./
drwxr-xr-x 6 root root 4096 Dec 8 12:30 ../
-rwxr-xr-x 1 root root 67480 Oct 23 09:23 java*
-rwxr-xr-x 1 root root 67504 Oct 23 09:23 jpackage*
-rwxr-xr-x 1 root root 67504 Oct 23 09:23 keytool*
-rwxr-xr-x 1 root root 67512 Oct 23 09:23 rmiregistry*
```
So, maybe we also want a variant method to find `jmap` for the master branch?
```scala
private lazy val jmapCmd: Option[String] = {
def jmapExists(maybeJavaHome: String): Boolean =
Files.exists(Paths.get(maybeJavaHome, "/bin/jmap"))
Option(System.getProperty("java.home")).filter(jmapExists)
.map(javaHome => javaHome + "/bin/jmap")
}
```
--
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-53955][CORE] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3421440990
cc @dongjoon-hyun as you are the author of this feature
cc @mridulm, I think the `System.getProperty("java.home")` comes from your suggestion, https://github.com/apache/spark/pull/41709#discussion_r1240849467. Could you please take a look at this change?
Note, this causes a real issue only on Spark 3.5 with JDK 8, but from my understanding, we should always prefer using `JAVA_HOME` if possible.
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap for JDK 8 [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3429448238
@dongjoon-hyun I see your point, it's a dedicated issue for JDK, I modified the PR title by adding "for JDK 8" suffix, do you think it's clear now?
--
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-53955][CORE] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3421973178
@sarutak I think we should take them one by one.
[AppStatusListenerSuite.scala](https://github.com/apache/spark/blob/2bb73fbdeb19f0a972786d3ea33d3263bf84ab66/core/src/test/scala/org/apache/spark/status/AppStatusListenerSuite.scala#L76) you mentioned is actually affecting display in Spark UI, changing this may surprise users, I lean towards not touching it, but can do if someone thinks we should.
In `SparkBuild.scala`, I think the Spark devs already noticed that, that's why the parent dir of `java.home` is used, see https://github.com/apache/spark/pull/5441
```
.orElse(sys.props.get("java.home").map { p => new File(p).getParentFile().getAbsolutePath() })
```
it's correct for JDK 8 and prior. I think we should change it to
```
.orElse(sys.props.get("java.home"))
```
for Spark 4.0 and master, since it's not sure for JDK 17+
--
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-53955][CORE] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #52665:
URL: https://github.com/apache/spark/pull/52665#discussion_r2444950162
##########
core/src/main/scala/org/apache/spark/util/Utils.scala:
##########
@@ -2112,7 +2112,8 @@ private[spark] object Utils
/** Return a heap dump. Used to capture dumps for the web UI */
def getHeapHistogram(): Array[String] = {
val pid = String.valueOf(ProcessHandle.current().pid())
- val jmap = System.getProperty("java.home") + "/bin/jmap"
+ val jmap = Option(System.getenv("JAVA_HOME"))
Review Comment:
@pan3793
OK, I understand using `System.getenv` instead of `sys.env` is intended.
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap for JDK 8 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed pull request #52665: [SPARK-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap for JDK 8
URL: https://github.com/apache/spark/pull/52665
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3427992126
@dongjoon-hyun this is not about incompatibility between different versions of JDK, the real issue is, in JDK8, `jmap` exists under `JAVA_HOME/bin`, but not under `JAVA_HOME/jre/bin`. I updated the PR description to reflect it.
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap for JDK 8 [spark]
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3619764499
hmm, JAVA_HOME can point to arbitrary path and need not be valid - right ?
Given that, we should check for validity of path (more precisely if jmap is actually present at `$JAVA_HOME/bin/jmap` ?
Example:
```
$ export JAVA_HOME=/dev/null
$ scala
Welcome to Scala 2.13.16 (OpenJDK 64-Bit Server VM, Java 17.0.17).
Type in expressions for evaluation. Or try :help.
scala> System.getenv("JAVA_HOME")
val res1: String = /dev/null
scala> System.getProperty("java.home")
val res2: String = /usr/lib/jvm/java-17-openjdk-amd64
scala>
```
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap for JDK 8 [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3440476470
@dongjoon-hyun thanks for your review and approval
cc @HyukjinKwon @LuciferYang @viirya @mridulm could you please take a look when you have time? thank you in advance
--
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-53955][CORE] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #52665:
URL: https://github.com/apache/spark/pull/52665#discussion_r2444926039
##########
core/src/main/scala/org/apache/spark/util/Utils.scala:
##########
@@ -2112,7 +2112,8 @@ private[spark] object Utils
/** Return a heap dump. Used to capture dumps for the web UI */
def getHeapHistogram(): Array[String] = {
val pid = String.valueOf(ProcessHandle.current().pid())
- val jmap = System.getProperty("java.home") + "/bin/jmap"
+ val jmap = Option(System.getenv("JAVA_HOME"))
Review Comment:
@sarutak the suggestted approach is simpler but will load all env vars/properties into a new map, that's say, is not efficient that Java's method.
--
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-53955][CORE] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #52665:
URL: https://github.com/apache/spark/pull/52665#discussion_r2444907961
##########
core/src/main/scala/org/apache/spark/util/Utils.scala:
##########
@@ -2112,7 +2112,8 @@ private[spark] object Utils
/** Return a heap dump. Used to capture dumps for the web UI */
def getHeapHistogram(): Array[String] = {
val pid = String.valueOf(ProcessHandle.current().pid())
- val jmap = System.getProperty("java.home") + "/bin/jmap"
+ val jmap = Option(System.getenv("JAVA_HOME"))
Review Comment:
```suggestion
val jmap = sys.env.get("JAVA_HOME").getOrElse(sys.props("java.home")) + "/bin/jmap"
```
--
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-53955][CORE][3.5] Prefer to detect Java Home from env JAVA_HOME on finding jmap [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #52665:
URL: https://github.com/apache/spark/pull/52665#issuecomment-3424505568
@dongjoon-hyun I have retargeted this PR to branch-3.5
--
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