[jdk17u-dev] RFR: 8324646: Avoid Class.forName in SecureRandom constructor [v2]
Oli Gillespie
ogillespie at openjdk.org
Wed Apr 3 08:43:26 UTC 2024
> Improve performance of SecureRandom constructor by backporting (both clean) two changes:
>
> 1. [JDK-8280970](https://bugs.openjdk.org/browse/JDK-8280970) removes some unused code in Provider.java. This is not really functionally needed, but it changes the same areas of code and makes the actual performance fix apply cleanly.
> 2. [JDK-8324646](https://bugs.openjdk.org/browse/JDK-8324646) is the actual performance fix - avoiding Class.forName calls in every construction.
>
> Note - because of the unused code still present in JDK17, there is actually an alternative fix which makes use of it, which is very simple:
>
>
> diff --git a/src/java.base/share/classes/java/security/Provider.java b/src/java.base/share/classes/java/security/Provider.java
> index af8ebeeda57..28bf642d0c8 100644
> --- a/src/java.base/share/classes/java/security/Provider.java
> +++ b/src/java.base/share/classes/java/security/Provider.java
> @@ -1851,7 +1851,7 @@ public abstract class Provider extends Properties {
> null : constructorParameter.getClass();
> } else {
> ctrParamClz = cap.constructorParameterClassName == null?
> - null : Class.forName(cap.constructorParameterClassName);
> + null : cap.getConstructorParameterClass(); // actually make use of the cached class!
> if (constructorParameter != null) {
> if (ctrParamClz == null) {
> throw new InvalidParameterException
>
>
> This has the same performance benefits as making the two backports. But, it means 17 will be diverged from later versions, and I think the backported fix is cleaner overall.
>
> Benchmark results (`make test TEST=micro:org.openjdk.bench.java.security.SecureRandomBench`):
>
>
> Before: 2614 ± 127 ns/op
> After: 2150 ± 116 ns/op
Oli Gillespie has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
- Merge remote-tracking branch 'origin/master' into backport-8324646-class-for-name-3
- Backport 8ef918d6678437a5b351b172bb4cf144eeaa975f
- Backport 63e11cfa3f887515ca36ab5147c3e6fa540978d3
-------------
Changes:
- all: https://git.openjdk.org/jdk17u-dev/pull/2310/files
- new: https://git.openjdk.org/jdk17u-dev/pull/2310/files/5b15fdb2..280a1fcc
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk17u-dev&pr=2310&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk17u-dev&pr=2310&range=00-01
Stats: 18933 lines in 478 files changed: 4610 ins; 12996 del; 1327 mod
Patch: https://git.openjdk.org/jdk17u-dev/pull/2310.diff
Fetch: git fetch https://git.openjdk.org/jdk17u-dev.git pull/2310/head:pull/2310
PR: https://git.openjdk.org/jdk17u-dev/pull/2310
More information about the jdk-updates-dev
mailing list