Skip to content

Speed up ZincRunner #76

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

Merged
merged 4 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 2 deletions rules/private/phases/phase_zinc_compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ load(
def phase_zinc_compile(ctx, g):
toolchain = ctx.toolchains["//rules/scala:toolchain_type"]
analysis_store = ctx.actions.declare_file("{}/analysis_store.gz".format(ctx.label.name))
analysis_store_text = ctx.actions.declare_file("{}/analysis_store.text.gz".format(ctx.label.name))
mains_file = ctx.actions.declare_file("{}.jar.mains.txt".format(ctx.label.name))
used = ctx.actions.declare_file("{}/deps_used.txt".format(ctx.label.name))
tmp = ctx.actions.declare_directory("{}/tmp".format(ctx.label.name))
Expand Down Expand Up @@ -77,7 +76,6 @@ def phase_zinc_compile(ctx, g):
g.classpaths.jar,
mains_file,
analysis_store,
analysis_store_text,
used,
tmp,
] + g.semanticdb.outputs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class CommonArguments private (
val analyses: List[Analysis],
val compilerBridge: Path,
val compilerClasspath: List[Path],
val compilerOptions: List[String],
val compilerOptions: Array[String],

/**
* With [[https://bazel.build/remote/multiplex#multiplex_sandboxing multiplex sandboxing]], Bazel generates a separate
Expand All @@ -32,7 +32,7 @@ class CommonArguments private (
val compilerOptionsReferencingPaths: List[String],
val classpath: List[Path],
val debug: Boolean,
val javaCompilerOptions: List[String],
val javaCompilerOptions: Array[String],
val label: String,
val logLevel: LogLevel,
val mainManifest: Path,
Expand Down Expand Up @@ -215,8 +215,8 @@ object CommonArguments {
compilerBridge = SandboxUtil.getSandboxPath(workDir, namespace.get[Path]("compiler_bridge")),
compilerClasspath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("compiler_classpath")),
compilerOptions = Option(namespace.getList[String]("compiler_option"))
.map(_.asScala.toList)
.getOrElse(List.empty),
.map(_.asScala.toArray)
.getOrElse(Array.empty),
compilerOptionsReferencingPaths = adjustCompilerOptions(
workDir,
Option(namespace.getList[String]("compiler_option_referencing_path"))
Expand All @@ -225,7 +225,7 @@ object CommonArguments {
),
classpath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("classpath")),
debug = namespace.getBoolean("debug"),
javaCompilerOptions = namespace.getList[String]("java_compiler_option").asScala.toList,
javaCompilerOptions = namespace.getList[String]("java_compiler_option").asScala.toArray,
label = namespace.getString("label"),
logLevel = LogLevel(namespace.getString("log_level")),
mainManifest = SandboxUtil.getSandboxPath(workDir, namespace.get[Path]("main_manifest")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,16 +209,14 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] {

val compileOptions =
CompileOptions.create
.withSources(sources.map(source => PlainVirtualFile(source.toAbsolutePath().normalize())).toArray)
.withClasspath((classesOutputDir +: deps.map(_.classpath)).map(path => PlainVirtualFile(path)).toArray)
.withSources(sources.view.map(source => PlainVirtualFile(source.toAbsolutePath().normalize())).toArray)
.withClasspath((classesOutputDir +: deps.view.map(_.classpath)).map(path => PlainVirtualFile(path)).toArray)
.withClassesDirectory(classesOutputDir)
.withJavacOptions(workRequest.javaCompilerOptions.toArray)
.withJavacOptions(workRequest.javaCompilerOptions)
.withScalacOptions(
(
workRequest.plugins.map(p => s"-Xplugin:$p") ++
workRequest.compilerOptions ++
workRequest.compilerOptionsReferencingPaths
).toArray,
workRequest.plugins.view.map(p => s"-Xplugin:$p").toArray ++
workRequest.compilerOptions ++
workRequest.compilerOptionsReferencingPaths.toArray,
)

val compilers = {
Expand Down Expand Up @@ -304,11 +302,6 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] {

// create analyses
val pathString = analysisStorePath.toAbsolutePath().normalize().toString()
val analysisStoreText = AnalysisUtil.getAnalysisStore(
new File(pathString.substring(0, pathString.length() - 3) + ".text.gz"),
true,
readWriteMappers,
)
// Filter out libraryClassNames from the analysis because it is non-deterministic.
// Can stop doing this once the bug in Zinc is fixed. Check the comment on FilteredRelations
// for more info.
Expand All @@ -319,7 +312,18 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] {
infos = FilteredInfos.getFilteredInfos(originalResultAnalysis.infos),
)
}
analysisStoreText.set(AnalysisContents.create(resultAnalysis, compileResult.setup))

// This will be true if the `--worker_verbose` Bazel flag is set
if (verbosity >= 10) {
Copy link

Choose a reason for hiding this comment

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

I recommend adding a comment here that says verbosity = 10 when --worker_verbose is set. Otherwise this may look like an arbitrary constant to folks who don't know that.

Copy link
Author

Choose a reason for hiding this comment

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

Good idea.

val analysisStoreText = AnalysisUtil.getAnalysisStore(
new File(pathString.substring(0, pathString.length() - 3) + ".text.gz"),
true,
readWriteMappers,
)

analysisStoreText.set(AnalysisContents.create(resultAnalysis, compileResult.setup))
}

analysisStore.set(AnalysisContents.create(resultAnalysis, compileResult.setup))

// create used deps
Expand All @@ -329,7 +333,7 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] {
deps.filter(Dep.used(deps, resultAnalysis.relations, lookup)).filterNot { dep =>
val filteredDepFileName = FileUtil.getNameWithoutRulesJvmExternalStampPrefix(dep.file)

scalaInstance.libraryJars
scalaInstance.libraryJars.view
.map(FileUtil.getNameWithoutRulesJvmExternalStampPrefix)
.contains(filteredDepFileName)
}
Expand Down
1 change: 1 addition & 0 deletions tests/.bazelversion
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
7.4.1