From a9e30d226e0083fe4fe2e14c664c66a5970abd74 Mon Sep 17 00:00:00 2001 From: Rumou Duan Date: Fri, 24 Mar 2017 18:35:40 +0000 Subject: Add a GenerateJ2objcHeaderMap action to generate the J2ObjC header mapping. The header mapping used to be generated by J2ObjcTranspilationAction and consumed by dependent J2ObjcTranspilationActions. This setup led to chained J2ObjcTranspilationActions and long critical path build time. With GenerateJ2objcHeaderMap action, we break that chain and shorten the critical path build time. This feature is guarded behind a new build flag, --experimental_j2objc_header_map. -- PiperOrigin-RevId: 151149906 MOS_MIGRATED_REVID=151149906 --- .../build/lib/rules/objc/J2ObjcAspect.java | 90 ++++++++++++++++------ .../lib/rules/objc/J2ObjcCommandLineOptions.java | 7 ++ .../build/lib/rules/objc/J2ObjcConfiguration.java | 10 +++ 3 files changed, 82 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java index 237b124c1d..204329d43f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ParameterFile; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.analysis.ConfiguredAspect; import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -170,6 +171,15 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF .value( Label.parseAbsoluteUnchecked( toolsRepository + "//tools/j2objc:j2objc_wrapper"))) + .add( + attr("$j2objc_header_map", LABEL) + .allowedFileTypes(FileType.of(".py")) + .cfg(HOST) + .exec() + .singleArtifact() + .value( + Label.parseAbsoluteUnchecked( + toolsRepository + "//tools/j2objc:j2objc_header_map"))) .add( attr("$jre_emul_jar", LABEL) .cfg(HOST) @@ -423,7 +433,11 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF // J2ObjC merges all transitive input header mapping files into one header mapping file, // so we only need to re-export other dependent output header mapping files in proto rules and // rules where J2ObjC is not run (e.g., no sources). - if (isProtoRule(base) || exportedHeaderMappingFiles.isEmpty()) { + // We also add the transitive header mapping files if experimental J2ObjC header mapping is + // turned on. The experimental support does not merge transitive input header mapping files. + boolean experimentalJ2ObjcHeaderMap = + ruleContext.getFragment(J2ObjcConfiguration.class).experimentalJ2ObjcHeaderMap(); + if (isProtoRule(base) || exportedHeaderMappingFiles.isEmpty() || experimentalJ2ObjcHeaderMap) { exportedHeaderMappingFiles.addTransitive( depJ2ObjcMappingFileProvider.getHeaderMappingFiles()); } @@ -507,8 +521,12 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF argBuilder.addJoinExecPaths("--header-mapping", ",", depsHeaderMappingFiles); } + boolean experimentalJ2ObjcHeaderMap = + ruleContext.getFragment(J2ObjcConfiguration.class).experimentalJ2ObjcHeaderMap(); Artifact outputHeaderMappingFile = j2ObjcOutputHeaderMappingFile(ruleContext); - argBuilder.addExecPath("--output-header-mapping", outputHeaderMappingFile); + if (!experimentalJ2ObjcHeaderMap) { + argBuilder.addExecPath("--output-header-mapping", outputHeaderMappingFile); + } NestedSet depsClassMappingFiles = depJ2ObjcMappingFileProvider.getClassMappingFiles(); if (!depsClassMappingFiles.isEmpty()) { @@ -542,29 +560,51 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF ParameterFile.ParameterFileType.UNQUOTED, ISO_8859_1)); - SpawnAction.Builder builder = new SpawnAction.Builder() - .setMnemonic("TranspilingJ2objc") - .setExecutable(ruleContext.getPrerequisiteArtifact("$j2objc_wrapper", Mode.HOST)) - .addInput(ruleContext.getPrerequisiteArtifact("$j2objc_wrapper", Mode.HOST)) - .addInput(j2ObjcDeployJar) - .addInput(bootclasspathJar) - .addInputs(sources) - .addInputs(sourceJars) - .addTransitiveInputs(compileTimeJars) - .addTransitiveInputs(JavaHelper.getHostJavabaseInputs(ruleContext)) - .addTransitiveInputs(depsHeaderMappingFiles) - .addTransitiveInputs(depsClassMappingFiles) - .addInput(paramFile) - .setCommandLine(CustomCommandLine.builder() - .addPaths("@%s", paramFile.getExecPath()) - .build()) - .addOutputs(j2ObjcSource.getObjcSrcs()) - .addOutputs(j2ObjcSource.getObjcHdrs()) - .addOutput(outputHeaderMappingFile) - .addOutput(outputDependencyMappingFile) - .addOutput(archiveSourceMappingFile); - - ruleContext.registerAction(builder.build(ruleContext)); + SpawnAction.Builder transpilationAction = + new SpawnAction.Builder() + .setMnemonic("TranspilingJ2objc") + .setExecutable(ruleContext.getPrerequisiteArtifact("$j2objc_wrapper", Mode.HOST)) + .addInput(ruleContext.getPrerequisiteArtifact("$j2objc_wrapper", Mode.HOST)) + .addInput(j2ObjcDeployJar) + .addInput(bootclasspathJar) + .addInputs(sources) + .addInputs(sourceJars) + .addTransitiveInputs(compileTimeJars) + .addTransitiveInputs(JavaHelper.getHostJavabaseInputs(ruleContext)) + .addTransitiveInputs(depsHeaderMappingFiles) + .addTransitiveInputs(depsClassMappingFiles) + .addInput(paramFile) + .setCommandLine( + CustomCommandLine.builder().addPaths("@%s", paramFile.getExecPath()).build()) + .addOutputs(j2ObjcSource.getObjcSrcs()) + .addOutputs(j2ObjcSource.getObjcHdrs()) + .addOutput(outputDependencyMappingFile) + .addOutput(archiveSourceMappingFile); + if (!experimentalJ2ObjcHeaderMap) { + transpilationAction.addOutput(outputHeaderMappingFile); + } + ruleContext.registerAction(transpilationAction.build(ruleContext)); + + if (experimentalJ2ObjcHeaderMap) { + CustomCommandLine.Builder headerMapCommandLine = CustomCommandLine.builder(); + if (!Iterables.isEmpty(sources)) { + headerMapCommandLine.addJoinExecPaths("--source_files", ",", sources); + } + if (!Iterables.isEmpty(sourceJars)) { + headerMapCommandLine.addJoinExecPaths("--source_jars", ",", sourceJars); + } + headerMapCommandLine.addExecPath("--output_mapping_file", outputHeaderMappingFile); + ruleContext.registerAction(new SpawnAction.Builder() + .setMnemonic("GenerateJ2objcHeaderMap") + .setExecutable(ruleContext.getPrerequisiteArtifact("$j2objc_header_map", Mode.HOST)) + .addInput(ruleContext.getPrerequisiteArtifact("$j2objc_header_map", Mode.HOST)) + .addInputs(sources) + .addInputs(sourceJars) + .setCommandLine(headerMapCommandLine.build()) + .useParameterFile(ParameterFileType.SHELL_QUOTED) + .addOutput(outputHeaderMappingFile) + .build(ruleContext)); + } return new J2ObjcMappingFileProvider( NestedSetBuilder.stableOrder().add(outputHeaderMappingFile).build(), diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcCommandLineOptions.java index b54ef9ee84..4c6a42c67a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcCommandLineOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcCommandLineOptions.java @@ -51,6 +51,13 @@ public class J2ObjcCommandLineOptions extends FragmentOptions { ) public boolean explicitJreDeps; + @Option(name = "experimental_j2objc_header_map", + defaultValue = "false", + category = "flags", + help = "Whether to generate J2ObjC header map in parallel of J2ObjC transpilation." + ) + public boolean experimentalJ2ObjcHeaderMap; + @Override public void addAllLabels(Multimap labelMap) {} } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcConfiguration.java index bfe3074c5d..0253f6e386 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcConfiguration.java @@ -87,10 +87,12 @@ public class J2ObjcConfiguration extends Fragment { private final List translationFlags; private final boolean removeDeadCode; private final boolean explicitJreDeps; + private final boolean experimentalJ2ObjcHeaderMap; J2ObjcConfiguration(J2ObjcCommandLineOptions j2ObjcOptions) { this.removeDeadCode = j2ObjcOptions.removeDeadCode; this.explicitJreDeps = j2ObjcOptions.explicitJreDeps; + this.experimentalJ2ObjcHeaderMap = j2ObjcOptions.experimentalJ2ObjcHeaderMap; this.translationFlags = ImmutableList.builder() .addAll(J2OBJC_DEFAULT_TRANSLATION_FLAGS) .addAll(j2ObjcOptions.translationFlags) @@ -127,6 +129,14 @@ public class J2ObjcConfiguration extends Fragment { return explicitJreDeps; } + /** + * Returns whether to generate J2ObjC header map in a separate action in parallel of the J2ObjC + * transpilation action. + */ + public boolean experimentalJ2ObjcHeaderMap() { + return experimentalJ2ObjcHeaderMap; + } + @Override public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) { if (!Collections.disjoint(translationFlags, J2OBJC_BLACKLISTED_TRANSLATION_FLAGS)) { -- cgit v1.2.3