excelsior-oss / excelsior-jet-gradle-plugin

Excelsior JET Gradle Plugin provides Gradle users with an easy way to compile their applications down to optimized native Windows, OS X, or Linux executables with Excelsior JET http://www.excelsiorjet.com
GNU General Public License v3.0
26 stars 6 forks source link

Plugin does contain memory leak #39

Open Crazyjavahacking opened 6 years ago

Crazyjavahacking commented 6 years ago

In our huge multi-module project, first sync in IntelliJ runs correctly.

Second sync however runs much slower and potentially gets OOM Exception.

This is caused by Excelsior JET plugin as without the plugin everything works correctly.

Please analyze the heap dump. https://ufile.io/b563f

It looks like the ExcelsiorJetExtension_Decorated object contains 370MB !

Crazyjavahacking commented 6 years ago

This is uploaded to Google Drive:

https://drive.google.com/file/d/1FPMMCXFbucyTzHaK578AFBAwM-e2pJyY/view?usp=sharing

Please let me know about the status. This is blocking us from using this plugin. Thanks.

pjBooms commented 6 years ago

I have successfully downloaded .jprof file, thanx.

It looks like the ExcelsiorJetExtension_Decorated object contains 370MB

How did you get that? What I see from Eclipse MAT that total size of all com.excelsiorjet.* objects is just 5K.

On the other hand, Eclipse MAT has shown two leak suspects:

As far as I understand, io.spring.gradle is from the following plugin -- https://github.com/spring-gradle-plugins/dependency-management-plugin .

I doubt that, Excelsior JET Gradle Plugin could leak that objects: first it knows nothing about that plugin, second the only place where it works with dependencies is AbstractJetTask.getDependencies(). It queries Gradle for dependencies but does not store any internal Gradle objects inside its own structures.

Moreover, it queries for dependencies only when you directly call the plugin, it should not do anything if you just sync your project (else I could suspect that the plugin just triggers the leak in the Spring Dependency Management Plugin).

So I do not think that Excelsior JET plugin is the root cause of the leak or please provide more evident proof of it.

Crazyjavahacking commented 6 years ago

JProfiler showed the biggest objects and it is declaring ExcelsiorJetExtension_Decorated object contains 370MB.

It is weird if Eclipse MAT claims something different.

Let me further investigate.

pjBooms commented 6 years ago

Ok, I have downloaded JPofiler.

It shows that ExcelsiorJetExtension_Decorated object (that is created by Gradle) contains a transitive reference to org.gradle.api.internal.project.DefaultProject_Decorated object that in turn references all other Gradle objects that have all that 370MB in total including instances of Spring dependency management plugin (that of course also have back references to org.gradle.api.internal.project.DefaultProject). That means that we have a huge cyclic graph of Gradle objects and Excelsior JET plugin is just a small portion of that graph (but the analysys could show any object inside that cyc;le as containing 370 MB) and removing JET plugin from the cycle does not break it.
I think that Eclipse MAT suspects Spring dependency management plugin as a source of memory leak because it has 560 instances (while JET plugin has only 2 instances).

I think you'd better to reopen the issue you created in Gradle project attaching the same .hprof file. I bet they have more experience to analize such issues and they know how Gradle works inside to understand what is going on.

Crazyjavahacking commented 6 years ago

Does the plugin really need convention mapping? I understand the concept, but it seems like it is primarily creating the issues. At least for some properties it does not make much sense.

Without convention mapping and direct value association we no longer experience memory leaks.

pjBooms commented 6 years ago

At the time of writing the plugin, convention mapping was a common way for Gradle plugins to set conventions and default values. Now it seems to be not so. At least Gradle official doc encourages plugin developers to migrate from "convention mapping". So probably we should follow the recommendations.

Without convention mapping and direct value association we no longer experience memory leaks.

What do you mean by "direct value association"? Do you have a branch where you do this?

Crazyjavahacking commented 6 years ago

Do you mean Property functionality?

This version is the minimal version of Gradle Excelsior JET plugin supports? The mentioned functionality was added to Gradle 4.

Crazyjavahacking commented 6 years ago

I was tried to rewrite the plugin to use PropertyState type and Gradle 4 and the problems still remain. I will try to report the issue to dependency-management plugin, but for now the plugin is unusable for us.

Crazyjavahacking commented 6 years ago

@wilkinsona

wilkinsona commented 6 years ago

Hi, I'm a maintainer of the dependency management plugin mentioned above.

Suspecting objects as being leaked purely based on instance count (which is what MAT seems to be doing) doesn't make sense. The dependency management plugin will create one instance of DependencyManagementConfigurationContainer and one instance of DependencyManagement per Gradle project. So, if the huge multi-project build contains 560 projects, those numbers are exactly as I'd expect.

Without more information about the multi-project build, I don't think it's possible to diagnose this problem. The leak could be in this plugin, the dependency management plugin, or in Gradle itself. Given that the problem only occurs upon a second invocation, that would suggest to me that Gradle's daemon is at least contributing to the problem if not actually causing it.

