[SPARK-53955][CORE] Prefer to detect Java Home from env JAVA_HOME on finding jmap

[pan3793]

What changes were proposed in this pull request?

Prefer to detect Java Home from env JAVA_HOME on finding jmap, then System Properties java.home

Why are the changes needed?

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 JDK 8 or prior

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?

No.

How was this patch tested?

Manually verified.

Was this patch authored or co-authored using generative AI tooling?

No.

[pan3793]

cc @dongjoon-hyun as you are the author of this feature

cc @mridulm, I think the System.getProperty("java.home") comes from your suggestion, #41709 (comment). 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.

[sarutak]

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.
Should we change it as well?

[pan3793]

@sarutak I think we should take them one by one.

AppStatusListenerSuite.scala 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 #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 true for JDK 17+