Implement java_toolchain.header_compiler_builtin_processors

PiperOrigin-RevId: 261701270
buchgr · Aug 5, 2019 · 05c967f · 05c967f
1 parent be26ba0
commit 05c967f
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 10 deletions.
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java
@@ -185,6 +185,10 @@ private void processCommandlineArgs(Deque<String> argQueue) throws InvalidComman
         case "--processors":
           collectProcessorArguments(processorNames, argQueue, "-");
           break;
+        case "--builtin_processors":
+          // TODO(b/138842734): add support for built-in processors
+          collectProcessorArguments(new ArrayList<>(), argQueue, "-");
+          break;
         case "--extclasspath":
         case "--extdir":
           collectFlagArguments(extClassPath, argQueue, "-");

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
@@ -240,6 +240,7 @@ && getTranslations().isEmpty()) {
     builder.setTempDirectory(tempDir(classJar, label));
     builder.setClassDirectory(classDir(classJar, label));
     builder.setPlugins(attributes.plugins().plugins());
+    builder.setBuiltinProcessorNames(javaToolchain.getHeaderCompilerBuiltinProcessors());
     builder.setExtraData(JavaCommon.computePerPackageData(ruleContext, javaToolchain));
     builder.setStrictJavaDeps(attributes.getStrictJavaDeps());
     builder.setFixDepsTool(getJavaConfiguration().getFixDepsTool());

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java
@@ -22,6 +22,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -164,6 +165,7 @@ public void extend(ExtraActionInfo.Builder builder, List<String> arguments) {
   private PathFragment tempDirectory;
   private PathFragment classDirectory;
   private JavaPluginInfo plugins = JavaPluginInfo.empty();
+  private ImmutableSet<String> builtinProcessorNames = ImmutableSet.of();
   private NestedSet<Artifact> extraData = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER);
   private Label targetLabel;
   @Nullable private String injectingRuleKind;
@@ -336,6 +338,9 @@ private CustomCommandLine buildParamFileContents(
     result.addExecPaths("--sourcepath", sourcePathEntries);
     result.addExecPaths("--processorpath", plugins.processorClasspath());
     result.addAll("--processors", plugins.processorClasses());
+    result.addAll(
+        "--builtin_processors",
+        Sets.intersection(plugins.processorClasses().toSet(), builtinProcessorNames));
     result.addExecPaths("--source_jars", ImmutableList.copyOf(sourceJars));
     result.addExecPaths("--sources", sourceFiles);
     if (!javacOpts.isEmpty()) {
@@ -527,6 +532,13 @@ public JavaCompileActionBuilder setPlugins(JavaPluginInfo plugins) {
     return this;
   }
 
+  public JavaCompileActionBuilder setBuiltinProcessorNames(
+      ImmutableSet<String> builtinProcessorNames) {
+    this.builtinProcessorNames =
+        checkNotNull(builtinProcessorNames, "builtinProcessorNames must not be null");
+    return this;
+  }
+
   public void setExtraData(NestedSet<Artifact> extraData) {
     checkNotNull(extraData, "extraData must not be null");
     checkState(this.extraData.isEmpty());

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java
@@ -24,6 +24,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ExecutionRequirements;
@@ -235,13 +236,21 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab
       compileTimeDependencyArtifacts = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
     }
 
-    // The compilation uses API-generating annotation processors and has to fall back to
-    // javac-turbine.
+    // Enable the direct classpath optimization if there are no annotation processors.
     // N.B. we only check if the processor classes are empty, we don't care if there is plugin
     // data or dependencies if there are no annotation processors to run. This differs from
     // javac where java_plugin may be used with processor_class unset to declare Error Prone
     // plugins.
-    boolean requiresAnnotationProcessing = !plugins.processorClasses().isEmpty();
+    boolean useDirectClasspath = plugins.processorClasses().isEmpty();
+
+    // Use the optimized 'direct' implementation if it is available, and either there are no
+    // annotation processors or they are built in to the tool and listed in
+    // java_toolchain.header_compiler_direct_processors.
+    boolean useHeaderCompilerDirect =
+        javaToolchain.getHeaderCompilerDirect() != null
+            && javaToolchain
+                .getHeaderCompilerBuiltinProcessors()
+                .containsAll(plugins.processorClasses().toSet());
 
     SpawnAction.Builder builder = new SpawnAction.Builder();
 
@@ -263,7 +272,7 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab
     builder.addInputs(sourceFiles);
 
     FilesToRunProvider headerCompiler =
-        (!requiresAnnotationProcessing && javaToolchain.getHeaderCompilerDirect() != null)
+        useHeaderCompilerDirect
             ? javaToolchain.getHeaderCompilerDirect()
             : javaToolchain.getHeaderCompiler();
     // The header compiler is either a jar file that needs to be executed using
@@ -318,9 +327,7 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab
       builder.addResultConsumer(createResultConsumer(outputDepsProto));
     }
 
-    // The action doesn't require annotation processing, so use the non-javac-based turbine
-    // implementation.
-    if (!requiresAnnotationProcessing) {
+    if (useDirectClasspath) {
       NestedSet<Artifact> classpath;
       if (!directJars.isEmpty() || classpathEntries.isEmpty()) {
         classpath = directJars;
@@ -346,13 +353,23 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab
     // annotation processing.
 
     builder.addTransitiveInputs(classpathEntries);
-    builder.addTransitiveInputs(plugins.processorClasspath());
-    builder.addTransitiveInputs(plugins.data());
+    if (!useHeaderCompilerDirect) {
+      builder.addTransitiveInputs(plugins.processorClasspath());
+      builder.addTransitiveInputs(plugins.data());
+    }
     builder.addTransitiveInputs(compileTimeDependencyArtifacts);
 
     commandLine.addExecPaths("--classpath", classpathEntries);
     commandLine.addAll("--processors", plugins.processorClasses());
-    commandLine.addExecPaths("--processorpath", plugins.processorClasspath());
+    commandLine.addAll(
+        "--builtin_processors",
+        Sets.intersection(
+            plugins.processorClasses().toSet(),
+            javaToolchain.getHeaderCompilerBuiltinProcessors()));
+    commandLine.addAll("--processors", plugins.processorClasses());
+    if (!useHeaderCompilerDirect) {
+      commandLine.addExecPaths("--processorpath", plugins.processorClasspath());
+    }
     if (strictJavaDeps != StrictDepsMode.OFF) {
       commandLine.addExecPaths("--direct_dependencies", directJars);
       if (!compileTimeDependencyArtifacts.isEmpty()) {