On Wed, 24 Jan 2024 15:42:05 GMT, Oli Gillespie <ogilles...@openjdk.org> wrote:
> A typical call to `new SecureRandom()` is slowed down by looking for a > constructor in NativePRNG which takes `java.security.SecureRandomParameters`. > NativePRNG does not have such a constructor, so the search fails > [here](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/security/Provider.java#L1951-L1957), > incurring all the cost of the lookup and creating a subsequent exception. > > Creating a dummy constructor which takes and ignores this parameter will > speed up `new SecureRandom()` calls significantly. > > The benchmark from https://github.com/openjdk/jdk/pull/17559 shows around 80% > reduction in time taken to create a new SecureRandom with NativePRNG (default > on my machine). > > > Before > SecureRandomBench.newSecureRandom avgt 2930 ± 50 ns/op > > After > SecureRandomBench.newSecureRandom avgt 510 ± 16 ns/op The more I think about it, adding a new public constructor seems a bit ugly and potentially confusing. So I'll offer a few more ideas. Which do you think is best? 1. New public constructor with IllegalArgumentException if params != null // constructor, unused argument to speed up lookups from Provider public NativePRNG(SecureRandomParameters params) { this(); if (params != null) { throw new IllegalArgumentException("Unsupported params: " + params.getClass()); } } 2. Use `getConstructors()` instead of `getConstructor(...)`, and manually look for the match (basically implement our own `tryGetConstructor`). This way, we avoid the exception which is the main cost of the current approach, but don't need to add any special cases. Benchmark says this is only a little slower than 1. (550ns vs 510ns). // new style with params for (Constructor<?> c : getImplClass().getConstructors()) { Class<?>[] args = c.getParameterTypes(); if (args.length == 1 && args[0].equals(ctrParamClz)) { // found match return c.newInstance(ctorParamObj); } } // old style without params if (ctorParamObj == null) { return newInstanceOf(); } else { throw new NoSuchMethodException("..."); } 3. Add explicit handling of NativePRNG inside Provider. static HashSet<String> NO_ARG_SECURE_RANDOMS = new HashSet<>(); static { NO_ARG_SECURE_RANDOMS.add("sun.security.provider.NativePRNG"); NO_ARG_SECURE_RANDOMS.add("sun.security.provider.NativePRNG$Blocking"); NO_ARG_SECURE_RANDOMS.add("sun.security.provider.NativePRNG$NonBlocking"); }; private Object newInstanceUtil(Class<?> ctrParamClz, Object ctorParamObj) throws Exception { if (ctrParamClz == null) { return newInstanceOf(); } else if (ctorParamObj == null && NO_ARG_SECURE_RANDOMS.contains(getImplClass().getName())) { return newInstanceOf(); } else { 4. Add some kind of caching in Provider to only pay the cost once (not coded up yet, potentially a bit annoying to also save the NSME if we want to keep that behaviour). ------------- PR Comment: https://git.openjdk.org/jdk/pull/17560#issuecomment-1921098319
