Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache classloaders to allow warmup #61

Closed
nedtwigg opened this issue Jan 2, 2017 · 7 comments
Closed

Cache classloaders to allow warmup #61

nedtwigg opened this issue Jan 2, 2017 · 7 comments

Comments

@nedtwigg
Copy link
Member

nedtwigg commented Jan 2, 2017

Based on the profiling data available here, it looks like we can expect to see a ~7x speedup if we allowed JarState classloaders to be cached between gradle runs, so that the JIT could let them warm up.

The code in question would be JarState::openIsolatedClassLoader(). Currently this method is called at the beginning of a call to format, and then the classloader is closed at the end of the call. Here's a usage example.

@jbduncan
Copy link
Member

jbduncan commented Jan 2, 2017

@ben-manes I can't speak for @nedtwigg, but I admit I didn't understand what you meant in #55 (comment) when you said we could use the runtimeClasspath source set to allow Gradle to cache the classloaders. (I only vaguely understand what a source set is, for example how it's a collection of Java files like src/main/java.)

Would you kindly give me an example of how we could use the source set to do such caching? If not, would you kindly point me towards some documentation that I could read to learn more about this?

@nedtwigg
Copy link
Member Author

nedtwigg commented Jan 2, 2017

I think ben probably doesn't realize that we're loading our dependencies at runtime. But there might be a way to accomplish what he's talking about. In the initial issue, I mentioned how we could do the caching within spotless-lib. But we could also do it at the plugin-level. That is, by using Gradle's implementation of Provisioner, GradleProvisioner to cache them using gradle-specific mechanisms. I'm not aware of a way to create a persistent sourceSet during task configuration (we don't know what our deps are going to be until we configure the spotless task).

@ben-manes
Copy link

Yeah, I was shooting from the hip. Its more of what I'd be checking first, since I'd assume Gradle would have the same performance problem in many of its tasks.

An example ad hoc usage is the JavaExec task. In this build file I use sourceSets.javaPoet.runtimeClasspath for executing a code generator.

The question is, does this problem affect Gradle and has it optimized for that already? And if so, can you take advantage of that to keep it cached within the daemon. Ideally you wouldn't need your own caching logic (which may be ephemeral to that build execution) and delegate the responsibility to Gradle.

@oehme
Copy link

oehme commented Jan 4, 2017

An example ad hoc usage is the JavaExec task. In this build file I use sourceSets.javaPoet.runtimeClasspath for executing a code generator.

@ben-manes runtimeClasspath is just a FileCollection, not a ClassLoader. The classes will be loaded from scratch each time this task is executed.

@nedtwigg There is no ClassLoader cache that Gradle exposes. It's quite easy to do your own though:

  • if you are fine with loading again on each build (i.e. your classes are not too expensive to load), just create the class loader once for each task execution
  • if you want to reuse it across builds, make it a static field and invalidate it based on a comparison between the ClassLoader's URLs and the current list of dependencies. If they no longer match, close the old classloader and create a new one

The second option does add a bit of weight to the daemon, but it is often worth it if the framework you are calling has lots of classes. I did this for the Xtext plugin for instance, since loading all Xtext classes takes around 2.5s on my machine, which is definitely unacceptable on every build.

@ben-manes
Copy link

ben-manes commented Jan 4, 2017

@oehme You keep saying that when it was clear, repeatedly, that we were in agreement of the general approach. I merely recommended to use Gradle's facilities and do diff'ing in response to a suggestion of using my caching library. I'm unsubscribing from these threads.

@nedtwigg
Copy link
Member Author

nedtwigg commented Jan 5, 2017

Huge perf improvement in testing from this change to TestProvisioner: f97933c

Not helpful for caching classloaders, but gives some idea of how expensive gradle's dep resolution is.

@nedtwigg
Copy link
Member Author

nedtwigg commented Jan 6, 2017

Did some perf testing on my junit branch.

before: 21s
4cd509c
after: 8.4, 2.5, 1.6s

So about a 10x speedup. Also, since the cache was managing our classloaders, we could rip out a decent hunk of code: 15793ed

For now, the cache gets dumped whenever a gradle clean happens. Seems like a reasonable strategy.

@nedtwigg nedtwigg closed this as completed Jan 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants