Improve performance of BSP classpath retrieval

[abernardi597]

Issue reported in #1792

[lefou]

In #1793 I already applied your suggested fix and also added a test case. It would be nice, if we can rebase this PR after #1793 is merged.

I don't know how hard it would be to add a test case that shows the time spent in the (inefficient) module traversal (mentioned in #1792) in bspCompileClasspath, but it would be great to have a test which reproduces such a setup to make sure we perform good enough. The easiest way to produce a huge amount of modules is by creating a Cross module and have lots of instances.

[lefou]

I applied this test case #1792 (comment) and here is the output.

You can see it behaves better than before, e.g. no out of memory exception and shorter time spans, but still once the modules count with inter-dependencies goes up, there is a huge increase in runtime.

-------------------------------- Running Tests --------------------------------
mill.scalalib.bsp.BspModuleTests: [13/14] HelloBsp.resolvedIvyDeps 
mill.scalalib.bsp.BspModuleTests: [21/22] HelloBsp2.resolvedIvyDeps 
+ mill.scalalib.bsp.BspModuleTests.bspCompileClasspath.single module.dependent module 1727ms  
mill.scalalib.bsp.BspModuleTests: [13/14] HelloBsp.resolvedIvyDeps 
mill.scalalib.bsp.BspModuleTests: [13/14] Mod[1].resolvedIvyDeps 
+ mill.scalalib.bsp.BspModuleTests.bspCompileClasspath.single module.interdependencies are fast.index 1 (no deps) 316ms  137 msec
mill.scalalib.bsp.BspModuleTests: [13/14] HelloBsp.resolvedIvyDeps 
mill.scalalib.bsp.BspModuleTests: [85/86] Mod[10].resolvedIvyDeps 
+ mill.scalalib.bsp.BspModuleTests.bspCompileClasspath.single module.interdependencies are fast.index 10 678ms  515 msec
mill.scalalib.bsp.BspModuleTests: [13/14] HelloBsp.resolvedIvyDeps 
mill.scalalib.bsp.BspModuleTests: [165/166] Mod[20].resolvedIvyDeps 
+ mill.scalalib.bsp.BspModuleTests.bspCompileClasspath.single module.interdependencies are fast.index 20 845ms  717 msec
mill.scalalib.bsp.BspModuleTests: [13/14] HelloBsp.resolvedIvyDeps 
mill.scalalib.bsp.BspModuleTests: [245/246] Mod[30].resolvedIvyDeps 
X mill.scalalib.bsp.BspModuleTests.bspCompileClasspath.single module.interdependencies are fast.index 30 29498ms 
  utest.AssertionError: assert(timeSpent < 5000)
  timeSpent: Long = 29384
    utest.asserts.Asserts$.assertImpl(Asserts.scala:30)
    mill.scalalib.bsp.BspModuleTests$.$anonfun$tests$22(BspModuleTests.scala:112)
    mill.scalalib.bsp.BspModuleTests$.workspaceTest(BspModuleTests.scala:51)
    mill.scalalib.bsp.BspModuleTests$.run$1(BspModuleTests.scala:54)
    mill.scalalib.bsp.BspModuleTests$.$anonfun$tests$29(BspModuleTests.scala:119)
mill.scalalib.bsp.BspModuleTests: [13/14] HelloBsp.resolvedIvyDeps 
^C

[lefou]

Thank you for your pull request. In the meantime I came up with a test cases which shows these performance issue. The current logic is heavily using T.task as it depends on some non-static details from the current evaluator. But I think I've come up with another idea, how we can avoid all these ad-hoc and un-cached tasks and replace it with more granular cached targets. I'll open a PR once I have implemented it.

Even if that means, that we won't merge your PR, your work is much appreciated! It helped greatly to understand the issue and gave food for thoughts.

[abernardi597]

No problem, thanks for taking a look at this!
I'll keep an eye out for your other idea, since I'm interested in the cached approach.

[lefou]

@abernardi597 I opened #1803. I'd love to get your feedback for it.