Skip to content

Commit

Permalink
Wire up include scanning.
Browse files Browse the repository at this point in the history
Enable Bazel to use C/C++ include scanning if the --experimental_include_scanning flag is given. Include scanning can improve performance and incrementality by decreasing the size of compilation input trees. This change gives the community an opportunity to try it out. Include scanning must not be enabled by default because limitations of the builtin include parser can break builds.

Closes #13871.

PiperOrigin-RevId: 414955626
  • Loading branch information
benjaminp authored and Copybara-Service committed Dec 8, 2021
1 parent fc4d9d6 commit 6522472
Show file tree
Hide file tree
Showing 17 changed files with 213 additions and 72 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/BUILD
Expand Up @@ -139,6 +139,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions",
"//src/main/java/com/google/devtools/build/lib/buildeventservice",
"//src/main/java/com/google/devtools/build/lib/dynamic",
"//src/main/java/com/google/devtools/build/lib/includescanning",
"//src/main/java/com/google/devtools/build/lib/metrics:memory-use-recorder",
"//src/main/java/com/google/devtools/build/lib/metrics:metrics_module",
"//src/main/java/com/google/devtools/build/lib/network:noop_connectivity",
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
Expand Up @@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryDebugModule;
import com.google.devtools.build.lib.includescanning.IncludeScanningModule;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.BlazeRuntime;
import java.io.IOException;
Expand Down Expand Up @@ -77,7 +78,8 @@ public final class Bazel {
com.google.devtools.build.lib.metrics.PostGCMemoryUseRecorder.GcAfterBuildModule.class,
com.google.devtools.build.lib.packages.metrics.PackageMetricsModule.class,
com.google.devtools.build.lib.metrics.MetricsModule.class,
BazelBuiltinCommandModule.class);
BazelBuiltinCommandModule.class,
IncludeScanningModule.class);

