newInstance() Optimization for java.util classes (discussion)

[rmnattas]

While working on the performance of a specific workload (CoreNLP), the code uses a Generics class that makes coding easier for them.

This Generics class uses Class.newInstance() and @SuppressWarnings("unchecked”), and that moves checks away from the javac compilation to runtime checks. One of which is checkMemberAccess() which requires a stack walk to getCallerClass().

I believe java.util classes can’t be overridden, even by custom class loaders. So I’m wondering if it’s possible and beneficial overall to skip the checkMemberAccess() and the stack walk when instantiating a class under java.util (or wider?) as they’re always valid to instantiate from any class. Unless there’s cases that I don't know about.

Cost would be the check (package name check) during newInstance() or maybe set some flag in the Class object when constructed if better.

Any thoughts?

PS: Even though Class.newInstance() is deprecated since Java 9 but the idea can still apply.

[rmnattas]

@pshipton @tajila @zl-wang

[pshipton]

The security check for bootstrap classes is always required, to check if the package is in the package.access list. It would be very unusual for java.util to be a restricted package, but a custom SecurityManager could rely on the call to checkPackageAccess() to do something else so it does need to be called.

Looking at the code, I think the parameters used to call ReflectUtil.needsPackageAccessCheck() defeats some optimization, but basically we could avoid calling getCallerClass() if the classloader of the Class is the bootstrap loader, since we don't need the caller loader to determine that checkPackageAccess() should be called. Although there is another case, if the Class is a proxy class.

Where do you see checkMemberAccess() called from? newInstance() doesn't call it.

[rmnattas]

Where do you see checkMemberAccess() called from? newInstance() doesn't call it.

Didn't know about OpenJ9's Class.java and I was looking at OpenJDK's Class.java, hence some initial comments don't apply.

---

Custom SecurityManager

a custom SecurityManager could rely on the call to checkPackageAccess() to do something else so it does need to be called.

That's possible, but wondering about how frequent this is. This will eliminate the package.access check for java.util classes when there's a SecurityManager set. New to this code but as far as I see the package.access check only happens, in checkMemberAccess(), if a SecurityManager is set. Otherwise newInstanceImpl() only calls checkVisability().

Class.newInstance():
SecurityManager security = System.getSecurityManager();
if (security != null) {
    ClassLoader callerClassLoader = ClassLoader.getStackClassLoader(1);
    checkNonSunProxyMemberAccess(security, callerClassLoader, Member.PUBLIC);
}
...
return (T)J9VMInternals.newInstanceImpl(this);

---

ReflectUtil.needsPackageAccessCheck()

we could avoid calling getCallerClass() if the classloader of the Class is the bootstrap loader

I'll look more into this. It seems the caller class loader is used in a condition but possibly only walking the stack when needed.

---

Merging two stack walks

Looking at the code, it seems that there's the possibility of merging two stack walks (getStackClassLoader(1) & getStackClass(1)) into one and getting both the caller class and caller class loader.

Class.newInstance():
SecurityManager security = System.getSecurityManager();
if (security != null) {
    ClassLoader callerClassLoader = ClassLoader.getStackClassLoader(1);
    checkNonSunProxyMemberAccess(security, callerClassLoader, Member.PUBLIC);
}

/*[IF JAVA_SPEC_VERSION >= 12]*/
Class<?> callerClazz = getStackClass(1);
if (callerClazz.classLoader == ClassLoader.bootstrapClassLoader) {

---

Skipping stack walk in newInstanceImpl()

The main stack walk is apparently in newInstanceImpl(), and the result is used to call checkVisability(). This is the original optimization idea, to get the package name for the object being constructed and if it's a java.util class then we can skip the stack walking.

newInstanceImpl() Code

[rmnattas]

Prototype Results

Custom SecurityManager

Benefit is limited scoped, positive and negative effect are both minimal (2% in micro-benchmark). Net loss in the general cases and performance test workloads.

---

ReflectUtil.needsPackageAccessCheck()

Not tested yet. Expecting similar result to above.

---

Merging two stack walks

In the case of >=JDK12 and a security manager is set, the call to newInstacne() performs 2 stack walks that can be merged into one. Prototype Code: https://github.com/eclipse-openj9/openj9/compare/v0.35.0-release...rmnattas:openj9:util_opt_prototype

Improvement in micro-benchmark for newInstance() is around 40% for that specific case, but it's around 5% performance loss in other cases. Net loss of 4% in the performance test workloads.

---

Skipping stack walk in newInstanceImpl()

This is the targeted optimization. In the newInstanceImpl() a stack walk is performed to checkVisability. For java.util classes we can assume that it's public and we can skip the stack walk. Prototype Code: https://github.com/eclipse-openj9/openj9/compare/v0.35.0-release...rmnattas:openj9:newinstance_opt_prot

In micro-benchmark calling newInstance(), an improvement of close to 3x for classes under java.util, and around 5% loss for other classes. Although in general it seems to be a net loss. Net loss of 6% in the performance test workloads.