From 4c0cc327d6c9e7b39311fa5748fa2172393f171e Mon Sep 17 00:00:00 2001 From: Jaden Peterson Date: Mon, 21 Oct 2024 14:41:36 -0400 Subject: [PATCH] Pass SemanticDB compiler options in a separate list --- rules/private/phases/phase_semanticdb.bzl | 17 +++++- .../workers/common/CommonArguments.scala | 55 +++++++++++++------ .../workers/zinc/compile/ZincRunner.scala | 9 +-- 3 files changed, 58 insertions(+), 23 deletions(-) diff --git a/rules/private/phases/phase_semanticdb.bzl b/rules/private/phases/phase_semanticdb.bzl index 3d2d7f87..d2a7a9e5 100644 --- a/rules/private/phases/phase_semanticdb.bzl +++ b/rules/private/phases/phase_semanticdb.bzl @@ -6,7 +6,16 @@ load( ) def _semanticdb_directory_from_file(file): - return file.path[:file.path.find("META-INF") - 1] + """ + This is janky, but we're limited in what we can do in this function. From the + [documentation](https://bazel.build/rules/lib/builtins/Args#add_all) on `Args#add_all`: + + To avoid unintended retention of large analysis-phase data structures into the execution phase, + the `map_each` function must be declared by a top-level `def` statement; it may not be a + nested function closure by default. + """ + + return "{}/{}".format(file.root.path, file.short_path[:file.short_path.find("META-INF") - 1]) # # PHASE: semanticdb @@ -40,18 +49,20 @@ def phase_semanticdb(ctx, g): if scala_configuration.version.startswith("2"): arguments.add("--compiler_option=-P:semanticdb:failures:error") + arguments.add("--compiler_option_referencing_path=-P:semanticdb:sourceroot:${workDir}") arguments.add_all( [outputs[0]], - format_each = "--compiler_option=-P:semanticdb:targetroot:%s", + format_each = "--compiler_option_referencing_path=-P:semanticdb:targetroot:${path} %s", map_each = _semanticdb_directory_from_file, ) else: arguments.add_all( [outputs[0]], - format_each = "--compiler_option=-semanticdb-target:%s", + format_each = "--compiler_option_referencing_path=-semanticdb-target:${path} %s", map_each = _semanticdb_directory_from_file, ) + arguments.add("--compiler_option_referencing_path=-sourceroot:${workDir}") arguments.add("--compiler_option=-Ysemanticdb") g.out.providers.append( diff --git a/src/main/scala/higherkindness/rules_scala/workers/common/CommonArguments.scala b/src/main/scala/higherkindness/rules_scala/workers/common/CommonArguments.scala index bc52a046..d735b331 100644 --- a/src/main/scala/higherkindness/rules_scala/workers/common/CommonArguments.scala +++ b/src/main/scala/higherkindness/rules_scala/workers/common/CommonArguments.scala @@ -7,6 +7,7 @@ import common.sandbox.SandboxUtil import net.sourceforge.argparse4j.impl.{Arguments => ArgumentsImpl} import net.sourceforge.argparse4j.inf.{Argument, ArgumentParser, ArgumentType, Namespace} import java.util.{Collections, List as JList} +import scala.annotation.nowarn import scala.collection.mutable.Buffer import scala.jdk.CollectionConverters.* import java.io.File @@ -17,6 +18,18 @@ class CommonArguments private ( val compilerBridge: Path, val compilerClasspath: List[Path], val compilerOptions: List[String], + + /** + * With [[https://bazel.build/remote/multiplex#multiplex_sandboxing multiplex sandboxing]], Bazel generates a separate + * sandbox directory for each worker invocation, which means the paths to build artifacts won't be known until the + * execution phase. However, compiler options are usually generated during the analysis phase. + * + * Some compiler options (currently, only those regarding SemanticDB) reference paths to build artifacts and need to + * be adjusted to be relative to the sandbox directory. It's more performant for those options to be passed in a + * separate list so we don't have to scan through every compiler option and potentially modify it. In our experience, + * this resulted in a ~10% worker speedup. + */ + val compilerOptionsReferencingPaths: List[String], val classpath: List[Path], val debug: Boolean, val javaCompilerOptions: List[String], @@ -49,22 +62,22 @@ object Analysis { } object CommonArguments { - private val scala2SemanticDbTargetRootRegex = """-P:semanticdb:targetroot:(.*)""".r - private val scala3SemanticDbTargetRootRegex = """-semanticdb-target:(.*)""".r - - private def adjustCompilerOptions(workDir: Path, options: List[String]) = { - def adjustStringPath(path: String) = - SandboxUtil.getSandboxPath(workDir, Paths.get(path)).toString - - options.flatMap { - case scala2SemanticDbTargetRootRegex(path) => - List(s"-P:semanticdb:sourceroot:${workDir.toString}", s"-P:semanticdb:targetroot:${adjustStringPath(path)}") + private def adjustCompilerOptions(workDir: Path, options: List[String]) = options.map { option => + val i = option.lastIndexOf(' ') + val withPathReplaced = if (i == -1) { + option + } else { + val template = option.slice(0, i) + val path = option.slice(i + 1, option.length) - case scala3SemanticDbTargetRootRegex(path) => - List(s"-semanticdb-target:${adjustStringPath(path)}", s"-sourceroot:${workDir.toString}") - - case option => List(option) + template.replace( + "${path}": @nowarn("cat=lint-missing-interpolator"), + SandboxUtil.getSandboxPath(workDir, Paths.get(path)).toString, + ) } + + withPathReplaced + .replace("${workDir}": @nowarn("cat=lint-missing-interpolator"), workDir.toString) } /** @@ -95,6 +108,11 @@ object CommonArguments { .help("Compiler option") .action(ArgumentsImpl.append) .metavar("option") + parser + .addArgument("--compiler_option_referencing_path") + .help("Compiler option referencing the paths to build artifact(s)") + .action(ArgumentsImpl.append) + .metavar("option") parser .addArgument("--classpath") .help("Compilation classpath") @@ -195,9 +213,14 @@ object CommonArguments { analyses = analyses, compilerBridge = SandboxUtil.getSandboxPath(workDir, namespace.get[Path]("compiler_bridge")), compilerClasspath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("compiler_classpath")), - compilerOptions = adjustCompilerOptions( + compilerOptions = Option(namespace.getList[String]("compiler_option")) + .map(_.asScala.toList) + .getOrElse(List.empty), + compilerOptionsReferencingPaths = adjustCompilerOptions( workDir, - Option(namespace.getList[String]("compiler_option")).map(_.asScala.toList).getOrElse(List.empty), + Option(namespace.getList[String]("compiler_option_referencing_path")) + .map(_.asScala.toList) + .getOrElse(List.empty), ), classpath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("classpath")), debug = namespace.getBoolean("debug"), diff --git a/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala b/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala index c0313bf7..98fac434 100644 --- a/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala +++ b/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala @@ -214,10 +214,11 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] { .withClassesDirectory(classesOutputDir) .withJavacOptions(workRequest.javaCompilerOptions.toArray) .withScalacOptions( - Array.concat( - workRequest.plugins.map(p => s"-Xplugin:$p").toArray, - workRequest.compilerOptions.toArray, - ), + ( + workRequest.plugins.map(p => s"-Xplugin:$p") ++ + workRequest.compilerOptions ++ + workRequest.compilerOptionsReferencingPaths + ).toArray, ) val compilers = {