public static void main(String[] args) {
BlazeVersionInfo.setBuildInfo(tryGetBuildInfo());
Expand Down
Expand Up @@ -80,7 +80,7 @@ public void init(ConfiguredRuleClassProvider.Builder builder) {
builder.addRuleDefinition(new BazelCppRuleClasses.CcLibraryBaseRule());
builder.addRuleDefinition(new BazelCcLibraryRule());
builder.addRuleDefinition(new BazelCcImportRule());
builder.addRuleDefinition(new CcIncludeScanningRule());
builder.addRuleDefinition(new CcIncludeScanningRule(/* addGrepIncludes= */ false));
builder.addRuleDefinition(new FdoProfileRule());
builder.addRuleDefinition(new FdoPrefetchHintsRule());
builder.addRuleDefinition(new CcLinkingRule());
Expand Down
Expand Up @@ -42,6 +42,7 @@ java_library(
name = "bazel_cpp_semantics",
srcs = ["BazelCppSemantics.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection",
Expand Down
Expand Up @@ -15,6 +15,7 @@
package com.google.devtools.build.lib.bazel.rules.cpp;

import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.RuleErrorConsumer;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
Expand All @@ -31,6 +32,7 @@
import com.google.devtools.build.lib.rules.cpp.CppCompileActionBuilder;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;

/** C++ compilation semantics. */
Expand Down Expand Up @@ -78,15 +80,20 @@ public void finalizeCompileActionBuilder(
RuleErrorConsumer ruleErrorConsumer) {
CcToolchainProvider toolchain = actionBuilder.getToolchain();
if (language == Language.CPP) {
CppConfiguration cppConfig = configuration.getFragment(CppConfiguration.class);
Artifact sourceFile = actionBuilder.getSourceFile();
actionBuilder
.addTransitiveMandatoryInputs(
configuration.getFragment(CppConfiguration.class).useSpecificToolFiles()
&& !actionBuilder.getSourceFile().isTreeArtifact()
cppConfig.useSpecificToolFiles() && !actionBuilder.getSourceFile().isTreeArtifact()
? (actionBuilder.getActionName().equals(CppActionNames.ASSEMBLE)
? toolchain.getAsFiles()
: toolchain.getCompilerFiles())
: toolchain.getAllFiles())
.setShouldScanIncludes(false);
.setShouldScanIncludes(
cppConfig.experimentalIncludeScanning()
&& featureConfiguration.getRequestedFeatures().contains("cc_include_scanning")
&& !sourceFile.isFileType(CppFileTypes.ASSEMBLER)
&& !sourceFile.isFileType(CppFileTypes.CPP_MODULE));
} else {
actionBuilder
.addTransitiveMandatoryInputs(toolchain.getAllFilesIncludingLibc())
Expand All @@ -110,7 +117,7 @@ public HeadersCheckingMode determineStarlarkHeadersCheckingMode(

@Override
public boolean allowIncludeScanning() {
return false;
return true;
}

@Override
Expand Down
Expand Up @@ -138,7 +138,8 @@ public String toString() {
}

/** {@link SkyValue} encapsulating the source-state-dependent part of {@link Hints}. */
public static class HintsRules implements SkyValue {
public static final class HintsRules implements SkyValue {
static final HintsRules EMPTY = new HintsRules(ImmutableList.of());
private final ImmutableList<Rule> rules;

private HintsRules(ImmutableList<Rule> rules) {
Expand Down
Expand Up @@ -33,12 +33,14 @@
import com.google.devtools.build.lib.buildtool.BuildRequest;
import com.google.devtools.build.lib.concurrent.ExecutorUtil;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadHostile;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.includescanning.IncludeParser.Inclusion;
import com.google.devtools.build.lib.rules.cpp.CppIncludeExtractionContext;
import com.google.devtools.build.lib.rules.cpp.CppIncludeScanningContext;
import com.google.devtools.build.lib.rules.cpp.CppOptions;
import com.google.devtools.build.lib.rules.cpp.IncludeScanner.IncludeScanningHeaderData;
import com.google.devtools.build.lib.rules.cpp.SwigIncludeScanningContext;
import com.google.devtools.build.lib.runtime.BlazeModule;
Expand Down Expand Up @@ -66,6 +68,7 @@
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
import javax.annotation.Nullable;

/**
* Module that provides implementations of {@link CppIncludeExtractionContext},
Expand All @@ -74,16 +77,14 @@
public class IncludeScanningModule extends BlazeModule {
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();

private static final PathFragment INCLUDE_HINTS_FILENAME =
PathFragment.create("tools/cpp/INCLUDE_HINTS");

private final MutableSupplier<SpawnIncludeScanner> spawnIncludeScannerSupplier =
new MutableSupplier<>();
private final MutableSupplier<ArtifactFactory> artifactFactory = new MutableSupplier<>();
private IncludeScannerLifecycleManager lifecycleManager;

@Nullable
protected PathFragment getIncludeHintsFilename() {
return INCLUDE_HINTS_FILENAME;
return null;
}

@Override
Expand All @@ -106,7 +107,8 @@ public void registerActionContexts(
@ThreadHostile
public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
lifecycleManager =
new IncludeScannerLifecycleManager(env, request, spawnIncludeScannerSupplier);
new IncludeScannerLifecycleManager(
env, request, spawnIncludeScannerSupplier, getIncludeHintsFilename() != null);
builder.addExecutorLifecycleListener(lifecycleManager);
}

Expand All @@ -119,6 +121,11 @@ public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command)

@Override
public void beforeCommand(CommandEnvironment env) {
CppOptions cppOptions = env.getOptions().getOptions(CppOptions.class);
if (cppOptions != null && cppOptions.experimentalIncludeScanning) {
env.getReporter()
.handle(Event.warn("Include scanning enabled. This feature is unsupported."));
}
artifactFactory.set(env.getSkyframeBuildView().getArtifactFactory());
}

Expand All @@ -137,10 +144,13 @@ public void workspaceInit(

@VisibleForTesting
public static ImmutableMap<SkyFunctionName, SkyFunction> getSkyFunctions(
PathFragment includeHintsFile) {
return ImmutableMap.of(
IncludeScanningSkyFunctions.INCLUDE_HINTS,
new IncludeHintsFunction(includeHintsFile));
@Nullable PathFragment includeHintsFile) {
ImmutableMap.Builder<SkyFunctionName, SkyFunction> skyFunctions = ImmutableMap.builder();
if (includeHintsFile != null) {
skyFunctions.put(
IncludeScanningSkyFunctions.INCLUDE_HINTS, new IncludeHintsFunction(includeHintsFile));
}
return skyFunctions.buildOrThrow();
}

/**
Expand Down Expand Up @@ -225,6 +235,7 @@ public void extractSwigIncludes(
private static final class IncludeScannerLifecycleManager implements ExecutorLifecycleListener {
private final CommandEnvironment env;
private final IncludeScanningOptions options;
private final boolean useIncludeHints;

private final Supplier<SpawnIncludeScanner> spawnScannerSupplier;
private IncludeScannerSupplier includeScannerSupplier;
Expand All @@ -233,9 +244,11 @@ private static final class IncludeScannerLifecycleManager implements ExecutorLif
IncludeScannerLifecycleManager(
CommandEnvironment env,
BuildRequest buildRequest,
MutableSupplier<SpawnIncludeScanner> spawnScannerSupplier) {
MutableSupplier<SpawnIncludeScanner> spawnScannerSupplier,
boolean useIncludeHints) {
this.env = env;
this.options = buildRequest.getOptions(IncludeScanningOptions.class);
this.useIncludeHints = useIncludeHints;

spawnScannerSupplier.set(
new SpawnIncludeScanner(
Expand All @@ -258,26 +271,32 @@ private SwigIncludeScanningContextImpl getSwigActionContext() {
public void executionPhaseStarting(
ActionGraph actionGraph, Supplier<ImmutableSet<Artifact>> topLevelArtifacts)
throws AbruptExitException, InterruptedException {
try {
includeScannerSupplier.init(
new IncludeParser(
new IncludeParser.Hints(
(IncludeParser.HintsRules)
env.getSkyframeExecutor()
.evaluateSkyKeyForExecutionSetup(
env.getReporter(), IncludeHintsFunction.INCLUDE_HINTS_KEY),
env.getSkyframeBuildView().getArtifactFactory())));
} catch (ExecException e) {
throw new AbruptExitException(
DetailedExitCode.of(
FailureDetail.newBuilder()
.setMessage("could not initialize include hints: " + e.getMessage())
.setIncludeScanning(
IncludeScanning.newBuilder()
.setCode(IncludeScanning.Code.INITIALIZE_INCLUDE_HINTS_ERROR))
.build()),
e);
IncludeParser.HintsRules hintsRules;
if (useIncludeHints) {
try {
hintsRules =
(IncludeParser.HintsRules)
env.getSkyframeExecutor()
.evaluateSkyKeyForExecutionSetup(
env.getReporter(), IncludeHintsFunction.INCLUDE_HINTS_KEY);
} catch (ExecException e) {
throw new AbruptExitException(
DetailedExitCode.of(
FailureDetail.newBuilder()
.setMessage("could not initialize include hints: " + e.getMessage())
.setIncludeScanning(
IncludeScanning.newBuilder()
.setCode(IncludeScanning.Code.INITIALIZE_INCLUDE_HINTS_ERROR))
.build()),
e);
}
} else {
hintsRules = IncludeParser.HintsRules.EMPTY;
}
includeScannerSupplier.init(
new IncludeParser(
new IncludeParser.Hints(
hintsRules, env.getSkyframeBuildView().getArtifactFactory())));
}

@Override
Expand Down
Expand Up @@ -537,19 +537,13 @@ private Artifact locateRelative(
}
ArtifactRoot root = includer.getRoot();
Artifact sourceArtifact =
artifactFactory.resolveSourceArtifactWithAncestor(
name, parentDirectory, root, RepositoryName.MAIN);
artifactFactory.resolveSourceArtifactWithAncestor(name, parent, root, RepositoryName.MAIN);
if (sourceArtifact == null) {
// If the name had up-level references, this path may not be under any package. Otherwise,
// we must have gotten an artifact, since it should be under the same package as the
// including artifact.
Preconditions.checkState(
name.containsUplevelReferences(),
"%s %s %s %s",
name,
parentDirectory,
rootRelativePath,
root);
name.containsUplevelReferences(), "%s %s %s %s", name, parent, rootRelativePath, root);
}
return sourceArtifact;
}
Expand Down
Expand Up @@ -837,6 +837,10 @@ public boolean generateLlvmLCov() {
return cppOptions.generateLlvmLcov;
}

public boolean experimentalIncludeScanning() {
return cppOptions.experimentalIncludeScanning;
}

public boolean objcShouldScanIncludes() {
return cppOptions.objcScanIncludes;
}
Expand Down
Expand Up @@ -1005,6 +1005,25 @@ public Label getPropellerOptimizeLabel() {
help = "If enabled, write CppCompileAction exposed action.args to parameters file.")
public boolean useArgsParamsFile;

@Option(
name = "experimental_unsupported_and_brittle_include_scanning",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
effectTags = {
OptionEffectTag.LOADING_AND_ANALYSIS,
OptionEffectTag.EXECUTION,
OptionEffectTag.CHANGES_INPUTS
},
help =
"Whether to narrow inputs to C/C++ compilation by parsing #include lines from input"
+ " files. This can improve performance and incrementality by decreasing the size of"
+ " compilation input trees. However, it can also break builds because the include"
+ " scanner does not fully implement C preprocessor semantics. In particular, it does"
+ " not understand dynamic #include directives and ignores preprocessor conditional"
+ " logic. Use at your own risk. Any issues relating to this flag that are filed will"
+ " be closed.")
public boolean experimentalIncludeScanning;

@Option(
name = "experimental_objc_include_scanning",
defaultValue = "false",
Expand Down Expand Up @@ -1175,6 +1194,7 @@ public FragmentOptions getHost() {
host.parseHeadersSkippedIfCorrespondingSrcsFound = parseHeadersSkippedIfCorrespondingSrcsFound;
host.strictSystemIncludes = strictSystemIncludes;
host.useArgsParamsFile = useArgsParamsFile;
host.experimentalIncludeScanning = experimentalIncludeScanning;

// Save host options for further use.
host.hostCoptList = hostCoptList;
Expand Down
Expand Up @@ -459,14 +459,25 @@ public static Label ccToolchainTypeAttribute(RuleDefinitionEnvironment env) {

/** Ancestor for all rules that do include scanning. */
public static final class CcIncludeScanningRule implements RuleDefinition {
private final boolean addGrepIncludes;

public CcIncludeScanningRule(boolean addGrepIncludes) {
this.addGrepIncludes = addGrepIncludes;
}

public CcIncludeScanningRule() {
this(true);
}

@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
return builder
.add(
attr("$grep_includes", LABEL)
.cfg(ExecutionTransitionFactory.create())
.value(env.getToolsLabel("//tools/cpp:grep-includes")))
.build();
if (addGrepIncludes) {
builder.add(
attr("$grep_includes", LABEL)
.cfg(ExecutionTransitionFactory.create())
.value(env.getToolsLabel("//tools/cpp:grep-includes")));
}
return builder.build();
}

@Override
Expand Down

0 comments on commit 6522472

Please sign in to comment.