gwt-eclipse-plugin icon indicating copy to clipboard operation
gwt-eclipse-plugin copied to clipboard

Retrieving the GWT SDK version is incredibly slow in large projects

Open CrushaKRool opened this issue 7 years ago • 17 comments

Background: We have a very large code base that consists of more than 150 sub projects, with roughly 70% of them being either directly or transitively referenced by a distribution project that ties everything together. The whole code base is a Gradle project, and we use the Buildship plugin to import everything into Eclipse. We also have some Gradle build scripts to set the correct GWT settings (enabling the GWT nature on the distribution project, assigning the SDK and configuring other project-specific options) before the import starts.

Problems: We noticed that having the GWT nature enabled on the project has a severe negative impact on the performance of Eclipse in general:

  • Without the GWT nature, the import takes roughly 15 minutes (on an SSD). With it, that time almost doubles.
  • Validating the GWT project with the GWTProjectValidator implementation has a 4-5 minute wind-up time before it even starts doing work. Since this validation is performed during the import and also before any launch (either the application itself, a Gradle task or a unit test), we end up with a lot of idle waiting time.
  • The UI menu for launch configurations freezes completely and refuses to subsequently open after switching to the overview for the GWT launch configuration.
  • Eclipse is often waiting a long time for background work to complete, which delays many user actions like saving of a file and rebuilding the workspace.

Cause: Since this is pretty much a show stopper that makes Eclipse nearly unusable for us, we started investigating the cause and found it in ProjectBoundSdk.createClassLoader() (in GwtSdk.java), which is called when the getVersion() method is used to retrieve a string for the SDK's version number. This apparently searches the entire classpath of the project and all its referenced subprojects for a specific class in order to call a method on it via reflection.

To be even more precise, the call to JavaRuntime.computeDefaultRuntimeClassPath(javaProject) in that method is what's taking several minutes to return. This call causes Buildship to resolve its classpath container, which apparently involves many calls to File.getCanonicalPath() (aka slow native file system operations). I don't know if this is something that's specific to Gradle Buildship or if it is done the same way in other project types.

getVersion() is called in many places:

  • The GWTProjectValidator.
  • The LaunchConfigurationUpdater calls this several times, which in turn gets activated whenever the LaunchConfigAffectingChangesListener detects changes to the classpath. (This also runs during the initial project import.)
  • equals() and hashCode() implementation of AbstractSdk. All of these are likely responsible for the problems mentioned above.

createClassLoader() apparently also has been identified as performance critical in the past, as the TODO comments there indicate:

/**
 * Returns a {@link ClassLoader} that is backed by the project's runtime classpath.
 *
 * TODO: This returns a classloader which contains ALL of the jars of the project. Lookups on
 * this thing are going to be SLOW. Can we optimize this? We could create a classloader that
 * just contains the jars that GWT requires. Maybe caching is the right solution here.
 *
 * TODO: Why can't we just delegate to {@link #getClasspathEntries()} when generating the
 * classloader URLs? Why do we have to add every URL that is part of the project? That would
 * certainly speed up lookups on this classloader. Maybe we cannot do this because project-bound
 * sdks handle the case of source-based runtimes, and in that case, we need all of the
 * dependencies as part of the classloader.
 */

Workaround: For our own needs, we added a workaround where we simply read the SDK version string from the plugin's project preferences, which we write there ourselves (via the Gradle build script). If it's missing, we fall back to whole classpath lookup thing, but that case shouldn't ever happen again for us if the project is set up correctly. We could confirm that this change solves the perceived problems.

Whether such a hack is suited for the "official" release is up to you to decide. Perhaps you can think of a more elegant solution as well. I am aware that this is probably not as easy to reproduce, since it requires quite a large project.

CrushaKRool avatar Aug 05 '18 03:08 CrushaKRool

Nice job drilling the issue. Would you like to add a patch for it?

branflake2267 avatar Aug 06 '18 18:08 branflake2267

