VisitableURLClassLoader should be registered as parallelCapable

[jochenberger]

Since VisitableURLClassLoader does not override any methods related to class loading and does not hold any state, it should be registered as parallel capable.
I see a lot of locking in VisitableURLClassLoader.loadClass in one of my tasks (that compiles a set of Groovy classes).
See https://blogs.oracle.com/dholmes/entry/parallel_classloading_revisited_fully_concurrent, https://docs.oracle.com/javase/7/docs/api/java/lang/ClassLoader.html.

[eriwen]

Hi @jochenberger, thanks for the request.

Can you please expand on why this change is important and concretely explain how this affects your build (e.g. from a user perspective)?

[jochenberger]

It makes parallel loading/compilation of classes possible. See ClassLoader.loadClass(String, boolean) and ClassLoader.getClassLoadingLock(String). If the ClassLoader isn't parallelCapable, it synchronizes over the this instead of an object that's tied to the class name.
The change will prevent some unnecessary locking and will make it possible to load classes using multiple threads.
From the ClassLoader Javadocs:

Class loaders that support concurrent loading of classes are known as parallel capable class loaders and are required to register themselves at their class initialization time by invoking the ClassLoader.registerAsParallelCapable method. Note that the ClassLoader class is registered as parallel capable by default. However, its subclasses still need to register themselves if they are parallel capable.

See URLClassLoader.java for an example.

[eriwen]

@jochenberger Ok. I'm really looking for: "In what way and how much would a typical user benefit if we did this change and nothing more?" We want to know if this would affect everybody, or just those running --parallel and whether the gain is significant enough that we should do this instead of other performance optimizations.

[jsotuyod]

The change would only impact under multithreading (--parallel). The synchronization is still performed, just under different objects, and without threads, the JVM could remove the synchronization on runtime in either case.

I haven't profiled gradle for this scenario, but have seen it in other tools such as PMD (which performed this very change not long ago, see pmd/pmd#101).

Still, the important thing here, is that this is low hanging fruit. VisitableURLClassLoader is already thread-safe, so the change is literally just adding :

static {
  ClassLoader.registerAsParallelCapable();
}

No tests need to be changed / added, since this is part of the JRE, and current tests would detect misbehavior / performance regressions.

I volunteer to submit this as a PR if you think this is taking focus away from the core team.

[jochenberger]

I would have created a PR, but I don't want to be forced to sign a CLA. But I'd appreciate if you did it @jsotuyod.

[lhotari]

+1
I've had this on my performance wish list for some time. I'd appreciate if @jsotuyod sends a PR for this.
There is also this Oracle JDK doc about the feature: http://docs.oracle.com/javase/7/docs/technotes/guides/lang/cl-mt.html

[lhotari]

However there is a risk in this change. Explained in Recommendations for Multithreaded Custom Class Loaders

Custom class loaders that do not have a history of deadlocks do not require any changes. In particular, you do not need to change custom class loaders that follow the recommended acyclic hierarchical delegation model, that is, delegating first to their parent class. For backward compatibility, the Java SE 7 release continues to lock a class loader object unless it registers as parallel capable.

[jochenberger]

Fixed via #772