Add functionality to build a global flamegraph of implicit searches #50
Conversation
| object ScalaSettingsOps { | ||
| def isScala212: Boolean = true | ||
| def isScala213: Boolean = false |
There was a problem hiding this comment.
That is a bit controversial, perhaps there is a more elegant way to do these checks.
There was a problem hiding this comment.
Just for the context, scala/scala#9575.
|
@lolgab since you made the initial feature request, could we interest you in taking this for a quick spin before merge...? |
| val names = | ||
| stackedNames.getOrElse(id, sys.error(s"Stack name for search id ${id} doesn't exist!")) | ||
| val stackName = names.mkString(";") | ||
| //val count = implicitSearchesByType.getOrElse(tpe, sys.error(s"No counter for ${tpe}")) |
There was a problem hiding this comment.
Is this commented out line needed?
There was a problem hiding this comment.
Preserved it as a historical artefact 🤷🏻
| def foldImplicitStacks(outputPaths: Seq[AbsolutePath]): Unit = | ||
| if (outputPaths.nonEmpty) { | ||
| // This part is memory intensive and hence the use of java collections | ||
| val stacksJavaList = new java.util.ArrayList[String]() |
There was a problem hiding this comment.
Did you consider using a scala.collection.mutable.ArrayBuffer?
There was a problem hiding this comment.
I left this as is (as Jorge implemented it initially). But I also felt that we can change this collection over time.
|
|
||
| val globalDir = | ||
| ProfileDbPath.toGraphsProfilePath( | ||
| config.sourceRoot.resolve(RelativePath(s"target/$scalaDir/classes")) |
There was a problem hiding this comment.
The target directory is customizable in sbt. This doesn't work if you customize it. Maybe the plugin should receive the ScalaDir directory as parameter which opens the door to use the plugin also in other build tools like mill where the directory structure is different.
There was a problem hiding this comment.
Yeah, I thought about this one. If we agree on the approach, I will iterate on this. From my understanding, scalac-profiling was designed for SBT mostly. There are also a few other things related to SBT's output.
There was a problem hiding this comment.
In my projects at work I have:
// avoid invalidating compilation cache when enabling/disabling coverage
target := { if (ScoverageKeys.coverageEnabled.value) target.value / "coverage" else target.value }which will break here. But we don't need to fix this problem now. It can be iterated over later :)
There was a problem hiding this comment.
@lolgab but this would also change targets for particular modules, and currently generated flamegraphs will be put into the coverage space, won't it?
There was a problem hiding this comment.
Yes. but your hardcoding of target will not work.
There was a problem hiding this comment.
So perhaps we need to generalize the task and make target configurable overall, wdyt?
There was a problem hiding this comment.
but your hardcoding of target will not work.
btw, have you tried specifying the source root directory?
There was a problem hiding this comment.
You can ask for crossTarget and then look inside for the classes.
For example it can be something like: s"-P:scalac-profiling:crossTarget=${crossTarget.value}".
crossTarget points by default to target/scala-2.13 but it can be configured and matches what sbt uses.
If you change target sbt automatically changes crossTarget.
There was a problem hiding this comment.
But yeah, it doesn't need to be done in this PR. Just something to keep in mind for later.
|
This is ready for merge, or is it still under discussion...? |
|
@SethTisue as we agreed with @lolgab, their concern about using scalac-profiling in other than SBT build tools will be resolved within #55. So, this should be good to go. |
Resolves #43
This PR brings the
-P:scalac-profiling:generate-global-flamegraphplugin option that enables building the global flamegraph of implicit searches for all compilation units. In fact, these graphs are built us assembling the flamegraphs of all compilation units. The Flamegraph tool accumulates all repetitive tokens on its own in the resulting graph.