I've seen the issue show up on projects and been wanting to look into it. I've got time scheduled on the roadmap but haven't made it to where I can spend the time yet. Thanks for reporting.

branflake2267 avatar Aug 06 '18 18:08 branflake2267

Okay, I made a fork and cleaned up the workaround for a pull request. (First time actually using GitHub or Git for that matter, so I hope everything went okay)

It is of course just a workaround, since it (in order to not get slowed down during import) requires that users have the project property set before even starting the import. Which may be a bit much to ask if they don't use Gradle to generate those setting files automatically. Nonetheless there is also a text box to enter the string in the project properties page, so it can be changed via UI if people don't want to mess with the bare files.

If you want to come up with a cleaner solution, feel free.

CrushaKRool avatar Aug 08 '18 13:08 CrushaKRool

Nice job. I'll take a look at it today.

branflake2267 avatar Aug 08 '18 15:08 branflake2267

Thanks for submitting a patch too!

branflake2267 avatar Aug 08 '18 15:08 branflake2267

A question on the side: are you still able to deploy the project with its current configuration nowadays?

I tried to use the "Maven Clean Install" command from Eclipse to package up a local distribution of the plugin, but it could not resolve some of the external dependencies.

First the deployment target for Eclipse Mars failed on the Google Cloud Tools:

[INFO] Adding repository https://dl.google.com/eclipse/google-cloud-eclipse/stable [ERROR] Failed to resolve target definition E:\GitWorkingCopies\gwt-eclipse-plugin\eclipse\mars\gwt-eclipse-mars.target: Could not find "com.google.cloud.tools.eclipse.suite.feature.feature.group/1.6.0.201803071812" in the repositories of the current location

I worked around that by leaving only the deployment target for Oxygen in the POM. Next it could not resolve a dependency that was used by the com.gwtplugins.gwt.eclipse.wtp.test and com.gwtplugins.gwt.eclipse.wtp.maven.test modules:

[INFO] Resolving dependencies of MavenProject: com.gwtplugins.eclipse:com.gwtplugins.gwt.eclipse.wtp.test:3.0.0-SNAPSHOT @ E:\GitWorkingCopies\gwt-eclipse-plugin\plugins\com.gwtplugins.gwt.eclipse.wtp.test\pom.xml [INFO] {osgi.os=linux, osgi.ws=gtk, org.eclipse.update.install.features=true, osgi.arch=x86} [ERROR] Cannot resolve project dependencies: [ERROR] You requested to install 'org.eclipse.jst.web_sdk.feature.feature.group 0.0.0' but it could not be found

I ended up working around that by just removing both modules from the POM, since they were only test cases (and at least one of them is entirely commented out anyway).

Only after that did I manage to build a distribution. So I was wondering if others are experiencing the same issue or whether it may be related to me using Eclipse Photon (which also required fixing compile errors in 4 classes, probably due to changes in the API).

CrushaKRool avatar Aug 09 '18 09:08 CrushaKRool

Sorry I need to document that.

The goal, install the TBD plugin, and set the target, this will regenerate the targets, which fixes the problems.

  1. install TBD plugin, listed in the link below.
  2. Set the targets manually in Eclipse, this will re-generate the targets.
  3. Sometimes the tbd target has to tuned, you'll know if it errors. I've had to change the path to the GCP repo...

https://github.com/gwt-plugins/gwt-eclipse-plugin/blob/master/eclipse/oxygen/gwt-eclipse-oxygen.tpd

Does that help?

branflake2267 avatar Aug 09 '18 15:08 branflake2267

Once you install the TBD plugin, then you right click on the TBD file and set as target. This will generate the Eclilpse targets file. Which is nice to do it this way. It cuts down on making errors with targets.

branflake2267 avatar Aug 09 '18 15:08 branflake2267

Thanks, that did the trick. The plugin did actually error with

An internal error occurred during: "Setting target platform definition file". org.eclipse.pde.internal.core.PDECore.acquireService(Ljava/lang/String;)Ljava/lang/Object;

but it did update the targets nonetheless.

