OOM while collecting transitive dependencies

[MIOB]

Hi,

I have faced an issue with coursier in my project. I tried to fetch our company's internal artifacts with the '--json-output-file` flag, but the coursier failed with OOM. Actually, the problem arose quite a long ago but giving it ~ 50Gb heap solved the issue (actually building JSON report took about 10 minutes with such settings, but it was acceptable), so I preferred to do nothing. Now the artifact tree has become ever more complex and this does not work anymore.

Unfortunately, I can't provide a reproducible example because mentioned artifacts are part of our closed-source project and are available only in an internal repo.

I have made some investigation and have found that the issue is caused by building a JSON report, more specifically it is in calculating all transitive dependencies of each particular artifact. Imagine that you have artifact A which depends on artifacts B and C. And artifact B depends on artifact C too. In order to calculate all transitive dependencies of A the current algorithm first goes to B and tries to calculate all transitive dependencies of B, then it goes to C (because B depends on C) and does the same procedure. Then it goes to C (because A depends on C) and does the same procedure for C again instead of using the already calculated value. In the usual artifacts tree, it is not a problem because usually, you do not have a lot of complex dependencies with a lot of dependents. But in our project, we have, for example, some util artifact which has about ~100 non-unique dependencies, and almost any artifact in the project depends on util. Because of that, the current algorithm visited all ~100 utils' dependencies hundreds of thousands of times...

I'm not a scala developer and not good at functional programming (really, I have no idea what Cofree is and how it is supposed to work :) ), so I was not able to fix the issue without rewriting functional code to imperative style. Thus, I just replaced the current algorithm with plain old DFS and it fixed the issue (the JSON report now is built with the default memory setting for about 4 seconds). I do not expect my code to be merged into the master (I created PR just for highlighting the problematic code) because I believe there should be a much more elegant solution in a scala-way functional style. But I would appreciate it if you find time to fix the issue.

Thanks in advance!

[mzuehlke]

The problem of the exzessive memory usage should be fixed with a recently merged PR #2533.
Have you tried a new version ?

[MIOB]

Yes, I reproduced the problem yesterday in master.

[alexarchambault]

Thanks @MIOB!

I opened #2588, to make the code more "functional", and fix formatting, as I can't push on the branch of this PR.

The CI errors look legit. The two failing tests can be run locally with

$ ./mill -i -w cli.test "coursier.cli.util.JsonReportTests.JsonReport should prevent walking a tree with cycles"
$ ./mill -i -w cli.test "coursier.cli.util.JsonReportTests.JsonReport should prevent walking a tree in which a dependency depends on itself"

It seems it has to see with cycles. (Either test fixtures that need to be updated, or cycles handling that need to be slightly tweaked.)

[nkoroste]

+1 I'm facing the same exact issue at my company. Even though #2533 significantly helped, I still see the issue on v2.1.0-RC2

[nkoroste]

Any updates on this? @alexarchambault do you think it's hard to fix those tests? I just tried your change in our repo and it not only that it fixed OOM but also improved execution time from 15 min to like 20 seconds

[mauriciogg]

First time seeing scala so I'm not sure what Im I doing, but isn't the fix just avoid traversing seen nodes?
With the following changes all tests (./mill -i -w cli.test) pass:

private def transitiveOf[A: Eq: Show](
   elem: A,
   fetchChildren: A => Seq[A]
 ): Eval[Map[A, Set[String]]] = {
   val knownElems = mutable.HashMap[String, mutable.Set[String]]()
   def collectDeps[A: Eq: Show](
     elem: A,
     fetchChildren: A => Seq[A],
     seen: mutable.Set[String]
   ): mutable.Set[String] = {
     val key = elem.show
     if (seen.contains(key)) {
       return seen
     }
     seen.add(key)
     knownElems.get(key).getOrElse {
       val deps = mutable.HashSet[String]()
       knownElems.put(key, deps)
       for (child <- fetchChildren(elem)) {
         if (!seen.contains(child.show)) {
           deps.add(child.show)
           val childDeps = collectDeps(child, fetchChildren, seen)
           for (dep <- childDeps)
             deps.add(dep)
         }
       }
       deps
     }
   }
   val result = collectDeps(elem, fetchChildren, mutable.HashSet[String]()).toSet[String]
   Eval.now(Map(elem -> result))
 }

[alexarchambault]

I took and tweaked a bit @mauriciogg's suggestion in #2588 (thanks for it, @mauriciogg!), that's just been merged. So closing this one, thanks for having opened it, @MIOB!