RFR: 8324648: Avoid NoSuchMethodError when instantiating NativePRNG

[Oli Gillespie]

On Wed, 24 Jan 2024 15:42:05 GMT, Oli Gillespie <ogillespie at 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