@Crazyjavahacking You opened https://github.com/gradle/gradle/issues/3946 but then closed it as the problem didn't happen with just standard Gradle plugins in your project. IMO, that shouldn't have meant that it was closed as one of the non-standard plugins could be doing something that's perfectly fine but exposes a leak in Gradle's daemon.

I'm downloading the heap dump to take a look, but the site where it's hosted is very slow so it'll be a while before I can do anything with it. In the meantime, it would be useful to have as much detail as possible about the build that's triggering the problem.

Crazyjavahacking commented 6 years ago

I was even trying to refactor the ExcelsiorJetPlugin.addJetBuildConventions(Project, Task) to Gradle's 4 PopertyState:

private void addJetBuildConventions(Project project, Task archiveTask) {
    extension.version = project.provider {
        if (project.version.toString() == "unspecified") {
            project.logger.warn(Txt.s("ExcelsiorJetGradlePlugin.ProjectVersionIsNotSet"))
            return '1.0'
        } else {
            return project.version.toString()
        }
    }
    extension.artifactName = project.provider { getArchiveName(archiveTask) }
    if (!isWar) {
        extension.mainJar = project.provider { archiveTask.archivePath }
    } else  {
        extension.mainWar = project.provider { archiveTask.archivePath }
    }
    extension.jetHome = project.provider { System.getProperty("jet.home") }

    extension.jetResourcesDir = project.file("src/main/jetresources")

    extension.groupId = project.provider {
        if (project.group.toString().isEmpty()) {
            return extension.vendor
        } else {
            return project.group.toString()
        }
    }

    extension.appType = isWar ? ApplicationType.TOMCAT.toString()
                              : ApplicationType.PLAIN.toString()
}

but the problems still remain.

This is weird my only explanation is that the closures somehow magically store references that are never freed.

Crazyjavahacking commented 6 years ago

I would be also afraid of the code manipulating with Expando metaclass in ExcelsiorJetExtension:

ExcelsiorInstallerConfig excelsiorInstaller = {
    def config = new ExcelsiorInstallerConfig()
    //init embedded configurations
    config.metaClass.afterInstallRunnable = {Closure closure ->
        applyClosure(closure, config.afterInstallRunnable)
    }
    config.metaClass.installationDirectory = {Closure closure ->
        applyClosure(closure, config.installationDirectory)
    }
    config.metaClass.uninstallCallback = {Closure closure ->
        applyClosure(closure, config.uninstallCallback)
    }

    config.shortcuts = new ArrayList()
    config.metaClass.shortcuts = {Closure closure ->
        applyClosure(closure, config)
    }
    config.metaClass.shortcut = {Closure closure ->
        def shortcut = new Shortcut()
        shortcut.metaClass.icon = {Closure iconClosure ->
            applyClosure(iconClosure, shortcut.icon)
        }
        applyClosure(closure, shortcut)
        config.shortcuts.add(shortcut)
    }
}
wilkinsona commented 6 years ago

I've looked at the heapdump in YourKit. I haven't analysed it exhaustively, but the paths to GC roots for all of the dependency management plugin objects that I looked at all seemed to include instances of objects from this plugin. I'm not sure how significant that is, though, as @Crazyjavahacking has yet to describe the project structure in detail.

One thing that I did observe is that there are 562 Gradle Project instance in the heap. That matches pretty well with the 560 instances of some dependency management plugin types (assuming that a couple of projects do not have the plugin applied).

pjBooms commented 6 years ago

ExcelsiorJetPlugin.addJetBuildConventions(Project, Task) indeed stores reference to project (and archieveTask) inside the closures (as they capture it). However I cant't understand how it can lead to the memory leak. As far as I understand, Gradle should make previous instances of plugins to be unavailable on project sync.

Crazyjavahacking commented 6 years ago

It's as easy as storing one strong reference to this plugin and because of references to Project and so spring objects most of the heap cannot be garbage collected.

I would assume Gradle daemon will use the same instance of the ClassLoader, not new one for each run.

So from my perspective the question is why and which code exactly stores reference to this plugin.

pjBooms commented 6 years ago

So from my perspective the question is why and which code exactly stores reference to this plugin

So maybe we should ask Gradle developers for this as they have deeper knowledge here?

Crazyjavahacking commented 6 years ago

Unless anyone has a better idea, then yes. @wilkinsona ?

Crazyjavahacking commented 6 years ago

@wilkinsona : Our project is indeed huge multi-module build, so yes we have around 500-600 modules.

Crazyjavahacking commented 6 years ago

Reopened the Gradle issue.

wilkinsona commented 6 years ago

When was the heap dump taken? If the build contains 500 - 600 projects, then I can't see any sign of a leak.

Crazyjavahacking commented 6 years ago

What do you mean by when?

After the first IntelliJ's refresh the Gradle daemon is started and import is done in ~ 1 min. JVM process takes about 600MB.

On the second run, objects are not freed which was investigated in JVisualVM and the heap starts to increase till ~ 800MB where GC takes majority of CPU time. The whole import takes couple of minutes till it reach OOM.

The dump was taken during the 2nd sync.

oehme commented 6 years ago

I don't see any leak at first glance either. There only seem to be the project instances from the current build. Does the problem go away if you give Gradle more memory or does it just crash later?