diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules/cpp')
5 files changed, 90 insertions, 34 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java index ae66887989..9ecde15051 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java @@ -30,8 +30,10 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -210,7 +212,8 @@ public final class CppCompilationContext implements TransitiveInfoProvider { return usePic ? picModuleInfo.transitiveModules : moduleInfo.transitiveModules; } - public Collection<Artifact> getUsedModules(boolean usePic, Set<Artifact> usedHeaders) { + public Collection<TransitiveModuleHeaders> getUsedModules( + boolean usePic, Set<Artifact> usedHeaders) { return usePic ? picModuleInfo.getUsedModules(usedHeaders) : moduleInfo.getUsedModules(usedHeaders); @@ -654,7 +657,7 @@ public final class CppCompilationContext implements TransitiveInfoProvider { this.picModuleInfo.setHeaderModule(picHeaderModule); return this; } - + /** * Sets that the context will be used by a compilation that needs transitive module maps. */ @@ -662,7 +665,7 @@ public final class CppCompilationContext implements TransitiveInfoProvider { this.provideTransitiveModuleMaps = provideTransitiveModuleMaps; return this; } - + /** * Builds the {@link CppCompilationContext}. */ @@ -767,13 +770,13 @@ public final class CppCompilationContext implements TransitiveInfoProvider { * context. */ private final Artifact headerModule; - + /** All header files that are compiled into this module. */ private final ImmutableSet<Artifact> modularHeaders; /** All header files that are contained in this module. */ private final ImmutableSet<Artifact> textualHeaders; - + /** * All transitive modules that this context depends on, excluding headerModule. */ @@ -797,12 +800,13 @@ public final class CppCompilationContext implements TransitiveInfoProvider { this.transitiveModuleHeaders = transitiveModuleHeaders; } - public Collection<Artifact> getUsedModules(Set<Artifact> usedHeaders) { - Set<Artifact> result = new LinkedHashSet<>(); + public Collection<TransitiveModuleHeaders> getUsedModules(Set<Artifact> usedHeaders) { + List<TransitiveModuleHeaders> result = new ArrayList<>(); for (TransitiveModuleHeaders transitiveModule : transitiveModuleHeaders) { - if (result.contains(transitiveModule.module)) { - // If result already contains this module, we will have added it and all its - // transitive dependencies to result already. No need to check whether to add it again. + if (transitiveModule.module.equals(headerModule)) { + // Do not add the module of the current rule for both: + // 1. the module compile itself + // 2. compiles of other translation units of the same rule. continue; } boolean providesUsedHeader = false; @@ -813,15 +817,7 @@ public final class CppCompilationContext implements TransitiveInfoProvider { } } if (providesUsedHeader) { - result.addAll(transitiveModule.transitiveModules.toCollection()); - if (!transitiveModule.module.equals(headerModule)) { - // Only add the module itself if it is not the main headerModule. This is done because - // we don't want to pass the header module when compiling the translation unit that - // defines it. Due to how modules are implemented, #includes from a translation unit to - // the headers that it implements remain textual. Thus, adding this header module as a - // dependency would only prolong the build without any possible benefit. - result.add(transitiveModule.module); - } + result.add(transitiveModule); } } return result; @@ -834,8 +830,8 @@ public final class CppCompilationContext implements TransitiveInfoProvider { private Artifact headerModule = null; private final Set<Artifact> modularHeaders = new LinkedHashSet<>(); private final Set<Artifact> textualHeaders = new LinkedHashSet<>(); - private NestedSetBuilder<Artifact> transitiveModules = NestedSetBuilder.stableOrder(); - private NestedSetBuilder<TransitiveModuleHeaders> transitiveModuleHeaders = + private final NestedSetBuilder<Artifact> transitiveModules = NestedSetBuilder.stableOrder(); + private final NestedSetBuilder<TransitiveModuleHeaders> transitiveModuleHeaders = NestedSetBuilder.stableOrder(); public Builder setHeaderModule(Artifact headerModule) { @@ -927,5 +923,13 @@ public final class CppCompilationContext implements TransitiveInfoProvider { this.headers = headers; this.transitiveModules = transitiveModules; } + + public Artifact getModule() { + return module; + } + + public Collection<Artifact> getTransitiveModules() { + return transitiveModules.toCollection(); + } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index e8683fca58..0ef064b77a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -53,6 +53,8 @@ import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CppCompileActionContext.Reply; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool; +import com.google.devtools.build.lib.skyframe.ActionLookupValue; +import com.google.devtools.build.lib.skyframe.ActionLookupValue.ActionLookupKey; import com.google.devtools.build.lib.util.DependencySet; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.Fingerprint; @@ -63,6 +65,9 @@ import com.google.devtools.build.lib.util.ShellEscaper; 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 com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; @@ -207,6 +212,9 @@ public class CppCompileAction extends AbstractAction */ private Collection<Artifact> additionalInputs = null; + /** Set when a two-stage input discovery is used. */ + private Collection<Artifact> usedModules = null; + private CcToolchainFeatures.Variables overwrittenVariables = null; private ImmutableList<Artifact> resolvedInputs = ImmutableList.<Artifact>of(); @@ -504,8 +512,18 @@ public class CppCompileAction extends AbstractAction if (shouldPruneModules) { Set<Artifact> initialResultSet = Sets.newLinkedHashSet(initialResult); - Collection<Artifact> usedModules = context.getUsedModules(usePic, initialResultSet); + Set<Artifact> usedModules = Sets.newLinkedHashSet(); + for (CppCompilationContext.TransitiveModuleHeaders usedModule : + context.getUsedModules(usePic, initialResultSet)) { + usedModules.add(usedModule.getModule()); + if (!cppConfiguration.getPruneMoreModules()) { + usedModules.addAll(usedModule.getTransitiveModules()); + } + } initialResultSet.addAll(usedModules); + if (cppConfiguration.getPruneMoreModules()) { + this.usedModules = usedModules; + } initialResult = initialResultSet; } @@ -533,6 +551,39 @@ public class CppCompileAction extends AbstractAction } @Override + public Iterable<Artifact> discoverInputsStage2(SkyFunction.Environment env) + throws ActionExecutionException, InterruptedException { + if (this.usedModules == null) { + return null; + } + Map<Artifact, SkyKey> skyKeys = new HashMap<>(); + for (Artifact artifact : this.usedModules) { + skyKeys.put(artifact, ActionLookupValue.key((ActionLookupKey) artifact.getArtifactOwner())); + } + Map<SkyKey, SkyValue> skyValues = env.getValues(skyKeys.values()); + Set<Artifact> additionalModules = Sets.newLinkedHashSet(); + for (Artifact artifact : this.usedModules) { + SkyKey skyKey = skyKeys.get(artifact); + ActionLookupValue value = (ActionLookupValue) skyValues.get(skyKey); + Preconditions.checkNotNull( + value, "Owner %s of %s not in graph %s", artifact.getArtifactOwner(), artifact, skyKey); + CppCompileAction action = (CppCompileAction) value.getGeneratingAction(artifact); + for (Artifact input : action.getInputs()) { + if (CppFileTypes.CPP_MODULE.matches(input.getFilename())) { + additionalModules.add(input); + } + } + } + this.additionalInputs = + new ImmutableList.Builder<Artifact>() + .addAll(this.additionalInputs) + .addAll(additionalModules) + .build(); + this.usedModules = null; + return additionalModules; + } + + @Override public Iterable<Artifact> getInputsWhenSkippingInputDiscovery() { if (useHeaderModules && cppConfiguration.getSkipUnusedModules()) { this.additionalInputs = context.getTransitiveModules(usePic).toCollection(); @@ -1169,7 +1220,6 @@ public class CppCompileAction extends AbstractAction public void execute( ActionExecutionContext actionExecutionContext) throws ActionExecutionException, InterruptedException { - if (useHeaderModules) { // If modules pruning is used, modules will be supplied via additionalInputs, otherwise they // are regular inputs. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index d1ee7f3967..9a3780d07c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -263,17 +263,7 @@ public class CppCompileActionBuilder { if (!fake && !shouldScanIncludes) { realMandatoryInputsBuilder.addTransitive(context.getDeclaredIncludeSrcs()); } - // We disable pruning header modules in CPP_MODULE_COMPILEs as that would lead to - // module-out-of-date errors. The problem surfaces if a module A depends on a module B, but the - // headers of module A don't actually use any of B's headers. Then we would not need to rebuild - // A when B changes although we are storing a dependency in it. If B changes and we then build - // something that uses A (a header of it), we mark A and all of its transitive deps as inputs. - // We still don't need to rebuild A, as none of its inputs have changed, but we do rebuild B - // now and then the two modules are out of sync. - boolean shouldPruneModules = - shouldScanIncludes - && useHeaderModules - && !getActionName().equals(CppCompileAction.CPP_MODULE_COMPILE); + boolean shouldPruneModules = shouldScanIncludes && useHeaderModules; if (useHeaderModules && !shouldPruneModules) { realMandatoryInputsBuilder.addTransitive(context.getTransitiveModules(usePic)); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index 91ef450c2c..f0b6e4d61b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -1777,6 +1777,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return cppOptions.skipUnusedModules; } + public boolean getPruneMoreModules() { + return cppOptions.pruneMoreModules; + } + public LibcTop getLibcTop() { return cppOptions.libcTop; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java index 0a1f16b089..0af42a5aee 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java @@ -546,6 +546,14 @@ public class CppOptions extends FragmentOptions { public boolean skipUnusedModules; @Option( + name = "experimental_prune_more_modules", + defaultValue = "false", + category = "experimental", + help = "If enabled, modules pruning is used when building modules themselves." + ) + public boolean pruneMoreModules; + + @Option( name = "experimental_omitfp", defaultValue = "false", category = "semantics", |