[SPARK-45392][CORE][SQL][SS] Replace Class.newInstance() with Class.getDeclaredConstructor().newInstance()
#43193
Conversation
|
Given this is deprecated, we have to move our usage out ... but one caveat is the loss of cached constructor we can no longer leverage - something to watch out for if this is in potentially perf sensitive paths as you work through the draft. |
|
Good point~ one way is to add a Guava Cache to cache these no-argument Constructors to alleviate this problem, another way is to maintain the status quo and continue to use the I'm testing the addition of a Guava Cache. If the tests pass, I will update the code first. |
|
Note - we need the cache only if we think this is in a hot path, else might not be worth it :-) |
This reverts commit 00b9bd2.
|
Personally, I don't think they are hotpaths, let's use |
Class.newInstance() with Class.getDeclaredConstructor().newInstance()Class.newInstance() with Class.getDeclaredConstructor().newInstance()
|
There is also use in |
done ~ |
Class.newInstance() with Class.getDeclaredConstructor().newInstance()Class.newInstance() with Class.getDeclaredConstructor().newInstance()
|
Merged to master for Apache Spark 4.0.0. |
|
Thanks @dongjoon-hyun @mridulm and @srowen ~ |
… source ### What changes were proposed in this pull request? This is a followup of #43193 to fix a behavior change. `Class.getDeclaredConstructor().newInstance()` will wrap the exception with `InvocationTargetException`, and this PR unwraps it to still throw the original exception from the data source ### Why are the changes needed? restore a behavior change ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? new test ### Was this patch authored or co-authored using generative AI tooling? no Closes #43446 from cloud-fan/follow. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: yangjie01 <yangjie01@baidu.com>
What changes were proposed in this pull request?
This PR replaces
Class.newInstance()withClass.getDeclaredConstructor().newInstance()to clean up the use of deprecated APIs refer tohttps://github.com/openjdk/jdk/blob/dfacda488bfbe2e11e8d607a6d08527710286982/src/java.base/share/classes/java/lang/Class.java#L557-L583
Note: The new API no longer has the
cachedConstructorcapability that comes with theClass.newInstance(). Currently, I think there are no hotspots in the places that have been fixed. If hotspots are indeed discovered in the future, they can be optimized by adding a Loading Cache.Why are the changes needed?
Clean up the use of deprecated APIs.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Pass GitHub Actions
Was this patch authored or co-authored using generative AI tooling?
No