From 41aa37270a87e61bdf739284602842d6a23f94d1 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 2 Feb 2017 19:16:37 +0000 Subject: Experimental feature to do header thinning of Objective-C compilation actions. -- PiperOrigin-RevId: 146385106 MOS_MIGRATED_REVID=146385106 --- .../build/lib/rules/objc/CompilationSupport.java | 22 ++ .../lib/rules/objc/IntermediateArtifacts.java | 5 + .../lib/rules/objc/LegacyCompilationSupport.java | 240 +++++++++++++++------ .../lib/rules/objc/ObjcCommandLineOptions.java | 29 +++ .../build/lib/rules/objc/ObjcCompileAction.java | 132 +++++++++--- .../build/lib/rules/objc/ObjcConfiguration.java | 14 ++ .../lib/rules/objc/ObjcConfigurationLoader.java | 12 +- .../build/lib/rules/objc/ObjcRuleClasses.java | 25 ++- 8 files changed, 388 insertions(+), 91 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java index 7f2191a1c7..a47526d4db 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java @@ -1022,4 +1022,26 @@ public abstract class CompilationSupport { } return parents.build(); } + + /** + * Returns true when ObjC header thinning is enabled via configuration and an a valid + * header_scanner executable target is provided. + */ + protected boolean isHeaderThinningEnabled() { + if (objcConfiguration.useExperimentalHeaderThinning() + && ruleContext.isAttrDefined(ObjcRuleClasses.HEADER_SCANNER_ATTRIBUTE, BuildType.LABEL)) { + FilesToRunProvider tool = getHeaderThinningToolExecutable(); + // Additional here to ensure that an Executable Artifact exists to disable where the tool + // is an empty filegroup + return tool != null && tool.getExecutable() != null; + } + return false; + } + + protected FilesToRunProvider getHeaderThinningToolExecutable() { + return ruleContext + .getPrerequisite(ObjcRuleClasses.HEADER_SCANNER_ATTRIBUTE, Mode.HOST) + .getProvider(FilesToRunProvider.class); + } + } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java index db3c1ad425..e9b795b4c1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java @@ -270,6 +270,11 @@ public final class IntermediateArtifacts { } } + /** The artifact for the .headers file output by the header thinning action for this source. */ + public Artifact headersListFile(Artifact source) { + return inUniqueObjsDir(source, ".headers_list"); + } + /** * The artifact for the .gcno file that should be generated when compiling the {@code source} * artifact. diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java index 3dfdd5f4c8..b8fc926967 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java @@ -40,10 +40,13 @@ import com.google.common.base.Joiner; import com.google.common.base.Optional; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; +import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; @@ -64,11 +67,14 @@ import com.google.devtools.build.lib.rules.apple.AppleToolchain; import com.google.devtools.build.lib.rules.apple.DottedVersion; import com.google.devtools.build.lib.rules.apple.Platform; import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; +import com.google.devtools.build.lib.rules.cpp.CppFileTypes; import com.google.devtools.build.lib.rules.cpp.CppModuleMap; +import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.List; +import java.util.Map.Entry; /** * Constructs command lines for objc compilation, archiving, and linking. Uses hard-coded @@ -92,14 +98,25 @@ public class LegacyCompilationSupport extends CompilationSupport { }; /** - * Returns information about the given rule's compilation artifacts. Dependencies specified - * in the current rule's attributes are obtained via {@code ruleContext}. Output locations - * are determined using the given {@code intermediateArtifacts} object. The fact that these - * are distinct objects allows the caller to generate compilation actions pertaining to - * a configuration separate from the current rule's configuration. + * Set of {@link com.google.devtools.build.lib.util.FileType} of source artifacts that are + * compatible with header thinning. */ - static CompilationArtifacts compilationArtifacts(RuleContext ruleContext, - IntermediateArtifacts intermediateArtifacts) { + private static final FileTypeSet SOURCES_FOR_HEADER_THINNING = + FileTypeSet.of( + CppFileTypes.OBJC_SOURCE, + CppFileTypes.OBJCPP_SOURCE, + CppFileTypes.CPP_SOURCE, + CppFileTypes.C_SOURCE); + + /** + * Returns information about the given rule's compilation artifacts. Dependencies specified in the + * current rule's attributes are obtained via {@code ruleContext}. Output locations are determined + * using the given {@code intermediateArtifacts} object. The fact that these are distinct objects + * allows the caller to generate compilation actions pertaining to a configuration separate from + * the current rule's configuration. + */ + static CompilationArtifacts compilationArtifacts( + RuleContext ruleContext, IntermediateArtifacts intermediateArtifacts) { PrerequisiteArtifacts srcs = ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).errorsForNonMatching(SRCS_TYPE); return new CompilationArtifacts.Builder() @@ -168,6 +185,23 @@ public class LegacyCompilationSupport extends CompilationSupport { Iterable priorityHeaders, Optional moduleMap) { ImmutableList.Builder objFiles = new ImmutableList.Builder<>(); + ImmutableMap sourceFilesToHeadersListFiles = ImmutableMap.of(); + if (isHeaderThinningEnabled()) { + sourceFilesToHeadersListFiles = + generateHeadersListArtifactsForSources( + Iterables.concat( + compilationArtifacts.getSrcs(), compilationArtifacts.getNonArcSrcs())); + if (!sourceFilesToHeadersListFiles.isEmpty()) { + registerHeaderScanningAction( + sourceFilesToHeadersListFiles, + objcProvider, + compilationArtifacts, + priorityHeaders, + moduleMap, + extraCompileArgs); + } + } + for (Artifact sourceFile : compilationArtifacts.getSrcs()) { Artifact objFile = intermediateArtifacts.objFile(sourceFile); objFiles.add(objFile); @@ -189,7 +223,8 @@ public class LegacyCompilationSupport extends CompilationSupport { priorityHeaders, moduleMap, compilationArtifacts, - Iterables.concat(extraCompileArgs, ImmutableList.of("-fobjc-arc"))); + Iterables.concat(extraCompileArgs, ImmutableList.of("-fobjc-arc")), + sourceFilesToHeadersListFiles); } } for (Artifact nonArcSourceFile : compilationArtifacts.getNonArcSrcs()) { @@ -212,7 +247,8 @@ public class LegacyCompilationSupport extends CompilationSupport { priorityHeaders, moduleMap, compilationArtifacts, - Iterables.concat(extraCompileArgs, ImmutableList.of("-fno-objc-arc"))); + Iterables.concat(extraCompileArgs, ImmutableList.of("-fno-objc-arc")), + sourceFilesToHeadersListFiles); } } @@ -223,38 +259,30 @@ public class LegacyCompilationSupport extends CompilationSupport { } } - private CustomCommandLine compileActionCommandLine( - Artifact sourceFile, - Artifact objFile, + /** + * Configures a {@link CustomCommandLine.Builder} with common compilation arguments for building + * Objective-C sources. + * + *

The command line arguments generated by this method are common to both ObjcCompile and + * ObjcHeaderScanning actions. Arguments that are only necessary or useful when doing more than + * preprocessing (i.e. building an object file via ObjcCompile) are not specified here. + * + * @param commandLine existing {@link CustomCommandLine.Builder} to add common arguments to + * @return passed in {@code commandLine} with common arguments added + */ + private CustomCommandLine.Builder commonCompileActionCommandLine( + CustomCommandLine.Builder commandLine, ObjcProvider objcProvider, Iterable priorityHeaders, Optional moduleMap, Optional pchFile, - Optional dotdFile, Iterable otherFlags, - boolean collectCodeCoverage, boolean isCPlusPlusSource) { - CustomCommandLine.Builder commandLine = new CustomCommandLine.Builder().add(CLANG); - if (isCPlusPlusSource) { commandLine.add("-stdlib=libc++"); commandLine.add("-std=gnu++11"); } - // The linker needs full debug symbol information to perform binary dead-code stripping. - if (objcConfiguration.shouldStripBinary()) { - commandLine.add("-g"); - } - - List coverageFlags = ImmutableList.of(); - if (collectCodeCoverage) { - if (buildConfiguration.isLLVMCoverageMapFormatEnabled()) { - coverageFlags = CLANG_LLVM_COVERAGE_FLAGS; - } else { - coverageFlags = CLANG_GCOV_COVERAGE_FLAGS; - } - } - commandLine .add(compileFlagsForClang(appleConfiguration)) .add(commonLinkAndCompileFlagsForClang(objcProvider, objcConfiguration, appleConfiguration)) @@ -266,9 +294,61 @@ public class LegacyCompilationSupport extends CompilationSupport { .addBeforeEachPath("-isystem", objcProvider.get(INCLUDE_SYSTEM)) .add(otherFlags) .addFormatEach("-D%s", objcProvider.get(DEFINE)) - .add(coverageFlags) .add(getCompileRuleCopts()); + // Add module map arguments. + if (moduleMap.isPresent()) { + // If modules are enabled for the rule, -fmodules is added to the copts already. (This implies + // module map usage). Otherwise, we need to pass -fmodule-maps. + if (!attributes.enableModules()) { + commandLine.add("-fmodule-maps"); + } + // -fmodule-map-file only loads the module in Xcode 7, so we add the module maps's directory + // to the include path instead. + // TODO(bazel-team): Use -fmodule-map-file when Xcode 6 support is dropped. + commandLine + .add("-iquote") + .add(moduleMap.get().getArtifact().getExecPath().getParentDirectory().toString()) + .add("-fmodule-name=" + moduleMap.get().getName()); + } + + return commandLine; + } + + private CustomCommandLine compileActionCommandLine( + Artifact sourceFile, + Artifact objFile, + ObjcProvider objcProvider, + Iterable priorityHeaders, + Optional moduleMap, + Optional pchFile, + Optional dotdFile, + Iterable otherFlags, + boolean collectCodeCoverage, + boolean isCPlusPlusSource) { + CustomCommandLine.Builder commandLine = + commonCompileActionCommandLine( + new CustomCommandLine.Builder().add(CLANG), + objcProvider, + priorityHeaders, + moduleMap, + pchFile, + otherFlags, + isCPlusPlusSource); + + // The linker needs full debug symbol information to perform binary dead-code stripping. + if (objcConfiguration.shouldStripBinary()) { + commandLine.add("-g"); + } + + if (collectCodeCoverage) { + if (buildConfiguration.isLLVMCoverageMapFormatEnabled()) { + commandLine.add(CLANG_LLVM_COVERAGE_FLAGS); + } else { + commandLine.add(CLANG_GCOV_COVERAGE_FLAGS); + } + } + // Add input source file arguments commandLine.add("-c"); if (!sourceFile.getExecPath().isAbsolute() @@ -305,31 +385,7 @@ public class LegacyCompilationSupport extends CompilationSupport { // Add Dotd file arguments. if (dotdFile.isPresent()) { - commandLine - .add("-MD") - .addExecPath("-MF", dotdFile.get()); - } - - // Add module map arguments. - if (moduleMap.isPresent()) { - // If modules are enabled for the rule, -fmodules is added to the copts already. (This implies - // module map usage). Otherwise, we need to pass -fmodule-maps. - if (!attributes.enableModules()) { - commandLine.add("-fmodule-maps"); - } - // -fmodule-map-file only loads the module in Xcode 7, so we add the module maps's directory - // to the include path instead. - // TODO(bazel-team): Use -fmodule-map-file when Xcode 6 support is dropped. - commandLine - .add("-iquote") - .add( - moduleMap - .get() - .getArtifact() - .getExecPath() - .getParentDirectory() - .toString()) - .add("-fmodule-name=" + moduleMap.get().getName()); + commandLine.add("-MD").addExecPath("-MF", dotdFile.get()); } return commandLine.build(); @@ -342,7 +398,8 @@ public class LegacyCompilationSupport extends CompilationSupport { Iterable priorityHeaders, Optional moduleMap, CompilationArtifacts compilationArtifacts, - Iterable otherFlags) { + Iterable otherFlags, + ImmutableMap sourceToOutput) { boolean isCPlusPlusSource = ObjcRuleClasses.CPP_SOURCES.matches(sourceFile.getExecPath()); boolean runCodeCoverage = buildConfiguration.isCodeCoverageEnabled() && ObjcRuleClasses.isInstrumentable(sourceFile); @@ -372,18 +429,23 @@ public class LegacyCompilationSupport extends CompilationSupport { } // TODO(bazel-team): Remove private headers from inputs once they're added to the provider. - ruleContext.registerAction( + ObjcCompileAction.Builder compileBuilder = ObjcCompileAction.Builder.createObjcCompileActionBuilderWithAppleEnv( appleConfiguration, appleConfiguration.getSingleArchPlatform()) .setDotdPruningPlan(objcConfiguration.getDotdPruningPlan()) .setSourceFile(sourceFile) .addTransitiveHeaders(objcProvider.get(HEADER)) - .addHeaders(compilationArtifacts.getPrivateHdrs()) + .addHeaders(compilationArtifacts.getPrivateHdrs()) .addTransitiveMandatoryInputs(moduleMapInputs) .addTransitiveMandatoryInputs(objcProvider.get(STATIC_FRAMEWORK_FILE)) .addTransitiveMandatoryInputs(objcProvider.get(DYNAMIC_FRAMEWORK_FILE)) .setDotdFile(dotdFile) - .addInputs(compilationArtifacts.getPchFile().asSet()) + .addMandatoryInputs(compilationArtifacts.getPchFile().asSet()); + if (sourceToOutput.containsKey(sourceFile)) { + compileBuilder.setHeadersListFile(sourceToOutput.get(sourceFile)); + } + ruleContext.registerAction( + compileBuilder .setMnemonic("ObjcCompile") .setExecutable(xcrunwrapper(ruleContext)) .setCommandLine(commandLine) @@ -456,6 +518,64 @@ public class LegacyCompilationSupport extends CompilationSupport { .build(ruleContext.getActionOwner())); } + private ImmutableMap generateHeadersListArtifactsForSources( + Iterable sources) { + ImmutableMap.Builder sourceFileToHeadersListBuilder = + ImmutableMap.builder(); + for (Artifact source : sources) { + // Tree artifacts are not currently supported + if (!source.isTreeArtifact() && SOURCES_FOR_HEADER_THINNING.matches(source.getFilename())) { + sourceFileToHeadersListBuilder.put(source, intermediateArtifacts.headersListFile(source)); + } + } + return sourceFileToHeadersListBuilder.build(); + } + + private void registerHeaderScanningAction( + ImmutableMap sourceFilesToHeadersListFiles, + ObjcProvider objcProvider, + CompilationArtifacts compilationArtifacts, + Iterable priorityHeaders, + Optional moduleMap, + Iterable otherFlags) { + FilesToRunProvider headerScannerExecutable = getHeaderThinningToolExecutable(); + + CustomCommandLine.Builder cmdLine = CustomCommandLine.builder(); + for (Entry entry : sourceFilesToHeadersListFiles.entrySet()) { + cmdLine.addJoinPaths( + ":", Lists.newArrayList(entry.getKey().getExecPath(), entry.getValue().getExecPath())); + } + cmdLine.add("--"); + commonCompileActionCommandLine( + cmdLine, + objcProvider, + priorityHeaders, + moduleMap, + compilationArtifacts.getPchFile(), + otherFlags, + true); + + NestedSet moduleMapInputs = NestedSetBuilder.emptySet(Order.STABLE_ORDER); + if (objcConfiguration.moduleMapsEnabled()) { + moduleMapInputs = objcProvider.get(MODULE_MAP); + } + + ruleContext.registerAction( + new SpawnAction.Builder() + .setMnemonic("ObjcHeaderScanning") + .setExecutable(headerScannerExecutable) + .setCommandLine(cmdLine.build()) + .addInputs(sourceFilesToHeadersListFiles.keySet()) + .addInputs(compilationArtifacts.getPrivateHdrs()) + .addInputs(compilationArtifacts.getPchFile().asSet()) + .addTransitiveInputs(objcProvider.get(ObjcProvider.HEADER)) + .addTransitiveInputs(moduleMapInputs) + .addTransitiveInputs(objcProvider.get(ObjcProvider.STATIC_FRAMEWORK_FILE)) + .addTransitiveInputs(objcProvider.get(ObjcProvider.DYNAMIC_FRAMEWORK_FILE)) + .addOutputs(sourceFilesToHeadersListFiles.values()) + .build(ruleContext)); + } + private void registerArchiveActions(List objFiles, Artifact archive) { Artifact objList = intermediateArtifacts.archiveObjList(); registerObjFilelistAction(objFiles, objList); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java index 6df5a446a2..6c9800fbc8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java @@ -275,6 +275,35 @@ public class ObjcCommandLineOptions extends FragmentOptions { ) public boolean enableAppleBinaryNativeProtos; + @Option( + name = "experimental_objc_header_thinning", + defaultValue = "false", + category = "flags", + help = + "If set then ObjcCompile actions will have their action inputs reduced by running a tool " + + "to detect which headers are actually required for compilation." + ) + public boolean experimentalObjcHeaderThinning; + + @Option( + name = "objc_header_scanner_tool", + defaultValue = "@bazel_tools//tools/objc:header_scanner", + category = "undocumented", + converter = LabelConverter.class, + help = + "Location of tool to scan Objective-C code for inclusions and output a .headers_list " + + "file." + ) + public Label objcHeaderScannerTool; + + @Override + public FragmentOptions getHost(boolean fallback) { + ObjcCommandLineOptions host = (ObjcCommandLineOptions) super.getHost(fallback); + // This should have the same value in both target and host configurations + host.objcHeaderScannerTool = this.objcHeaderScannerTool; + return host; + } + @SuppressWarnings("unchecked") @Override public List> getPotentialSplitTransitions() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java index 70509aa493..55e1baefff 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java @@ -21,6 +21,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.ActionInput; @@ -35,6 +36,7 @@ import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; @@ -47,11 +49,15 @@ import com.google.devtools.build.lib.rules.cpp.HeaderDiscovery.DotdPruningMode; import com.google.devtools.build.lib.rules.cpp.IncludeScanningContext; import com.google.devtools.build.lib.util.DependencySet; import com.google.devtools.build.lib.util.Fingerprint; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; -import java.util.HashMap; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.Map; +import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; /** @@ -60,6 +66,10 @@ import javax.annotation.concurrent.GuardedBy; *

We don't use a plain SpawnAction here because we implement .d input pruning, which requires * post-execution filtering of input artifacts. * + *

Additionally the header thinning feature is implemented here which, like .d input pruning, + * reduces the action inputs to just the required set of headers for improved performance. Unlike + * dotd it does so via the discoverInputs mechanism before execution. + * *

We don't use a CppCompileAction because the ObjcCompileAction uses custom logic instead of the * CROSSTOOL to construct its command line. */ @@ -79,7 +89,7 @@ public class ObjcCompileAction extends SpawnAction { public Iterable getInputFiles() { return ImmutableList.builder() .addAll(super.getInputFiles()) - .addAll(filterHeaderFiles()) + .addAll(discoveredInputs) .build(); } } @@ -89,6 +99,9 @@ public class ObjcCompileAction extends SpawnAction { private final NestedSet mandatoryInputs; private final HeaderDiscovery.DotdPruningMode dotdPruningPlan; private final NestedSet headers; + private final Artifact headersListFile; + + private Iterable discoveredInputs; // This can be read/written from multiple threads, so accesses must be synchronized. @GuardedBy("this") @@ -114,7 +127,8 @@ public class ObjcCompileAction extends SpawnAction { Artifact sourceFile, NestedSet mandatoryInputs, HeaderDiscovery.DotdPruningMode dotdPruningPlan, - NestedSet headers) { + NestedSet headers, + @Nullable Artifact headersListFile) { super( owner, tools, @@ -135,8 +149,9 @@ public class ObjcCompileAction extends SpawnAction { this.sourceFile = sourceFile; this.mandatoryInputs = mandatoryInputs; this.dotdPruningPlan = dotdPruningPlan; - this.inputsKnown = (dotdPruningPlan == DotdPruningMode.DO_NOT_USE); + this.inputsKnown = (dotdPruningPlan == DotdPruningMode.DO_NOT_USE && headersListFile == null); this.headers = headers; + this.headersListFile = headersListFile; } private Iterable filterHeaderFiles() { @@ -172,8 +187,50 @@ public class ObjcCompileAction extends SpawnAction { } @Override - public Iterable discoverInputs(ActionExecutionContext actionExecutionContext) { - return filterHeaderFiles(); + public Iterable discoverInputs(ActionExecutionContext actionExecutionContext) + throws ActionExecutionException, InterruptedException { + if (headersListFile != null) { + discoveredInputs = findRequiredHeaderInputs(); + // For header thinning we update action inputs here as it won't be done post execution + updateActionInputs(discoveredInputs); + } else { + discoveredInputs = filterHeaderFiles(); + } + return discoveredInputs; + } + + /** Reads the header scanning output file and discovers all those headers as outputs. */ + private Iterable findRequiredHeaderInputs() + throws ActionExecutionException, InterruptedException { + try { + Map map = getAllowedDerivedInputsMap(false); + ImmutableList.Builder includeBuilder = ImmutableList.builder(); + for (String line : + FileSystemUtils.readLines(headersListFile.getPath(), StandardCharsets.UTF_8)) { + if (line.isEmpty()) { + continue; + } + + Artifact header = map.get(new PathFragment(line)); + if (header == null) { + throw new ActionExecutionException( + String.format( + "Unable to map header file (%s) found during header scanning of %s." + + " This is usually the result of a case mismatch.", + line, sourceFile.getExecPathString()), + this, + true); + } + includeBuilder.add(header); + } + return includeBuilder.build(); + } catch (IOException ex) { + throw new ActionExecutionException( + String.format("Error reading headers file %s", headersListFile.getExecPathString()), + ex, + this, + false); + } } @Override @@ -209,7 +266,7 @@ public class ObjcCompileAction extends SpawnAction { public NestedSet discoverInputsFromDotdFiles( Path execRoot, ArtifactResolver artifactResolver) throws ActionExecutionException { if (dotdFile == null) { - return NestedSetBuilder.stableOrder().build(); + return NestedSetBuilder.emptySet(Order.STABLE_ORDER); } return new HeaderDiscovery.Builder() .setAction(this) @@ -217,7 +274,7 @@ public class ObjcCompileAction extends SpawnAction { .setDotdFile(dotdFile) .setDependencySet(processDepset(execRoot)) .setPermittedSystemIncludePrefixes(ImmutableList.of()) - .setAllowedDerivedinputsMap(getAllowedDerivedInputsMap()) + .setAllowedDerivedinputsMap(getAllowedDerivedInputsMap(true)) .build() .discoverInputsFromDotdFiles(execRoot, artifactResolver); } @@ -232,25 +289,27 @@ public class ObjcCompileAction extends SpawnAction { } } - /** Utility function that adds artifacts to an input map, but only if they are sources. */ - private void addToMapIfSource(Map map, Iterable artifacts) { - for (Artifact artifact : artifacts) { - if (!artifact.isSourceArtifact()) { - map.put(artifact.getExecPath(), artifact); + /** + * Returns a map of input and header artifacts for this action. + * + * @param excludeSourceArtifacts If true artifacts where {@link Artifact#isSourceArtifact()} is + * true are excluded from the map + */ + private Map getAllowedDerivedInputsMap(boolean excludeSourceArtifacts) { + // LinkedHashMap required here as it is not guaranteed that entries are unique and to preserve + // insertion order + Map allowedDerivedInputMap = new LinkedHashMap<>(); + for (Artifact artifact : Iterables.concat(mandatoryInputs, headers)) { + if (!excludeSourceArtifacts || !artifact.isSourceArtifact()) { + allowedDerivedInputMap.put(artifact.getExecPath(), artifact); } } + return allowedDerivedInputMap; } @Override public Iterable getAllowedDerivedInputs() { - return getAllowedDerivedInputsMap().values(); - } - - private Map getAllowedDerivedInputsMap() { - Map allowedDerivedInputMap = new HashMap<>(); - addToMapIfSource(allowedDerivedInputMap, getInputs()); - allowedDerivedInputMap.put(sourceFile.getExecPath(), sourceFile); - return allowedDerivedInputMap; + return getAllowedDerivedInputsMap(true).values(); } /** @@ -260,18 +319,17 @@ public class ObjcCompileAction extends SpawnAction { */ @VisibleForTesting @ThreadCompatible - public final synchronized void updateActionInputs(NestedSet discoveredInputs) + public final synchronized void updateActionInputs(Iterable discoveredInputs) throws ActionExecutionException { inputsKnown = false; - NestedSetBuilder inputs = NestedSetBuilder.stableOrder(); + Iterable inputs = Collections.emptyList(); Profiler.instance().startTask(ProfilerTask.ACTION_UPDATE, this); try { - inputs.addTransitive(mandatoryInputs); - inputs.addTransitive(discoveredInputs); + inputs = Iterables.concat(mandatoryInputs, discoveredInputs); inputsKnown = true; } finally { Profiler.instance().completeTask(ProfilerTask.ACTION_UPDATE); - setInputs(inputs.build()); + setInputs(inputs); } } @@ -280,9 +338,12 @@ public class ObjcCompileAction extends SpawnAction { Fingerprint f = new Fingerprint(); f.addString(GUID); f.addString(super.computeKey()); - f.addBoolean(dotdFile.artifact() == null); + f.addBoolean(dotdFile == null || dotdFile.artifact() == null); f.addBoolean(dotdPruningPlan == HeaderDiscovery.DotdPruningMode.USE); - f.addPath(dotdFile.getSafeExecPath()); + f.addBoolean(headersListFile == null); + if (dotdFile != null) { + f.addPath(dotdFile.getSafeExecPath()); + } return f.hexDigestAndReset(); } @@ -291,6 +352,7 @@ public class ObjcCompileAction extends SpawnAction { private DotdFile dotdFile; private Artifact sourceFile; + private Artifact headersListFile; private final NestedSetBuilder mandatoryInputs = new NestedSetBuilder<>(STABLE_ORDER); private HeaderDiscovery.DotdPruningMode dotdPruningPlan; private final NestedSetBuilder headers = NestedSetBuilder.stableOrder(); @@ -322,6 +384,17 @@ public class ObjcCompileAction extends SpawnAction { return this; } + /** + * Sets a .headers_list file that is generated for the header thinning feature. File is used to + * discover required inputs to compile action and update action inputs. + */ + public Builder setHeadersListFile(Artifact headersListFile) { + Preconditions.checkNotNull(headersListFile); + this.headersListFile = headersListFile; + this.addMandatoryInput(headersListFile); + return this; + } + /** Sets the source file that is being compiled in this action */ public Builder setSourceFile(Artifact sourceFile) { Preconditions.checkNotNull(sourceFile); @@ -408,7 +481,8 @@ public class ObjcCompileAction extends SpawnAction { sourceFile, mandatoryInputs.build(), dotdPruningPlan, - headers.build()); + headers.build(), + headersListFile); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java index 8d2bc203b0..3e4c260c8c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java @@ -80,6 +80,8 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment { private final boolean experimentalObjcLibrary; private final boolean enableAppleBinaryNativeProtos; private final HeaderDiscovery.DotdPruningMode dotdPruningPlan; + private final boolean experimentalHeaderThinning; + private final Label objcHeaderScannerTool; ObjcConfiguration(ObjcCommandLineOptions objcOptions, BuildConfiguration.Options options, @Nullable BlazeDirectories directories) { @@ -118,6 +120,8 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment { objcOptions.useDotdPruning ? HeaderDiscovery.DotdPruningMode.USE : HeaderDiscovery.DotdPruningMode.DO_NOT_USE; + this.experimentalHeaderThinning = objcOptions.experimentalObjcHeaderThinning; + this.objcHeaderScannerTool = objcOptions.objcHeaderScannerTool; } /** @@ -341,4 +345,14 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment { public HeaderDiscovery.DotdPruningMode getDotdPruningPlan() { return dotdPruningPlan; } + + /** Returns true if header thinning of ObjcCompile actions is enabled to reduce action inputs. */ + public boolean useExperimentalHeaderThinning() { + return experimentalHeaderThinning; + } + + /** Returns the label for the ObjC header scanner tool. */ + public Label getObjcHeaderScannerTool() { + return objcHeaderScannerTool; + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java index a23a234d52..ec8fc4e523 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java @@ -33,10 +33,20 @@ public class ObjcConfigurationLoader implements ConfigurationFragmentFactory { throws InvalidConfigurationException, InterruptedException { Options options = buildOptions.get(BuildConfiguration.Options.class); ObjcCommandLineOptions objcOptions = buildOptions.get(ObjcCommandLineOptions.class); - + validate(objcOptions); return new ObjcConfiguration(objcOptions, options, env.getBlazeDirectories()); } + private static void validate(ObjcCommandLineOptions objcOptions) + throws InvalidConfigurationException { + if (objcOptions.experimentalObjcHeaderThinning && objcOptions.useDotdPruning) { + throw new InvalidConfigurationException( + "Cannot use Objective-C dotd pruning " + + "(--objc_use_dotd_pruning) and experimental Objective-C header thinning " + + "(--experimental_objc_header_thinning) at the same time."); + } + } + @Override public Class creates() { return ObjcConfiguration.class; diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java index d775156ae7..3d848b897f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java @@ -70,6 +70,8 @@ public class ObjcRuleClasses { * Name of the attribute used for implicit dependency on the libtool wrapper. */ public static final String LIBTOOL_ATTRIBUTE = "$libtool"; + /** Name of the attribute used for implicit dependency on the header_scanner wrapper. */ + public static final String HEADER_SCANNER_ATTRIBUTE = ":header_scanner"; static final String CLANG = "clang"; static final String CLANG_PLUSPLUS = "clang++"; @@ -654,7 +656,7 @@ public class ObjcRuleClasses { Iterables.concat( ALLOWED_CC_DEPS_RULE_CLASSES, ImmutableList.of("experimental_objc_library")); - + @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { return builder @@ -741,6 +743,27 @@ public class ObjcRuleClasses { @import path_to_package_target; */ .add(attr("enable_modules", BOOLEAN)) + /* Provides the label for header_scanner tool that is used to scan inclusions for ObjC + sources and provide a list of required headers via a .header_list file. + + Either points to a label for a binary which can be executed for header scanning or an + empty filegroup to indicate that the tool is unavailable. Due to the possibility of this + being an empty filegroup and executable prerequisites validating that they contain at + least one artifact this attribute cannot be #exec(). */ + .add( + attr(HEADER_SCANNER_ATTRIBUTE, LABEL) + .cfg(HOST) + .value( + new LateBoundLabel( + "//tools/objc:header_scanner", ObjcConfiguration.class) { + @Override + public Label resolve( + Rule rule, AttributeMap attributes, BuildConfiguration configuration) { + return configuration + .getFragment(ObjcConfiguration.class) + .getObjcHeaderScannerTool(); + } + })) .build(); } @Override -- cgit v1.2.3