Move FDO support to the analysis phase by wrapping FdoSupport in its own SkyFunction.
This removes one of the two reasons for the existence of BuildConfiguration#prepareToBuild() which makes implementing dynamic configurations impossible and also makes FDO support halfway sane; now FDO is exactly as ugly as remote repositories, that is to say, reasonably okay.
Ideally, we'd implement the zip extraction as an Action and make it a TreeArtifact, but support for TreeArtifacts is not mature yet enough, so it's not possible at the moment.
--
MOS_MIGRATED_REVID=119150223
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 7a4f72a..98a57b8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -403,13 +403,6 @@
});
}
- private void prepareToBuild(BuildConfigurationCollection configurations)
- throws ViewCreationFailedException {
- for (BuildConfiguration config : configurations.getAllConfigurations()) {
- config.prepareToBuild(directories.getExecRoot());
- }
- }
-
@ThreadCompatible
public AnalysisResult update(
LoadingResult loadingResult,
@@ -484,7 +477,6 @@
}
}
- prepareToBuild(configurations);
skyframeExecutor.injectWorkspaceStatusData();
SkyframeAnalysisResult skyframeAnalysisResult;
try {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index e0ced04..33aa510 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -34,7 +34,6 @@
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.Dependency;
import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection.Transitions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
@@ -169,18 +168,6 @@
}
/**
- * The fragment may use this hook to perform I/O and read data into memory that is used during
- * analysis. During the analysis phase disk I/O operations are disallowed.
- *
- * <p>This hook is called for all configurations after the loading phase is complete.
- *
- * <p>Do not use this method to change your fragment's state.
- */
- @SuppressWarnings("unused")
- public void prepareHook(Path execPath) throws ViewCreationFailedException {
- }
-
- /**
* Adds all the roots from this fragment.
*/
@SuppressWarnings("unused")
@@ -2332,21 +2319,6 @@
}
/**
- * Prepare the fdo support. It reads data into memory that is used during analysis. The analysis
- * phase is generally not allowed to perform disk I/O. This code is here because it is
- * conceptually part of the analysis phase, and it needs to happen when the loading phase is
- * complete.
- *
- * <p>C++ also requires this to resolve artifacts that are unconditionally included in every
- * compilation.</p>
- */
- public void prepareToBuild(Path execRoot) throws ViewCreationFailedException {
- for (Fragment fragment : fragments.values()) {
- fragment.prepareHook(execRoot);
- }
- }
-
- /**
* Declares dependencies on any relevant Skyframe values (for example, relevant FileValues).
*/
public void declareSkyframeDependencies(SkyFunction.Environment env) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
index 2af2575..a0c142e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.actions.ActionContextProvider;
import com.google.devtools.build.lib.actions.Executor.ActionContext;
import com.google.devtools.build.lib.actions.SimpleActionContextProvider;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext;
import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses;
@@ -31,6 +32,8 @@
import com.google.devtools.build.lib.rules.android.WriteAdbArgsActionContext;
import com.google.devtools.build.lib.rules.cpp.CppCompileActionContext;
import com.google.devtools.build.lib.rules.cpp.CppLinkActionContext;
+import com.google.devtools.build.lib.rules.cpp.FdoSupportFunction;
+import com.google.devtools.build.lib.rules.cpp.FdoSupportValue;
import com.google.devtools.build.lib.rules.cpp.IncludeScanningContext;
import com.google.devtools.build.lib.rules.genquery.GenQuery;
import com.google.devtools.build.lib.runtime.BlazeModule;
@@ -39,6 +42,8 @@
import com.google.devtools.build.lib.runtime.GotOptionsEvent;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.util.ResourceFileLoader;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.common.options.Converters.AssignmentConverter;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionsBase;
@@ -194,4 +199,10 @@
}
}));
}
+
+ @Override
+ public ImmutableMap<SkyFunctionName, SkyFunction> getSkyFunctions(BlazeDirectories directories) {
+ return ImmutableMap.<SkyFunctionName, SkyFunction>of(
+ FdoSupportValue.SKYFUNCTION, new FdoSupportFunction());
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index 579bb53..b1639ea 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -178,7 +178,7 @@
}
public TransitiveLipoInfoProvider collectTransitiveLipoLabels(CcCompilationOutputs outputs) {
- if (cppConfiguration.getFdoSupport().getFdoRoot() == null
+ if (CppHelper.getFdoSupport(ruleContext).getFdoRoot() == null
|| !cppConfiguration.isLipoContextCollector()) {
return TransitiveLipoInfoProvider.EMPTY;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index 79d1c8d..1ec2c57 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -922,7 +922,7 @@
}
private TransitiveLipoInfoProvider collectTransitiveLipoInfo(CcCompilationOutputs outputs) {
- if (ruleContext.getFragment(CppConfiguration.class).getFdoSupport().getFdoRoot() == null) {
+ if (CppHelper.getFdoSupport(ruleContext).getFdoRoot() == null) {
return TransitiveLipoInfoProvider.EMPTY;
}
NestedSetBuilder<IncludeScannable> scannableBuilder = NestedSetBuilder.stableOrder();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
index 6cc3a5c..e3efe6c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
@@ -33,15 +33,21 @@
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
+import com.google.devtools.build.lib.analysis.config.CompilationMode;
import com.google.devtools.build.lib.cmdline.Label;
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.packages.License;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
+import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException;
import com.google.devtools.build.lib.util.Preconditions;
+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 java.io.IOException;
import java.util.List;
import java.util.Map;
@@ -57,6 +63,29 @@
@Override
public ConfiguredTarget create(RuleContext ruleContext) {
+ CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
+ Path fdoZip = ruleContext.getConfiguration().getCompilationMode() == CompilationMode.OPT
+ ? cppConfiguration.getFdoZip()
+ : null;
+ SkyKey fdoKey = FdoSupportValue.key(
+ cppConfiguration.getLipoMode(),
+ fdoZip,
+ cppConfiguration.getFdoInstrument());
+
+ SkyFunction.Environment skyframeEnv = ruleContext.getAnalysisEnvironment().getSkyframeEnv();
+ FdoSupportValue fdoSupport;
+ try {
+ fdoSupport = (FdoSupportValue) skyframeEnv.getValueOrThrow(
+ fdoKey, FdoException.class, IOException.class);
+ } catch (FdoException | IOException e) {
+ ruleContext.ruleError("cannot initialize FDO: " + e.getMessage());
+ return null;
+ }
+
+ if (skyframeEnv.valuesMissing()) {
+ return null;
+ }
+
final Label label = ruleContext.getLabel();
final NestedSet<Artifact> crosstool = ruleContext.getPrerequisite("all_files", Mode.HOST)
.getProvider(FileProvider.class).getFilesToBuild();
@@ -72,7 +101,6 @@
final PathFragment runtimeSolibDir = ruleContext.getConfiguration()
.getBinFragment().getRelative(runtimeSolibDirBase);
- CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
// Static runtime inputs.
TransitiveInfoCollection staticRuntimeLibDep = selectDep(ruleContext, "static_runtime_libs",
cppConfiguration.getStaticRuntimeLibsLabel());
@@ -173,6 +201,7 @@
RuleConfiguredTargetBuilder builder =
new RuleConfiguredTargetBuilder(ruleContext)
.add(CcToolchainProvider.class, provider)
+ .add(FdoSupportProvider.class, new FdoSupportProvider(fdoSupport.getFdoSupport()))
.setFilesToBuild(new NestedSetBuilder<Artifact>(Order.STABLE_ORDER).build())
.add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY));
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 f151dea..c1831ce 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
@@ -25,9 +25,7 @@
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
-import com.google.devtools.build.lib.actions.Root;
import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.CompilationMode;
@@ -40,7 +38,6 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.rules.cpp.CppConfigurationLoader.CppConfigurationParameters;
-import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.util.Preconditions;
@@ -51,7 +48,6 @@
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LinkingModeFlags;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode;
-import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.protobuf.TextFormat;
import com.google.protobuf.TextFormat.ParseException;
@@ -64,7 +60,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.zip.ZipException;
/**
* This class represents the C/C++ parts of the {@link BuildConfiguration},
@@ -284,7 +279,7 @@
private final boolean toolchainNeedsPic;
private final boolean usePicForBinaries;
- private final FdoSupport fdoSupport;
+ private final Path fdoZip;
// TODO(bazel-team): All these labels (except for ccCompilerRuleLabel) can be removed once the
// transition to the cc_compiler rule is complete.
@@ -390,10 +385,7 @@
}
}
- this.fdoSupport = new FdoSupport(
- cppOptions.fdoInstrument, params.fdoZip,
- cppOptions.lipoMode, execRoot);
-
+ this.fdoZip = params.fdoZip;
this.stripBinaries =
(cppOptions.stripBinaries == StripMode.ALWAYS
|| (cppOptions.stripBinaries == StripMode.SOMETIMES
@@ -1481,8 +1473,9 @@
* Returns true if it is AutoFDO LIPO build.
*/
public boolean isAutoFdoLipo() {
- return cppOptions.fdoOptimize != null && FdoSupport.isAutoFdo(cppOptions.fdoOptimize)
- && getLipoMode() != LipoMode.OFF;
+ return cppOptions.fdoOptimize != null
+ && CppFileTypes.GCC_AUTO_PROFILE.matches(cppOptions.fdoOptimize)
+ && getLipoMode() != LipoMode.OFF;
}
/**
@@ -1639,13 +1632,6 @@
}
/**
- * Returns the FDO support object.
- */
- public FdoSupport getFdoSupport() {
- return fdoSupport;
- }
-
- /**
* Return the name of the directory (relative to the bin directory) that
* holds mangled links to shared libraries. This name is always set to
* the '{@code _solib_<cpu_archictecture_name>}.
@@ -1892,35 +1878,10 @@
}
@Override
- public void prepareHook(Path execRoot) throws ViewCreationFailedException {
- try {
- getFdoSupport().prepareToBuild(execRoot);
- } catch (ZipException e) {
- throw new ViewCreationFailedException("Error reading provided FDO zip file", e);
- } catch (FdoException | IOException e) {
- throw new ViewCreationFailedException("Error while initializing FDO support", e);
- }
- }
-
- @Override
- public void declareSkyframeDependencies(Environment env) {
- getFdoSupport().declareSkyframeDependencies(env, execRoot);
- }
-
- @Override
- public void addRoots(List<Root> roots) {
- // Fdo root can only exist for the target configuration.
- FdoSupport fdoSupport = getFdoSupport();
- if (fdoSupport.getFdoRoot() != null) {
- roots.add(fdoSupport.getFdoRoot());
- }
- }
-
- @Override
public Map<String, String> getCoverageEnvironment() {
ImmutableMap.Builder<String, String> env = ImmutableMap.builder();
env.put("COVERAGE_GCOV_PATH", getGcovExecutable().getPathString());
- PathFragment fdoInstrument = getFdoSupport().getFdoInstrument();
+ PathFragment fdoInstrument = cppOptions.fdoInstrument;
if (fdoInstrument != null) {
env.put("FDO_DIR", fdoInstrument.getPathString());
}
@@ -1990,6 +1951,14 @@
);
}
+ public PathFragment getFdoInstrument() {
+ return cppOptions.fdoInstrument;
+ }
+
+ public Path getFdoZip() {
+ return fdoZip;
+ }
+
/**
* Return set of features enabled by the CppConfiguration, specifically
* the FDO and LIPO related features enabled by options.
@@ -1997,15 +1966,15 @@
@Override
public ImmutableSet<String> configurationEnabledFeatures(RuleContext ruleContext) {
ImmutableSet.Builder<String> requestedFeatures = ImmutableSet.builder();
- FdoSupport fdoSupport = getFdoSupport();
- if (fdoSupport.getFdoInstrument() != null) {
+ if (cppOptions.fdoInstrument != null) {
requestedFeatures.add(CppRuleClasses.FDO_INSTRUMENT);
}
- if (fdoSupport.getFdoOptimizeProfile() != null
- && !fdoSupport.isAutoFdoEnabled()) {
+
+ boolean isFdo = fdoZip != null && compilationMode == CompilationMode.OPT;
+ if (isFdo && !CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip)) {
requestedFeatures.add(CppRuleClasses.FDO_OPTIMIZE);
}
- if (fdoSupport.isAutoFdoEnabled()) {
+ if (isFdo && CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip)) {
requestedFeatures.add(CppRuleClasses.AUTOFDO);
}
if (isLipoOptimizationOrInstrumentation()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 97db3a5..300c87e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -214,6 +214,13 @@
return false;
}
+ @Nullable public static FdoSupport getFdoSupport(RuleContext ruleContext) {
+ return ruleContext
+ .getPrerequisite(":cc_toolchain", Mode.TARGET)
+ .getProvider(FdoSupportProvider.class)
+ .getFdoSupport();
+ }
+
/**
* This almost trivial method looks up the :cc_toolchain attribute on the rule context, makes sure
* that it refers to a rule that has a {@link CcToolchainProvider} (gives an error otherwise), and
@@ -508,8 +515,9 @@
/**
* Returns the FDO build subtype.
*/
- public static String getFdoBuildStamp(CppConfiguration cppConfiguration) {
- if (cppConfiguration.getFdoSupport().isAutoFdoEnabled()) {
+ public static String getFdoBuildStamp(RuleContext ruleContext) {
+ CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
+ if (getFdoSupport(ruleContext).isAutoFdoEnabled()) {
return (cppConfiguration.getLipoMode() == LipoMode.BINARY) ? "ALIPO" : "AFDO";
}
if (cppConfiguration.isFdo()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
index 95d6379..b0a1b7d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
@@ -1213,24 +1213,6 @@
this.runtimeSolibDir = runtimeSolibDir;
return this;
}
-
- /**
- * Creates a builder without the need for a {@link RuleContext}.
- * This is to be used exclusively for testing purposes.
- *
- * <p>Link stamping is not supported if using this method.
- */
- @VisibleForTesting
- public static Builder createTestBuilder(
- final ActionOwner owner, final AnalysisEnvironment analysisEnvironment,
- final Artifact output, BuildConfiguration config) {
- return new Builder(null, output, config, analysisEnvironment, null) {
- @Override
- protected ActionOwner getOwner() {
- return owner;
- }
- };
- }
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index c8a8c1a..bd8bbfa 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -287,7 +287,7 @@
}
builder.setExtraSystemIncludePrefixes(additionalIncludes);
- builder.setFdoBuildStamp(CppHelper.getFdoBuildStamp(cppConfiguration));
+ builder.setFdoBuildStamp(CppHelper.getFdoBuildStamp(ruleContext));
builder.setFeatureConfiguration(featureConfiguration);
return builder;
}
@@ -367,8 +367,8 @@
}
if (ccRelativeName != null) {
- cppConfiguration.getFdoSupport().configureCompilation(builder, buildVariables, ruleContext,
- ccRelativeName, autoFdoImportPath, usePic, featureConfiguration);
+ CppHelper.getFdoSupport(ruleContext).configureCompilation(builder, buildVariables,
+ ruleContext, ccRelativeName, autoFdoImportPath, usePic, featureConfiguration);
}
if (gcnoFile != null) {
buildVariables.addVariable("gcov_gcno_file", gcnoFile.getExecPathString());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java
index b182c8c..18f2cf1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java
@@ -25,7 +25,7 @@
import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadHostile;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.skyframe.FileValue;
@@ -47,20 +47,13 @@
* Support class for FDO (feedback directed optimization) and LIPO (lightweight inter-procedural
* optimization).
*
- * <p>There is a 1:1 relationship between {@link CppConfiguration} objects and {@code FdoSupport}
- * objects. The FDO support of a build configuration can be retrieved using {@link
- * CppConfiguration#getFdoSupport()}.
- *
- * <p>With respect to thread-safety, the {@link #prepareToBuild} method is not thread-safe, and must
- * not be called concurrently with other methods on this class.
- *
* <p>Here follows a quick run-down of how FDO/LIPO builds work (for non-FDO/LIPO builds, none
* of this applies):
*
- * <p>{@link CppConfiguration#prepareHook} is called before the analysis phase, which calls
- * {@link #prepareToBuild}, which extracts the FDO .zip (in case we work with an explicitly
- * generated FDO profile file) or analyzes the .afdo.imports file next to the .afdo file (if
- * AutoFDO is in effect).
+ * <p>{@link FdoSupport#create} is called from {@link FdoSupportFunction} (a {@link SkyFunction}),
+ * which is requested from Skyframe by the {@code cc_toolchain} rule. It extracts the FDO .zip (in
+ * case we work with an explicitly generated FDO profile file) or analyzes the .afdo.imports file
+ * next to the .afdo file (if AutoFDO is in effect).
*
* <p>.afdo.imports files contain one import a line. A line is two paths separated by a colon,
* with functions in the second path being referenced by functions in the first path. These are
@@ -108,9 +101,30 @@
* {@code CachingAnalysisEnvironment.allowRegisteringActions}, which causes actions to be silently
* discarded after configured targets are created.
*/
+@Immutable
public class FdoSupport {
/**
+ * The FDO mode we are operating in.
+ *
+ * LIPO can only be active if this is not <code>OFF</code>, but all of the modes below can work
+ * with LIPO either off or on.
+ */
+ private enum FdoMode {
+ /** FDO is turned off. */
+ OFF,
+
+ /** Profiling-based FDO using an explicitly recorded profile. */
+ VANILLA,
+
+ /** FDO based on automatically collected data. */
+ AUTO_FDO,
+
+ /** Instrumentation-based FDO implemented on LLVM. */
+ LLVM_FDO,
+ }
+
+ /**
* Path within profile data .zip files that is considered the root of the
* profile information directory tree.
*/
@@ -170,24 +184,15 @@
private final LipoMode lipoMode;
/**
- * Flag indicating whether to use AutoFDO (as opposed to
- * instrumentation-based FDO).
+ * FDO mode.
*/
- private final boolean useAutoFdo;
-
- /**
- * Flag indicating whether to use LLVM instrumentation-based FDO (as
- * opposed to GCC instrumentation-based FDO).
- */
- private final boolean useLLVMFdo;
+ private final FdoMode fdoMode;
/**
* The {@code .gcda} files that have been extracted from the ZIP file,
* relative to the root of the ZIP file.
- *
- * <p>Set only in {@link #prepareToBuild}.
*/
- private ImmutableSet<PathFragment> gcdaFiles = ImmutableSet.of();
+ private final ImmutableSet<PathFragment> gcdaFiles;
/**
* Multimap from .gcda file base names to auxiliary input files.
@@ -198,10 +203,8 @@
*
* <p>The contents of the multimap are copied verbatim from the .gcda.imports
* files and not yet checked for validity.
- *
- * <p>Set only in {@link #prepareToBuild}.
*/
- private ImmutableMultimap<PathFragment, PathFragment> imports;
+ private final ImmutableMultimap<PathFragment, PathFragment> imports;
/**
* Creates an FDO support object.
@@ -210,83 +213,122 @@
* @param fdoProfile path to the profile file passed to --fdo_optimize option
* @param lipoMode value of the --lipo_mode option
*/
- public FdoSupport(PathFragment fdoInstrument, Path fdoProfile, LipoMode lipoMode, Path execRoot) {
+ private FdoSupport(FdoMode fdoMode, LipoMode lipoMode, Root fdoRoot, PathFragment fdoRootExecPath,
+ PathFragment fdoInstrument, Path fdoProfile, FdoZipContents fdoZipContents) {
this.fdoInstrument = fdoInstrument;
this.fdoProfile = fdoProfile;
- this.fdoRoot = (fdoProfile == null)
- ? null
- : Root.asDerivedRoot(execRoot, execRoot.getRelative("blaze-fdo"));
- this.fdoRootExecPath = fdoProfile == null
- ? null
- : fdoRoot.getExecPath().getRelative(FileSystemUtils.removeExtension(
- new PathFragment("_fdo").getChild(fdoProfile.getBaseName())));
+ this.fdoRoot = fdoRoot;
+ this.fdoRootExecPath = fdoRootExecPath;
this.fdoPath = fdoProfile == null
? null
: FileSystemUtils.removeExtension(new PathFragment("_fdo").getChild(
fdoProfile.getBaseName()));
this.lipoMode = lipoMode;
- this.useAutoFdo = fdoProfile != null && isAutoFdo(fdoProfile.getBaseName());
- this.useLLVMFdo = fdoProfile != null && isLLVMFdo(fdoProfile.getBaseName());
+ this.fdoMode = fdoMode;
+ this.gcdaFiles = fdoZipContents.gcdaFiles;
+ this.imports = fdoZipContents.imports;
}
public Root getFdoRoot() {
return fdoRoot;
}
- public void declareSkyframeDependencies(SkyFunction.Environment env, Path execRoot) {
+ /**
+ * Creates an initialized {@link FdoSupport} instance.
+ */
+ static FdoSupport create(SkyFunction.Environment env, PathFragment fdoInstrument,
+ Path fdoProfile, LipoMode lipoMode, Path execRoot) throws IOException, FdoException {
+ FdoMode fdoMode;
+ if (fdoProfile != null && isAutoFdo(fdoProfile.getBaseName())) {
+ fdoMode = FdoMode.AUTO_FDO;
+ } else if (fdoProfile != null && isLLVMFdo(fdoProfile.getBaseName())) {
+ fdoMode = FdoMode.LLVM_FDO;
+ } else if (fdoProfile != null) {
+ fdoMode = FdoMode.VANILLA;
+ } else {
+ fdoMode = FdoMode.OFF;
+ }
+
+ if (fdoProfile == null) {
+ lipoMode = LipoMode.OFF;
+ }
+
+ Root fdoRoot = (fdoProfile == null)
+ ? null
+ : Root.asDerivedRoot(execRoot, execRoot.getRelative("blaze-fdo"));
+
+ PathFragment fdoRootExecPath = fdoProfile == null
+ ? null
+ : fdoRoot.getExecPath().getRelative(FileSystemUtils.removeExtension(
+ new PathFragment("_fdo").getChild(fdoProfile.getBaseName())));
+
if (fdoProfile != null) {
- if (isLipoEnabled()) {
+ if (lipoMode != LipoMode.OFF) {
// Incrementality is not supported for LIPO builds, see FdoSupport#scannables.
// Ensure that the Skyframe value containing the configuration will not be reused to avoid
// incrementality issues.
PrecomputedValue.dependOnBuildId(env);
- return;
- }
-
- // IMPORTANT: Keep the following in sync with #prepareToBuild.
- Path path;
- if (useAutoFdo) {
- path = fdoProfile.getParentDirectory().getRelative(
- fdoProfile.getBaseName() + ".imports");
} else {
- path = fdoProfile;
+ Path path = fdoMode == FdoMode.AUTO_FDO ? getAutoFdoImportsPath(fdoProfile) : fdoProfile;
+ env.getValue(FileValue.key(RootedPath.toRootedPathMaybeUnderRoot(path,
+ ImmutableList.of(execRoot))));
}
- env.getValue(FileValue.key(RootedPath.toRootedPathMaybeUnderRoot(path,
- ImmutableList.of(execRoot))));
+ }
+
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ FdoZipContents fdoZipContents = extractFdoZip(
+ fdoMode, lipoMode, execRoot, fdoProfile, fdoRootExecPath);
+ return new FdoSupport(
+ fdoMode, lipoMode, fdoRoot, fdoRootExecPath, fdoInstrument, fdoProfile, fdoZipContents);
+ }
+
+ @Immutable
+ private static class FdoZipContents {
+ private final ImmutableSet<PathFragment> gcdaFiles;
+ private final ImmutableMultimap<PathFragment, PathFragment> imports;
+
+ private FdoZipContents(ImmutableSet<PathFragment> gcdaFiles,
+ ImmutableMultimap<PathFragment, PathFragment> imports) {
+ this.gcdaFiles = gcdaFiles;
+ this.imports = imports;
}
}
/**
- * Prepares the FDO support for building.
+ * Extracts the FDO zip file and collects data from it that's needed during analysis.
*
* <p>When an {@code --fdo_optimize} compile is requested, unpacks the given
* FDO gcda zip file into a clean working directory under execRoot.
*
* @throws FdoException if the FDO ZIP contains a file of unknown type
*/
- @ThreadHostile // must be called before starting the build
- public void prepareToBuild(Path execRoot)
+ private static FdoZipContents extractFdoZip(FdoMode fdoMode, LipoMode lipoMode, Path execRoot,
+ Path fdoProfile, PathFragment fdoRootExecPath)
throws IOException, FdoException {
// The execRoot != null case is only there for testing. We cannot provide a real ZIP file in
// tests because ZipFileSystem does not work with a ZIP on an in-memory file system.
// IMPORTANT: Keep in sync with #declareSkyframeDependencies to avoid incrementality issues.
+ ImmutableSet<PathFragment> gcdaFiles = ImmutableSet.of();
+ ImmutableMultimap<PathFragment, PathFragment> imports = ImmutableMultimap.of();
+
if (fdoProfile != null && execRoot != null) {
Path fdoDirPath = execRoot.getRelative(fdoRootExecPath);
FileSystemUtils.deleteTreesBelow(fdoDirPath);
FileSystemUtils.createDirectoryAndParents(fdoDirPath);
- if (useAutoFdo) {
- Path fdoImports = fdoProfile.getParentDirectory().getRelative(
- fdoProfile.getBaseName() + ".imports");
- if (isLipoEnabled()) {
- imports = readAutoFdoImports(fdoImports);
+ if (fdoMode == FdoMode.AUTO_FDO) {
+ if (lipoMode != LipoMode.OFF) {
+ imports = readAutoFdoImports(getAutoFdoImportsPath(fdoProfile));
}
FileSystemUtils.ensureSymbolicLink(
- execRoot.getRelative(getAutoProfilePath()), fdoProfile);
- } else if (useLLVMFdo) {
+ execRoot.getRelative(getAutoProfilePath(fdoProfile, fdoRootExecPath)), fdoProfile);
+ } else if (fdoMode == FdoMode.LLVM_FDO) {
FileSystemUtils.ensureSymbolicLink(
- execRoot.getRelative(getLLVMProfilePath()), fdoProfile);
+ execRoot.getRelative(getLLVMProfilePath(fdoProfile, fdoRootExecPath)), fdoProfile);
} else {
Path zipFilePath = new ZipFileSystem(fdoProfile).getRootDirectory();
if (!zipFilePath.getRelative("blaze-out").isDirectory()) {
@@ -296,11 +338,13 @@
ImmutableSet.Builder<PathFragment> gcdaFilesBuilder = ImmutableSet.builder();
ImmutableMultimap.Builder<PathFragment, PathFragment> importsBuilder =
ImmutableMultimap.builder();
- extractFdoZip(zipFilePath, fdoDirPath, gcdaFilesBuilder, importsBuilder);
+ extractFdoZipDirectory(zipFilePath, fdoDirPath, gcdaFilesBuilder, importsBuilder);
gcdaFiles = gcdaFilesBuilder.build();
imports = importsBuilder.build();
}
}
+
+ return new FdoZipContents(gcdaFiles, imports);
}
/**
@@ -320,7 +364,7 @@
* @throws IOException if any of the I/O operations failed
* @throws FdoException if the FDO ZIP contains a file of unknown type
*/
- private void extractFdoZip(Path sourceDir, Path targetDir,
+ private static void extractFdoZipDirectory(Path sourceDir, Path targetDir,
ImmutableSet.Builder<PathFragment> gcdaFilesBuilder,
ImmutableMultimap.Builder<PathFragment, PathFragment> importsBuilder)
throws IOException, FdoException {
@@ -328,7 +372,7 @@
Path targetFile = targetDir.getRelative(sourceFile.getBaseName());
if (sourceFile.isDirectory()) {
targetFile.createDirectory();
- extractFdoZip(sourceFile, targetFile, gcdaFilesBuilder, importsBuilder);
+ extractFdoZipDirectory(sourceFile, targetFile, gcdaFilesBuilder, importsBuilder);
} else {
if (CppFileTypes.COVERAGE_DATA.matches(sourceFile)) {
FileSystemUtils.copyFile(sourceFile, targetFile);
@@ -343,12 +387,13 @@
}
}
+
/**
* Reads a .gcda.imports file and stores the imports information.
*
* @throws FdoException if an auxiliary LIPO input was not found
*/
- private void readCoverageImports(Path importsFile,
+ private static void readCoverageImports(Path importsFile,
ImmutableMultimap.Builder<PathFragment, PathFragment> importsBuilder)
throws IOException, FdoException {
PathFragment key = importsFile.asFragment().relativeTo(ZIP_ROOT);
@@ -373,7 +418,7 @@
/**
* Reads a .afdo.imports file and stores the imports information.
*/
- private ImmutableMultimap<PathFragment, PathFragment> readAutoFdoImports(Path importsFile)
+ private static ImmutableMultimap<PathFragment, PathFragment> readAutoFdoImports(Path importsFile)
throws IOException, FdoException {
ImmutableMultimap.Builder<PathFragment, PathFragment> importBuilder =
ImmutableMultimap.builder();
@@ -396,6 +441,10 @@
return importBuilder.build();
}
+ private static Path getAutoFdoImportsPath(Path fdoProfile) {
+ return fdoProfile.getParentDirectory().getRelative(fdoProfile.getBaseName() + ".imports");
+ }
+
/**
* Returns the imports from the .afdo.imports file of a source file.
*
@@ -403,7 +452,7 @@
*/
private Collection<Artifact> getAutoFdoImports(RuleContext ruleContext,
PathFragment sourceExecPath, LipoContextProvider lipoContextProvider) {
- Preconditions.checkState(isLipoEnabled());
+ Preconditions.checkState(lipoMode != LipoMode.OFF);
ImmutableCollection<PathFragment> afdoImports = imports.get(sourceExecPath);
Preconditions.checkState(afdoImports != null,
"AutoFDO import data missing for %s", sourceExecPath);
@@ -427,7 +476,7 @@
* @param objectName the object file
*/
private Iterable<PathFragment> getImports(PathFragment objDirectory, PathFragment objectName) {
- Preconditions.checkState(isLipoEnabled());
+ Preconditions.checkState(lipoMode != LipoMode.OFF);
Preconditions.checkState(imports != null,
"Tried to look up imports of uninitialized FDOSupport");
PathFragment key = objDirectory.getRelative(FileSystemUtils.removeExtension(objectName));
@@ -449,7 +498,7 @@
// It is a bug if this method is called with useLipo if lipo is disabled. However, it is legal
// if is is called with !useLipo, even though lipo is enabled.
LipoContextProvider lipoInputProvider = CppHelper.getLipoContextProvider(ruleContext);
- Preconditions.checkArgument(lipoInputProvider == null || isLipoEnabled());
+ Preconditions.checkArgument(lipoInputProvider == null || lipoMode != LipoMode.OFF);
// FDO is disabled -> do nothing.
if ((fdoInstrument == null) && (fdoRoot == null)) {
@@ -474,12 +523,12 @@
if (!Iterables.isEmpty(auxiliaryInputs)) {
if (featureConfiguration.isEnabled(CppRuleClasses.AUTOFDO)) {
buildVariables.addVariable("fdo_profile_path",
- getAutoProfilePath().getPathString());
+ getAutoProfilePath(fdoProfile, fdoRootExecPath).getPathString());
}
if (featureConfiguration.isEnabled(CppRuleClasses.FDO_OPTIMIZE)) {
- if (useLLVMFdo) {
+ if (fdoMode == FdoMode.LLVM_FDO) {
buildVariables.addVariable("fdo_profile_path",
- getLLVMProfilePath().getPathString());
+ getLLVMProfilePath(fdoProfile, fdoRootExecPath).getPathString());
} else {
buildVariables.addVariable("fdo_profile_path",
fdoRootExecPath.getPathString());
@@ -498,12 +547,12 @@
// If --fdo_optimize was not specified, we don't have any additional inputs.
if (fdoProfile == null) {
return ImmutableSet.of();
- } else if (useAutoFdo || useLLVMFdo) {
+ } else if (fdoMode == FdoMode.AUTO_FDO || fdoMode == FdoMode.LLVM_FDO) {
ImmutableSet.Builder<Artifact> auxiliaryInputs = ImmutableSet.builder();
- PathFragment profileRootRelativePath = (useLLVMFdo)
- ? getLLVMProfileRootRelativePath()
- : getAutoProfileRootRelativePath();
+ PathFragment profileRootRelativePath = fdoMode == FdoMode.LLVM_FDO
+ ? getLLVMProfileRootRelativePath(fdoProfile)
+ : getAutoProfileRootRelativePath(fdoProfile);
Artifact artifact = env.getDerivedArtifact(
fdoPath.getRelative(profileRootRelativePath), fdoRoot);
env.registerAction(new FdoStubAction(ruleContext.getActionOwner(), artifact));
@@ -607,37 +656,29 @@
}
- private PathFragment getAutoProfilePath() {
- return fdoRootExecPath.getRelative(getAutoProfileRootRelativePath());
+ private static PathFragment getAutoProfilePath(Path fdoProfile, PathFragment fdoRootExecPath) {
+ return fdoRootExecPath.getRelative(getAutoProfileRootRelativePath(fdoProfile));
}
- private PathFragment getAutoProfileRootRelativePath() {
+ private static PathFragment getAutoProfileRootRelativePath(Path fdoProfile) {
return new PathFragment(fdoProfile.getBaseName());
}
- private PathFragment getLLVMProfilePath() {
- return fdoRootExecPath.getRelative(getLLVMProfileRootRelativePath());
+ private static PathFragment getLLVMProfilePath(Path fdoProfile, PathFragment fdoRootExecPath) {
+ return fdoRootExecPath.getRelative(getLLVMProfileRootRelativePath(fdoProfile));
}
- private PathFragment getLLVMProfileRootRelativePath() {
+ private static PathFragment getLLVMProfileRootRelativePath(Path fdoProfile) {
return new PathFragment(fdoProfile.getBaseName());
}
/**
- * Returns whether LIPO is enabled.
- */
- @ThreadSafe
- public boolean isLipoEnabled() {
- return fdoProfile != null && lipoMode != LipoMode.OFF;
- }
-
- /**
* Returns whether AutoFDO is enabled.
*/
@ThreadSafe
public boolean isAutoFdoEnabled() {
- return useAutoFdo;
+ return fdoMode == FdoMode.AUTO_FDO;
}
/**
@@ -664,29 +705,6 @@
}
/**
- * Returns the path of the FDO zip containing the .gcda profile files, or null
- * if FDO is not enabled.
- */
- @VisibleForTesting
- public Path getFdoOptimizeProfile() {
- return fdoProfile;
- }
-
- /**
- * Returns the path fragment of the instrumentation output dir for gcc when
- * FDO is enabled, or null if FDO is not enabled.
- */
- @ThreadSafe
- public PathFragment getFdoInstrument() {
- return fdoInstrument;
- }
-
- @VisibleForTesting
- public void setGcdaFilesForTesting(ImmutableSet<PathFragment> gcdaFiles) {
- this.gcdaFiles = gcdaFiles;
- }
-
- /**
* An exception indicating an issue with FDO coverage files.
*/
public static final class FdoException extends Exception {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java
new file mode 100644
index 0000000..b232a31
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java
@@ -0,0 +1,76 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.rules.cpp;
+
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException;
+import com.google.devtools.build.lib.skyframe.PrecomputedValue;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionException;
+import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+
+import java.io.IOException;
+
+import javax.annotation.Nullable;
+
+/**
+ * Wrapper for {@link FdoSupport} that turns it into a {@link SkyFunction}.
+ */
+public class FdoSupportFunction implements SkyFunction {
+
+ /**
+ * Wrapper for FDO exceptions.
+ */
+ public class FdoSkyException extends SkyFunctionException {
+ public FdoSkyException(Exception cause, Transience transience) {
+ super(cause, transience);
+ }
+ }
+
+ @Nullable
+ @Override
+ public SkyValue compute(SkyKey skyKey, Environment env)
+ throws FdoSkyException, InterruptedException {
+ BlazeDirectories blazeDirectories = PrecomputedValue.BLAZE_DIRECTORIES.get(env);
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ Path execRoot = blazeDirectories.getExecRoot();
+ FdoSupportValue.Key key = (FdoSupportValue.Key) skyKey.argument();
+ FdoSupport fdoSupport;
+ try {
+ fdoSupport = FdoSupport.create(env,
+ key.getFdoInstrument(), key.getFdoZip(), key.getLipoMode(), execRoot);
+ if (env.valuesMissing()) {
+ return null;
+ }
+ } catch (FdoException e) {
+ throw new FdoSkyException(e, Transience.PERSISTENT);
+ } catch (IOException e) {
+ throw new FdoSkyException(e, Transience.TRANSIENT);
+ }
+
+ return new FdoSupportValue(fdoSupport);
+ }
+
+ @Nullable
+ @Override
+ public String extractTag(SkyKey skyKey) {
+ return null;
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportProvider.java
new file mode 100644
index 0000000..f976214
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportProvider.java
@@ -0,0 +1,34 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.rules.cpp;
+
+import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+
+/**
+ * A {@link TransitiveInfoProvider} so that {@code cc_toolchain} can pass {@link FdoSupport} to the
+ * C++ rules.
+ */
+@Immutable
+public class FdoSupportProvider implements TransitiveInfoProvider {
+ private final FdoSupport fdoSupport;
+
+ public FdoSupportProvider(FdoSupport fdoSupport) {
+ this.fdoSupport = fdoSupport;
+ }
+
+ public FdoSupport getFdoSupport() {
+ return fdoSupport;
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java
new file mode 100644
index 0000000..eb03db6
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java
@@ -0,0 +1,101 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.rules.cpp;
+
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode;
+import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+
+import java.util.Objects;
+
+/**
+ * Wrapper for {@link FdoSupport}.
+ *
+ * <p>Things could probably be refactored such the attributes of {@link FdoSupport} are moved here
+ * and the code in it to {@link FdoSupportFunction}. This would let us eliminate {@link FdoSupport}.
+ *
+ * <p>The eventual plan is to migrate FDO functionality to the execution phase once directory
+ * artifacts work better, so this may not be worth it.
+ */
+@Immutable
+public class FdoSupportValue implements SkyValue {
+ public static final SkyFunctionName SKYFUNCTION = SkyFunctionName.create("FDO_SUPPORT");
+
+ /**
+ * {@link SkyKey} for {@link FdoSupportValue}.
+ */
+ @Immutable
+ public static class Key {
+ private final LipoMode lipoMode;
+ private final Path fdoZip;
+ private final PathFragment fdoInstrument;
+
+ private Key(LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument) {
+ this.lipoMode = lipoMode;
+ this.fdoZip = fdoZip;
+ this.fdoInstrument = fdoInstrument;
+ }
+
+ public LipoMode getLipoMode() {
+ return lipoMode;
+ }
+
+ public Path getFdoZip() {
+ return fdoZip;
+ }
+
+ public PathFragment getFdoInstrument() {
+ return fdoInstrument;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+
+ if (!(o instanceof Key)) {
+ return false;
+ }
+
+ Key that = (Key) o;
+ return Objects.equals(this.lipoMode, that.lipoMode)
+ && Objects.equals(this.fdoZip, that.fdoZip)
+ && Objects.equals(this.fdoInstrument, that.fdoInstrument);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(lipoMode, fdoZip, fdoInstrument);
+ }
+ }
+
+ private final FdoSupport fdoSupport;
+
+ FdoSupportValue(FdoSupport fdoSupport) {
+ this.fdoSupport = fdoSupport;
+ }
+
+ public FdoSupport getFdoSupport() {
+ return fdoSupport;
+ }
+
+ public static SkyKey key(LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument) {
+ return SkyKey.create(SKYFUNCTION, new Key(lipoMode, fdoZip, fdoInstrument));
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index 98ed76f..79c24e6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -75,6 +75,7 @@
private final ImmutableSet<String> features;
private final ImmutableMap<Artifact, Artifact> linkstamps;
private final ImmutableList<String> linkstampCompileOptions;
+ @Nullable private final String fdoBuildStamp;
@Nullable private final PathFragment runtimeSolibDir;
private final boolean nativeDeps;
private final boolean useTestOnlyFlags;
@@ -84,6 +85,7 @@
@Nullable private final Artifact paramFile;
@Nullable private final Artifact interfaceSoBuilder;
+
/**
* A string constant for the c++ link action, used to access the feature
* configuration.
@@ -105,6 +107,7 @@
ImmutableSet<String> features,
ImmutableMap<Artifact, Artifact> linkstamps,
ImmutableList<String> linkstampCompileOptions,
+ @Nullable String fdoBuildStamp,
@Nullable PathFragment runtimeSolibDir,
boolean nativeDeps,
boolean useTestOnlyFlags,
@@ -161,6 +164,7 @@
this.features = Preconditions.checkNotNull(features);
this.linkstamps = Preconditions.checkNotNull(linkstamps);
this.linkstampCompileOptions = linkstampCompileOptions;
+ this.fdoBuildStamp = fdoBuildStamp;
this.runtimeSolibDir = runtimeSolibDir;
this.nativeDeps = nativeDeps;
this.useTestOnlyFlags = useTestOnlyFlags;
@@ -590,7 +594,6 @@
}
// Stamp FDO builds with FDO subtype string
- String fdoBuildStamp = CppHelper.getFdoBuildStamp(cppConfiguration);
if (fdoBuildStamp != null) {
optionList.add("-D" + CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoBuildStamp + "\"");
}
@@ -1052,8 +1055,7 @@
}
CcToolchainFeatures.Variables.Builder buildVariables =
new CcToolchainFeatures.Variables.Builder();
- CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
- cppConfiguration.getFdoSupport().getLinkOptions(featureConfiguration, buildVariables);
+ CppHelper.getFdoSupport(ruleContext).getLinkOptions(featureConfiguration, buildVariables);
variables = buildVariables.build();
}
return new LinkCommandLine(
@@ -1071,6 +1073,7 @@
features,
linkstamps,
actualLinkstampCompileOptions,
+ CppHelper.getFdoBuildStamp(ruleContext),
runtimeSolibDir,
nativeDeps,
useTestOnlyFlags,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
index 9aab8b1..0746b62 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
@@ -98,7 +98,7 @@
public static final Precomputed<Map<BuildInfoKey, BuildInfoFactory>> BUILD_INFO_FACTORIES =
new Precomputed<>(SkyKey.create(SkyFunctions.PRECOMPUTED, "build_info_factories"));
- static final Precomputed<BlazeDirectories> BLAZE_DIRECTORIES =
+ public static final Precomputed<BlazeDirectories> BLAZE_DIRECTORIES =
new Precomputed<>(SkyKey.create(SkyFunctions.PRECOMPUTED, "blaze_directories"));
static final Precomputed<ImmutableMap<Action, ConflictException>> BAD_ACTIONS =
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/ZipFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/ZipFileSystem.java
index 393110d..4830d8f 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/ZipFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/ZipFileSystem.java
@@ -15,11 +15,15 @@
import com.google.common.base.Predicate;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.util.Preconditions;
+import java.io.Closeable;
import java.io.File;
import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -31,9 +35,11 @@
* Inherits the constraints imposed by ReadonlyFileSystem.
*/
@ThreadSafe
-public class ZipFileSystem extends ReadonlyFileSystem {
+public class ZipFileSystem extends ReadonlyFileSystem implements Closeable {
+ private final File tempFile; // In case this needs to be written to the file system
private final ZipFile zipFile;
+ private boolean open;
/**
* The sole purpose of this field is to hold a strong reference to all leaf
@@ -53,14 +59,25 @@
* Constructs a ZipFileSystem from a zip file identified with a given path.
*/
public ZipFileSystem(Path zipPath) throws IOException {
- // Throw some more specific exceptions than ZipFile does.
- // We do this using File instead of Path, in case zipPath points to an
- // InMemoryFileSystem. This case is not really supported but
- // can occur in tests.
- File file = zipPath.getPathFile();
- if (!file.exists()) {
+ if (!zipPath.exists()) {
throw new FileNotFoundException(String.format("File '%s' does not exist", zipPath));
}
+
+ File file = zipPath.getPathFile();
+ if (!file.exists()) {
+ // If the File says that it does not exist but the Path says that it does, we are probably
+ // dealing with a FileSystem that does not represent the actual file system we are running
+ // under. Then we copy the Path into a temporary File.
+ tempFile = File.createTempFile("bazel.test.", ".tmp");
+ file = tempFile;
+ byte[] contents = FileSystemUtils.readContent(zipPath);
+ try (OutputStream os = new FileOutputStream(tempFile)) {
+ os.write(contents);
+ }
+ } else {
+ tempFile = null;
+ }
+
if (!file.isFile()) {
throw new IOException(String.format("'%s' is not a file", zipPath));
}
@@ -70,6 +87,7 @@
this.zipFile = new ZipFile(file);
this.paths = populatePathTree();
+ this.open = true;
}
// ZipPath extends Path with a set-once ZipEntry field.
@@ -160,12 +178,14 @@
@Override
protected InputStream getInputStream(Path path) throws IOException {
+ Preconditions.checkState(open);
return zipFile.getInputStream(zipEntryNonNull(path));
}
@Override
protected Collection<Path> getDirectoryEntries(Path path)
throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
final Collection<Path> result = new ArrayList<>();
((ZipPath) path).applyToChildren(new Predicate<Path>() {
@@ -182,46 +202,54 @@
@Override
protected boolean exists(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
return zipEntry(path) != null;
}
@Override
protected boolean isDirectory(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
ZipEntry entry = zipEntry(path);
return entry != null && entry.isDirectory();
}
@Override
protected boolean isFile(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
ZipEntry entry = zipEntry(path);
return entry != null && !entry.isDirectory();
}
@Override
protected boolean isSpecialFile(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
return false;
}
@Override
protected boolean isReadable(Path path) throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
return true;
}
@Override
protected boolean isWritable(Path path) throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
return false;
}
@Override
protected boolean isExecutable(Path path) throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
return false;
}
@Override
protected PathFragment readSymbolicLink(Path path) throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
throw new NotASymlinkException(path);
}
@@ -229,22 +257,26 @@
@Override
protected long getFileSize(Path path, boolean followSymlinks)
throws IOException {
+ Preconditions.checkState(open);
return zipEntryNonNull(path).getSize();
}
@Override
protected long getLastModifiedTime(Path path, boolean followSymlinks)
throws FileNotFoundException {
+ Preconditions.checkState(open);
return zipEntryNonNull(path).getTime();
}
@Override
protected boolean isSymbolicLink(Path path) {
+ Preconditions.checkState(open);
return false;
}
@Override
protected FileStatus statIfFound(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
try {
return stat(path, followSymlinks);
} catch (FileNotFoundException e) {
@@ -255,4 +287,14 @@
}
}
+ @Override
+ public void close() {
+ if (open) {
+ close();
+ if (tempFile != null) {
+ tempFile.delete();
+ }
+ open = false;
+ }
+ }
}