CrushaKRool avatar Aug 10 '18 10:08 CrushaKRool

Nice job. I've been delayed by another fire at work. I've got it on my calendar to pull it run it, merge it, and then push to staging. Worst case it will fall on Saturday.

branflake2267 avatar Aug 10 '18 14:08 branflake2267

I'm pushing a fix to staging repo. It should be up shortly.

branflake2267 avatar Aug 13 '18 06:08 branflake2267

Fix is on staging.

branflake2267 avatar Aug 13 '18 06:08 branflake2267

Your solution is indeed better for the average user, since it does not involve any manual work (and is also not just a dirty hack like mine).

It is not as optimal for us, however, since the properties key that stores cached version depends strongly on the installation directory and SDK location of the end user. (It looks like this in my case: gwtVersion_E:\\Dev\\eclipse_photon\\plugins\\com.gwtplugins.gwt.eclipse.sdkbundle.gwt27_2.7.0.201808101238\\gwt-2.7.0) This makes it rather difficult to set this property in advance via a Gradle plugin, since that plugin won't know about the installation path (and has no reason to, especially if it runs in an environment without Eclipse). And without setting it in advance, we are still giving away 4-5 minutes that could be saved otherwise. I am not asking you to do anything about that. If it really bothers us too much, we'll just have to keep using our forked fix. Just wanted to point it out.

(Also, not sure if the GWT version is ever retrieved by more than one thread, in which case it could start multiple classpath resolves before finally having the version in its cache.)

CrushaKRool avatar Aug 13 '18 08:08 CrushaKRool

Gotcha, the configurator sets the sdk, so in Maven's instance, it overrides the default paths and I don't this repo doesn't have the gradle configurator yet. So with that in mind, I could provide another option, so it can be overridden so you don't have to use a fork. I'll look into the options to see how Gradle can set the sdk config.

branflake2267 avatar Aug 13 '18 17:08 branflake2267

If you're offering, I won't be declining. :)

We still might keep the fork around to deal with some minor annoyances that may pop up here and there (and which are likely caused by us abusing Gradle to deal with some quirks in the old GDT plugin that might no longer hold true), but it's nice of course if it doesn't diverge much from the origin.

CrushaKRool avatar Aug 13 '18 18:08 CrushaKRool

Sounds good. I have two long weekends in a row, so it might take me 3 weeks to get back to this. Although I have some dedicated time road mapped and coming to work on this plugin coming. I'm getting closer to that time.

branflake2267 avatar Aug 14 '18 15:08 branflake2267

Also, one more minor caveat if I was just using the current solution where it resolves the classpath once: Thus far my test environment used the premise that the GWT nature is only active on the distribution project. However, I just realized that the GWT Project Validator only finds issues in projects that have the GWT nature enabled and not in subprojects that are on the project's classpath (correct me if I am wrong here).

So in order to get proper error checking on our GWT code, I needed to enable the nature on 20 more projects, causing each of them to resolve their own classpath to determine the GWT version. A small consolation would be that the total increase in build time would be at most the amount of time it takes to resolve the classpath of the distribution project (since it includes all those subprojects).

Projects with different dependency hierarchies, where A depends on B and B depends on C, would probably have bigger increases in build time due to the redundant classpaths.

CrushaKRool avatar Aug 17 '18 15:08 CrushaKRool

@CrushaKRool, this issue should have been fixed by https://github.com/gwt-plugins/gwt-eclipse-plugin/pull/386 Is the issue still relevant?

protoism avatar Jan 31 '23 16:01 protoism

@protoism, well, as I pointed out in https://github.com/gwt-plugins/gwt-eclipse-plugin/issues/383#issuecomment-412445005, this is of course an improvement for the average user. So feel free to close it.

For our specific use-case, we have added a custom workaround in our fork that saves us even more time during the initial import, but it requires to configure the project with external tooling before importing into Eclipse, so it's not something I'd see as useful to regular users. We are content with maintaining that in our fork.

CrushaKRool avatar Feb 02 '23 17:02 CrushaKRool