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

Gradle spotless plugin using large amounts of ungcable heap in daemon when using 'ratchetFrom' #735

Closed
itsyaboiyeah opened this issue Nov 14, 2020 · 6 comments · Fixed by #772

Comments

@itsyaboiyeah
Copy link

itsyaboiyeah commented Nov 14, 2020

In my project, if I use the ratchetFrom argument when using spotless then the gradle daemon requires 4GB more heap than without it. The size of the repo on git is 130MB. Heap analysis shows the 4GB to not be able to get collected.

I am using gradle 6.6.1, my plugin version is 5.6.1.

My OS is Windows, but this occurs on Linux also.

Sorry I can't link the repo, it is private. It is a large multi module Scala project using gradle as the build tool, we apply the spotless plugin to most of the modules, the spotless configuration looks something like so (some fields redacted)

spotless {
  ratchetFrom 'origin/master'
  scala {
    target fileTree('.') {
      include '**/*.scala'
      exclude '**/build/**'
     }
   trimTrailingWhitespace()
   endWithNewline()
   scalafmt("$scalafmt").configFile("$rootDir/.scalafmt.conf")
  }
}

No errors are emitted when running.

Sorry I have not been able to artificially replicate the problem, it does not seem to be related to the size of the repo.

@nedtwigg
Copy link
Member

Hm. Thanks for info. It should be the case that the resources are closed when the build finishes:

getProject().getGradle().buildFinished(new Closure(null) {
@SuppressFBWarnings("UMAC_UNCALLABLE_METHOD_OF_ANONYMOUS_CLASS")
public Object doCall() {
gitRatchet.close();
return null;
}
});

@Override
public void close() {
gitRoots.values().stream()
.distinct()
.forEach(Repository::close);
}

Perhaps that .distinct() call is a mistake...

@itsyaboiyeah
Copy link
Author

Hello, I don't think it is this. I compiled spotless for myself with your proposed change and same result.

I did some small amount of debugging, during configuration a different Repository object is instantiated for each project, and then we have 4gb of heap used. But we just have the one repo, so this seems like the issue to me?

I deleted this line (since this logic seems to be pertaining to multiple repos forming one gradle projects, not the case for me)

repo = traverseParentsUntil(getDir(project).getParentFile(), getDir(parentProj));

And everything seemed to still work, and now the heap was the normal size.

Sorry if I'm way off base, and please let me know if there's anything I can run for you to see if it works for me.

@nedtwigg
Copy link
Member

a different Repository object is instantiated for each project,

Ruh-roh!

But we just have the one repo, so this seems like the issue to me?

I think you are right, but I think the recursion in GitRatchet is a little bit more broken than just that one line.

The tricky part of this code is handling submodules, where a single gradle build may have multiple git repositories within it. I think what the code is trying to do is:

  • map each Project to a Repository
  • base case (two of them)
    • 1 ) this Project root maps to a Repository root exactly (easy-peasy)
    • 2 ) this Project has no parent project, so we will scan up the directory tree until we find one
  • else
    • recurse on the parent project

The one weak spot of the model described above is a setup like this (note the GAP)

+- settings.gradle (top-level project)
+- .git/ (top-level git repo)
+- submoduleA/
|  +- .git/ (submodule)
|  +- settings.gradle (included build)
+- submoduleB/
   +- .git/ (submodule)
   +- subfolder (GAP)
      +- settings.gradle (included build)

I think the line that you deleted is meant to handle the GAP case, but it accidentally creates too many repositories. Perhaps this condition needs to move up into the while

if (Objects.equals(startWith, file)) {
return null;
}

PR's wanted!

@nedtwigg
Copy link
Member

nedtwigg commented Jan 4, 2021

I believe fixed in plugin-gradle 5.9.0 and plugin-maven 2.7.0, can you confirm?

@nedtwigg nedtwigg reopened this Jan 4, 2021
@nedtwigg
Copy link
Member

nedtwigg commented Feb 5, 2021

Bump

@itsyaboiyeah
Copy link
Author

Sorry! I hadn't seen this, have now confirmed it fixed. Thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants