Skip to content

[SPARK-55996][CORE] Remove default jdk.reflect.useDirectMethodHandle=false#54815

Closed
pan3793 wants to merge 4 commits intoapache:masterfrom
pan3793:useDirectMethodHandle
Closed

[SPARK-55996][CORE] Remove default jdk.reflect.useDirectMethodHandle=false#54815
pan3793 wants to merge 4 commits intoapache:masterfrom
pan3793:useDirectMethodHandle

Conversation

@pan3793
Copy link
Copy Markdown
Member

@pan3793 pan3793 commented Mar 15, 2026

What changes were proposed in this pull request?

This PR removes the auto-added Java options -Djdk.reflect.useDirectMethodHandle=false, which was added via SPARK-40729 to workaround Spark Shell issues (caused by ClosureCleaner) on JDK 18 to 21.

Why are the changes needed?

-Djdk.reflect.useDirectMethodHandle=false is a backdoor to disable JEP 416: Reimplement Core Reflection with Method Handles optimization to use the legacy JDK behaviors on JDK 18 ~ 21. Now, the Spark Shell issue was resolved by SPARK-53226/SPARK-52088 (#52956), we can remove this workaround.

Does this PR introduce any user-facing change?

No, but if the users hit bugs with the ClonedIndyLambda mode on Java 21, they are still able to add -Djdk.reflect.useDirectMethodHandle=false via spark.driver.extraJavaOptions to switch back to the legacy approach.

How was this patch tested?

Pass GHA, also update the JsonBenchmark-jdk21-results.txt(Jackson was mentioned in JEP 416 benchmark cases).

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

No.

name: Run
uses: ./.github/workflows/build_and_test.yml
with:
java: 21 No newline at end of file
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will revert later

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LuciferYang
Copy link
Copy Markdown
Contributor

We'd better conduct a more thorough evaluation of the impact on performance, as this could lead to MethodHandle being used by default across the board to replace the traditional reflection-based implementation approach.

Personally, I would prefer to remove it only when Java 25 becomes the default JDK, as by that time it will no longer have any function or impact.

@pan3793
Copy link
Copy Markdown
Member Author

pan3793 commented Mar 16, 2026

@LuciferYang, I'm optimistic about this because the JEP 416 says:

This degradation may, however, not have much effect on the performance of real-world applications. We ran several serialization and deserialization benchmarks using real-world libraries and found no degradation ...

but I'm okay to hold this until Spark moves to JDK 25 as the default JDK, if you have concerns.

@sarutak
Copy link
Copy Markdown
Member

sarutak commented Mar 16, 2026

I don’t think it’s necessary to remove it right now, but, if there are any issues, we can find them in the next preview release.
The change itself LGTM.

Copy link
Copy Markdown
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @pan3793 , @LuciferYang , @sarutak . All opinions make sense to me.

From my side, I'm more positive for removing this in order to reveal any issues (of not only the Apache Spark but also the libraries) during the Apache Spark 4.2.0 release process.

If there is any issues, we can add this back easily during the Apache Spark 4.2.0 RC period. So, +1.

Copy link
Copy Markdown
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, I also accept the changes in the current pr

@pan3793 pan3793 closed this in f64a7df Mar 17, 2026
@pan3793
Copy link
Copy Markdown
Member Author

pan3793 commented Mar 17, 2026

thanks, merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants