Skip to content

Commit

Permalink
Rename FdoSupportFunction to CcSkyframeSupportValue
Browse files Browse the repository at this point in the history 
This is a preparation for a cl removing package loading from CppConfiguration.

#6072.

RELNOTES: None.
PiperOrigin-RevId: 211453414
  • Loading branch information
@hlopko
hlopko authored and Copybara-Service committed Sep 4, 2018
1 parent ee77832 commit 411ab49
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 47 deletions.
6 changes: 3 additions & 3 deletions src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses;
import com.google.devtools.build.lib.bazel.rules.sh.BazelShRuleClasses;
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.CcSkyframeSupportValue;
import com.google.devtools.build.lib.rules.cpp.CcSupportFunction;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.BlazeRuntime;
import com.google.devtools.build.lib.runtime.Command;
Expand Down Expand Up @@ -99,7 +99,7 @@ public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) {
@Override
public void workspaceInit(
BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) {
builder.addSkyFunction(FdoSupportValue.SKYFUNCTION, new FdoSupportFunction(directories));
builder.addSkyFunction(CcSkyframeSupportValue.SKYFUNCTION, new CcSupportFunction(directories));
}

@Override
Expand Down
45 changes: 22 additions & 23 deletions .../build/lib/rules/cpp/FdoSupportValue.java → ...lib/rules/cpp/CcSkyframeSupportValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,36 +28,37 @@
/**
* A container for the path to the FDO profile.
*
* <p>{@link FdoSupportValue} is created from {@link FdoSupportFunction} (a {@link SkyFunction}),
* which is requested from Skyframe by the {@code cc_toolchain} rule. It's done this way because
* the path depends on both a command line argument and the location of the workspace and the latter
* is not available either during configuration creation or during the analysis phase.
* <p>{@link CcSkyframeSupportValue} is created from {@link CcSupportFunction} (a {@link
* SkyFunction}), which is requested from Skyframe by the {@code cc_toolchain}/{@code
* cc_toolchain_suite} rule. It's done this way because the path depends on both a command line
* argument and the location of the workspace and the latter is not available either during
* configuration creation or during the analysis phase.
*/
@AutoCodec
@Immutable
public class FdoSupportValue implements SkyValue {
public class CcSkyframeSupportValue implements SkyValue {
public static final SkyFunctionName SKYFUNCTION = SkyFunctionName.createHermetic("FDO_SUPPORT");

/** {@link SkyKey} for {@link FdoSupportValue}. */
/** {@link SkyKey} for {@link CcSkyframeSupportValue}. */
@Immutable
@AutoCodec
public static class Key implements SkyKey {
private static final Interner<Key> interner = BlazeInterners.newWeakInterner();

private final PathFragment fdoProfileArgument;
private final PathFragment filePath;

private Key(PathFragment fdoProfileArgument) {
this.fdoProfileArgument = fdoProfileArgument;
private Key(PathFragment filePath) {
this.filePath = filePath;
}

@AutoCodec.Instantiator
@AutoCodec.VisibleForSerialization
static Key of(PathFragment fdoProfileArgument) {
return interner.intern(new Key(fdoProfileArgument));
static Key of(PathFragment filePath) {
return interner.intern(new Key(filePath));
}

public PathFragment getFdoProfileArgument() {
return fdoProfileArgument;
public PathFragment getFilePath() {
return filePath;
}

@Override
Expand All @@ -71,12 +72,12 @@ public boolean equals(Object o) {
}

Key that = (Key) o;
return Objects.equals(this.fdoProfileArgument, that.fdoProfileArgument);
return Objects.equals(this.filePath, that.filePath);
}

@Override
public int hashCode() {
return Objects.hash(fdoProfileArgument);
return Objects.hash(filePath);
}

@Override
Expand All @@ -85,20 +86,18 @@ public SkyFunctionName functionName() {
}
}

/**
* Path of the profile file passed to {@code --fdo_optimize}
*/
/** Path of the profile file passed to {@code --fdo_optimize} */
// TODO(lberki): This should be a PathFragment.
// Except that CcProtoProfileProvider#getProfile() calls #exists() on it, which is ridiculously
// incorrect.
private final Path fdoProfile;
private final Path filePath;

FdoSupportValue(Path fdoProfile) {
this.fdoProfile = fdoProfile;
CcSkyframeSupportValue(Path filePath) {
this.filePath = filePath;
}

public Path getFdoProfile() {
return fdoProfile;
public Path getFilePath() {
return filePath;
}

public static SkyKey key(PathFragment fdoProfileArgument) {
Expand Down
22 changes: 12 additions & 10 deletions ...ild/lib/rules/cpp/FdoSupportFunction.java → ...uild/lib/rules/cpp/CcSupportFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,30 @@
* and thus we need to depend on {@link BlazeDirectories} somehow, which neither the configuration
* nor the analysis phase can "officially" do.
*
* <p>The fix is probably to make it possible for
* {@link com.google.devtools.build.lib.analysis.actions.SymlinkAction} to create workspace-relative
* <p>The fix is probably to make it possible for {@link
* com.google.devtools.build.lib.analysis.actions.SymlinkAction} to create workspace-relative
* symlinks because action execution can hopefully depend on {@link BlazeDirectories}.
*
* <p>There is also the awful and incorrect {@link Path#exists()} call in
* {@link com.google.devtools.build.lib.view.proto.CcProtoProfileProvider#getProfile(
* <p>There is also the awful and incorrect {@link Path#exists()} call in {@link
* com.google.devtools.build.lib.view.proto.CcProtoProfileProvider#getProfile(
* com.google.devtools.build.lib.analysis.RuleContext)} which needs a {@link Path}.
*/
public class FdoSupportFunction implements SkyFunction {
public class CcSupportFunction implements SkyFunction {
private final BlazeDirectories directories;

public FdoSupportFunction(BlazeDirectories directories) {
public CcSupportFunction(BlazeDirectories directories) {
this.directories = Preconditions.checkNotNull(directories);
}

@Nullable
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
FdoSupportValue.Key key = (FdoSupportValue.Key) skyKey.argument();
Path resolvedFdoProfile = key.getFdoProfileArgument() == null
? null : directories.getWorkspace().getRelative(key.getFdoProfileArgument());
return new FdoSupportValue(resolvedFdoProfile);
CcSkyframeSupportValue.Key key = (CcSkyframeSupportValue.Key) skyKey.argument();
Path resolvedPath =
key.getFilePath() == null
? null
: directories.getWorkspace().getRelative(key.getFilePath());
return new CcSkyframeSupportValue(resolvedPath);
}

@Nullable
Expand Down
18 changes: 11 additions & 7 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,10 +388,10 @@ public ConfiguredTarget create(RuleContext ruleContext)
return null;
}

SkyKey fdoKey = FdoSupportValue.key(fdoZip);
SkyKey fdoKey = CcSkyframeSupportValue.key(fdoZip);

SkyFunction.Environment skyframeEnv = ruleContext.getAnalysisEnvironment().getSkyframeEnv();
FdoSupportValue fdoSupport = (FdoSupportValue) skyframeEnv.getValue(fdoKey);
CcSkyframeSupportValue fdoSupport = (CcSkyframeSupportValue) skyframeEnv.getValue(fdoKey);
if (skyframeEnv.valuesMissing()) {
return null;
}
Expand Down Expand Up @@ -529,12 +529,12 @@ public ConfiguredTarget create(RuleContext ruleContext)
if (fdoMode == FdoMode.LLVM_FDO) {
profileArtifact =
convertLLVMRawProfileToIndexed(
fdoSupport.getFdoProfile().asFragment(), toolchainInfo, ruleContext);
fdoSupport.getFilePath().asFragment(), toolchainInfo, ruleContext);
if (ruleContext.hasErrors()) {
return null;
}
} else if (fdoMode == FdoMode.AUTO_FDO || fdoMode == FdoMode.XBINARY_FDO) {
Path fdoProfile = fdoSupport.getFdoProfile();
Path fdoProfile = fdoSupport.getFilePath();
profileArtifact = ruleContext.getUniqueDirectoryArtifact(
"fdo",
fdoProfile.getBaseName(),
Expand Down Expand Up @@ -597,9 +597,13 @@ public ConfiguredTarget create(RuleContext ruleContext)
new RuleConfiguredTargetBuilder(ruleContext)
.addNativeDeclaredProvider(ccProvider)
.addNativeDeclaredProvider(templateVariableInfo)
.addProvider(new FdoProvider(
fdoSupport.getFdoProfile(), fdoMode, cppConfiguration.getFdoInstrument(),
profileArtifact, prefetchHintsArtifact))
.addProvider(
new FdoProvider(
fdoSupport.getFilePath(),
fdoMode,
cppConfiguration.getFdoInstrument(),
profileArtifact,
prefetchHintsArtifact))
.setFilesToBuild(crosstool)
.addProvider(RunfilesProvider.simple(Runfiles.EMPTY));

Expand Down
8 changes: 4 additions & 4 deletions src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
import com.google.devtools.build.lib.packages.util.LoadingMock;
import com.google.devtools.build.lib.packages.util.MockCcSupport;
import com.google.devtools.build.lib.packages.util.MockToolsConfig;
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.CcSkyframeSupportValue;
import com.google.devtools.build.lib.rules.cpp.CcSupportFunction;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
Expand Down Expand Up @@ -132,8 +132,8 @@ AndroidSdkRepositoryRule.NAME, new AndroidSdkRepositoryFunction(),
repositoryHandlers, null, new AtomicBoolean(true), ImmutableMap::of, directories),
SkyFunctions.REPOSITORY,
new RepositoryLoaderFunction(),
FdoSupportValue.SKYFUNCTION,
new FdoSupportFunction(directories));
CcSkyframeSupportValue.SKYFUNCTION,
new CcSupportFunction(directories));
}

public static class Delegate extends AnalysisMock {
Expand Down

0 comments on commit 411ab49

Please sign in to comment.