Skip to content

Fix CPU starvation warning in MillBuildTool classpath extraction #17

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.

Sign up for GitHub

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

Merged
rochala merged 2 commits into from
Mar 30, 2026
+6 −5
Merged

Fix CPU starvation warning in MillBuildTool classpath extraction #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fceee20
Wrap blocking file checks in IO.blocking in MillBuildTool
rochala Mar 25, 2026
3d03dd0
Wrap PublicApiFilter.isPublic in IO.blocking in stream filters
rochala Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/src/cellar/AllSymbolsStream.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ object AllSymbolsStream:
})
.flatMap(syms => Stream.emits(syms))
}
.filter(PublicApiFilter.isPublic)
.evalFilter(sym => IO.blocking(PublicApiFilter.isPublic(sym)))
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change moves public-symbol filtering onto IO.blocking, but PublicApiFilter.isPublic is a pure predicate (no file-system calls). Besides the per-element context shift overhead, this is also a discrepancy with the PR title/description which focus on Mill classpath extraction; either document why symbol filtering needed this change (and how it addresses CPU-starvation warnings), or split/revert this part to keep the PR scoped.

Copilot uses AI. Check for mistakes.
4 changes: 2 additions & 2 deletions lib/src/cellar/SymbolLister.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ object SymbolLister:
Stream
.eval(IO.blocking(pkg.declarations))
.flatMap(decls => Stream.emits(decls))
.filter(PublicApiFilter.isPublic)
.evalFilter(sym => IO.blocking(PublicApiFilter.isPublic(sym)))
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PublicApiFilter.isPublic is a pure, in-memory predicate (see lib/src/cellar/PublicApiFilter.scala) and does not perform blocking I/O. Wrapping it in IO.blocking and switching from .filter to .evalFilter introduces an effect + blocking-pool shift per symbol, which can significantly hurt throughput when listing many symbols. Prefer keeping .filter(PublicApiFilter.isPublic) (or, if an effect is truly needed, use evalFilter(sym => IO(PublicApiFilter.isPublic(sym))) without blocking).

Copilot uses AI. Check for mistakes.

case ListTarget.Cls(cls) =>
Stream
.eval(IO.blocking(SymbolResolver.collectClassMembers(cls)))
.flatMap(syms => Stream.emits(syms))
.filter(PublicApiFilter.isPublic)
.evalFilter(sym => IO.blocking(PublicApiFilter.isPublic(sym)))
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern here: PublicApiFilter.isPublic is not blocking, so IO.blocking + evalFilter adds unnecessary overhead and shifts every element to the blocking pool. Consider reverting to .filter(PublicApiFilter.isPublic) (or evalFilter(sym => IO(...)) without blocking if you need an effect for composition).

Copilot uses AI. Check for mistakes.
5 changes: 3 additions & 2 deletions lib/src/cellar/build/MillBuildTool.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class MillBuildTool(cwd: Path, binary: String = "./mill") extends BuildTool:
for
compileResult <- ProcessRunner.run(List(binary, "show", s"$mod.compile"), Some(cwd))
_ <- checkCompileResult(compileResult, mod)
classesDir = parseClassesDir(compileResult.stdout)
classesDir <- IO.blocking(parseClassesDir(compileResult.stdout))
cpResult <- ProcessRunner.run(List(binary, "show", s"$mod.compileClasspath"), Some(cwd))
paths <- parseClasspathResult(cpResult, classesDir)
yield paths
Expand All @@ -38,9 +38,10 @@ class MillBuildTool(cwd: Path, binary: String = "./mill") extends BuildTool:
if result.exitCode != 0 then
IO.raiseError(CellarError.ClasspathExtractionFailed(BuildToolKind.Mill, result.stderr))
else
ClasspathOutputParser.parseJsonArray(result.stdout) match
IO.blocking(ClasspathOutputParser.parseJsonArray(result.stdout)).flatMap {
case Left(err) => IO.raiseError(CellarError.ClasspathExtractionFailed(BuildToolKind.Mill, err))
case Right(paths) => IO.pure(classesDir.toList ++ paths)
}

private def parseClassesDir(stdout: String): Option[Path] =
val marker = "\"classes\""
Expand Down
Loading