Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Enable populating java/kotlin/android artifacts for darwin (M1) from linux #18669

Open
wants to merge 2 commits into
base: release-6.2.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4,042 changes: 4,042 additions & 0 deletions CHANGELOG.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,7 @@ private static String buildCpuIdentifier(
CoreOptions options, @Nullable PlatformOptions platformOptions) {
if (options.platformInOutputDir && platformOptions != null) {
Label targetPlatform = platformOptions.computeTargetPlatform();
// Only use non-default platforms.
if (!PlatformOptions.platformIsDefault(targetPlatform)) {
return targetPlatform.getName();
}
return targetPlatform.getName();
}

// Fall back to using the CPU.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Strings.isNullOrEmpty;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.util.concurrent.Futures.immediateFailedFuture;
import static com.google.common.util.concurrent.Futures.immediateFuture;
import static com.google.common.util.concurrent.Futures.transform;
Expand Down Expand Up @@ -175,6 +176,25 @@ public class RemoteExecutionService {
private final AtomicBoolean shutdown = new AtomicBoolean(false);
private final AtomicBoolean buildInterrupted = new AtomicBoolean(false);

private boolean isActionCompatibleWithXPlatformCache(Spawn spawn) {
String repo = spawn.getResourceOwner().getOwner().getLabel().getPackageIdentifier().getRepository().getName();
if (!repo.isEmpty()) {
return false;
}
String mnemonic = spawn.getResourceOwner().getMnemonic();
return remoteOptions.remoteXPlatSupportedMnemonics.contains(mnemonic);
}

private boolean isInputRemovedForXPlatform(PathFragment pathFragment) {
String path = pathFragment.getPathString();
return remoteOptions.remoteXPlatRemovedInputs.stream().anyMatch(s -> path.indexOf(s) >= 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to #15638 if we use regex here, it could simplify declaration when we have lot of inputs to exclude.

}

private boolean isInputIgnoredForXPlatform(PathFragment pathFragment) {
String path = pathFragment.getPathString();
return remoteOptions.remoteXPlatIgnoredInputs.stream().anyMatch(s -> path.indexOf(s) >= 0);
}

public RemoteExecutionService(
Executor executor,
Reporter reporter,
Expand Down Expand Up @@ -218,7 +238,8 @@ static Command buildCommand(
List<String> arguments,
ImmutableMap<String, String> env,
@Nullable Platform platform,
RemotePathResolver remotePathResolver) {
RemotePathResolver remotePathResolver,
boolean actionCompatibleWithXPlatformCache) {
Command.Builder command = Command.newBuilder();
ArrayList<String> outputFiles = new ArrayList<>();
ArrayList<String> outputDirectories = new ArrayList<>();
Expand All @@ -239,6 +260,11 @@ static Command buildCommand(
command.setPlatform(platform);
}
for (String arg : arguments) {
if (actionCompatibleWithXPlatformCache) {
// Ensure the command line matches linux for M1
arg = arg.replace("macos_aarch64", "linux");
arg = arg.replace("darwin_arm64", "linux");
}
tjgq marked this conversation as resolved.
Show resolved Hide resolved
command.addArguments(decodeBytestringUtf8(arg));
}
// Sorting the environment pairs by variable name.
Expand Down Expand Up @@ -392,6 +418,17 @@ private MerkleTree buildInputMerkleTree(
SortedMap<PathFragment, ActionInput> inputMap =
remotePathResolver.getInputMapping(
context, /* willAccessRepeatedly= */ !remoteOptions.remoteDiscardMerkleTrees);

// Compute removed and ignored inputs, to make actionkey hash computation output match between platform.
boolean useXPlatformCache = isActionCompatibleWithXPlatformCache(spawn);
ImmutableSet<PathFragment> removedInputs = useXPlatformCache ? inputMap.keySet().stream().filter(this::isInputRemovedForXPlatform).collect(toImmutableSet()) : ImmutableSet.of();
ImmutableSet<PathFragment> ignoredInputs = useXPlatformCache ? inputMap.keySet().stream().filter(this::isInputIgnoredForXPlatform).collect(toImmutableSet()) : ImmutableSet.of();

// Remove inputs only present on specific platform, so that they won't contribute to actionkey hash.
for (PathFragment fragment : removedInputs) {
inputMap.remove(fragment);
}

if (!outputDirMap.isEmpty()) {
// The map returned by getInputMapping is mutable, but must not be mutated here as it is
// shared with all other strategies.
Expand All @@ -406,7 +443,8 @@ private MerkleTree buildInputMerkleTree(
context.getMetadataProvider(),
execRoot,
context.getPathResolver(),
digestUtil);
digestUtil,
ignoredInputs);
}
}

Expand Down Expand Up @@ -470,7 +508,7 @@ public MerkleTree uncachedBuildMerkleTreeVisitor(
}

@Nullable
private static ByteString buildSalt(Spawn spawn) {
private ByteString buildSalt(Spawn spawn) {
String workspace =
spawn.getExecutionInfo().get(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE);
if (workspace != null) {
Expand All @@ -479,10 +517,13 @@ private static ByteString buildSalt(Spawn spawn) {
.addProperties(
Platform.Property.newBuilder().setName("workspace").setValue(workspace).build())
.build();
return platform.toByteString();
ByteString salt = platform.toByteString();
return remoteOptions.remoteActionKeySalt.isEmpty() ?
salt : salt.concat(ByteString.copyFromUtf8(remoteOptions.remoteActionKeySalt));
}

return null;
return remoteOptions.remoteActionKeySalt.isEmpty() ?
null : ByteString.copyFromUtf8(remoteOptions.remoteActionKeySalt);
}

/**
Expand Down Expand Up @@ -528,14 +569,14 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context
} else {
platform = PlatformUtils.getPlatformProto(spawn, remoteOptions);
}

Command command =
buildCommand(
spawn.getOutputFiles(),
spawn.getArguments(),
spawn.getEnvironment(),
platform,
remotePathResolver);
remotePathResolver,
isActionCompatibleWithXPlatformCache(spawn));
Digest commandHash = digestUtil.compute(command);
Action action =
Utils.buildAction(
Expand Down Expand Up @@ -1400,7 +1441,7 @@ public void onError(@NonNull Throwable e) {
});
} else {
try (SilentCloseable c =
Profiler.instance().profile(ProfilerTask.UPLOAD_TIME, "upload outputs")) {
Profiler.instance().profile(ProfilerTask.UPLOAD_TIME, "upload outputs for " + action.getActionKey().getDigest().getHash())) {
UploadManifest manifest = buildUploadManifest(action, spawnResult);
manifest.upload(action.getRemoteActionExecutionContext(), remoteCache, reporter);
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public ExecutionResult execute(
ActionKey actionKey = new ActionKey(actionDigest);
CachedActionResult cachedActionResult;
try (SilentCloseable c =
Profiler.instance().profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) {
Profiler.instance().profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit for " + actionKey.getDigest().getHash())) {
cachedActionResult =
remoteCache.downloadActionResult(context, actionKey, /* inlineOutErr= */ true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,15 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
// This is done via a thread-local variable.
try {
RemoteActionResult result;
try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) {
try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit for " + action.getActionKey().getDigest().getHash())) {
result = remoteExecutionService.lookupCache(action);
}

String verb = (result != null && result.getExitCode() == 0) ? "HIT" : "MISS";
if (options.remotePrintExecutionMessages == RemoteOptions.ExecutionMessagePrintMode.ALL) {
System.out.println("RemoteSpawnCache: Lookup " + verb + " with key " + action.getActionKey().getDigest().getHash() + " for action " + action.getSpawn().getResourceOwner().getOwner().getLabel() + " [" + action.getSpawn().getResourceOwner().getMnemonic() + "]");
}

// In case the remote cache returned a failed action (exit code != 0) we treat it as a
// cache miss
if (result != null && result.getExitCode() == 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)

// Try to lookup the action in the action cache.
RemoteActionResult cachedResult;
try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) {
try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit for " + action.getActionKey().getDigest().getHash())) {
cachedResult = acceptCachedResult ? remoteExecutionService.lookupCache(action) : null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ static DirectoryTree fromActionInputs(
DigestUtil digestUtil)
throws IOException {
return fromActionInputs(
inputs, ImmutableSet.of(), metadataProvider, execRoot, artifactPathResolver, digestUtil);
inputs, ImmutableSet.of(), metadataProvider, execRoot, artifactPathResolver, digestUtil, ImmutableSet.of());
}

static DirectoryTree fromActionInputs(
Expand All @@ -75,12 +75,13 @@ static DirectoryTree fromActionInputs(
MetadataProvider metadataProvider,
Path execRoot,
ArtifactPathResolver artifactPathResolver,
DigestUtil digestUtil)
DigestUtil digestUtil,
Set<PathFragment> ignoredInputs)
throws IOException {
Map<PathFragment, DirectoryNode> tree = new HashMap<>();
int numFiles =
buildFromActionInputs(
inputs, toolInputs, metadataProvider, execRoot, artifactPathResolver, digestUtil, tree);
inputs, toolInputs, metadataProvider, execRoot, artifactPathResolver, digestUtil, ignoredInputs, tree);
return new DirectoryTree(tree, numFiles);
}

Expand Down Expand Up @@ -142,6 +143,7 @@ private static int buildFromActionInputs(
Path execRoot,
ArtifactPathResolver artifactPathResolver,
DigestUtil digestUtil,
Set<PathFragment> ignoredInputs,
Map<PathFragment, DirectoryNode> tree)
throws IOException {
return build(
Expand Down Expand Up @@ -170,6 +172,12 @@ private static int buildFromActionInputs(
case REGULAR_FILE:
{
Digest d = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize());

if (ignoredInputs.contains(path)) {
// Digest differs per platform, default to 0 for all platforms.
d = Digest.getDefaultInstance();
}

Path inputPath = artifactPathResolver.toPath(input);
boolean childAdded =
currDir.addChild(
Expand All @@ -188,6 +196,7 @@ private static int buildFromActionInputs(
execRoot,
artifactPathResolver,
digestUtil,
ignoredInputs,
tree);

case SYMLINK:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,17 +245,18 @@ public static MerkleTree build(
* @param digestUtil a hashing utility
*/
public static MerkleTree build(
SortedMap<PathFragment, ActionInput> inputs,
Set<PathFragment> toolInputs,
MetadataProvider metadataProvider,
Path execRoot,
ArtifactPathResolver artifactPathResolver,
DigestUtil digestUtil)
throws IOException {
SortedMap<PathFragment, ActionInput> inputs,
Set<PathFragment> toolInputs,
MetadataProvider metadataProvider,
Path execRoot,
ArtifactPathResolver artifactPathResolver,
DigestUtil digestUtil,
Set<PathFragment> ignoredInputs)
throws IOException {
try (SilentCloseable c = Profiler.instance().profile("MerkleTree.build(ActionInput)")) {
DirectoryTree tree =
DirectoryTreeBuilder.fromActionInputs(
inputs, toolInputs, metadataProvider, execRoot, artifactPathResolver, digestUtil);
DirectoryTreeBuilder.fromActionInputs(
inputs, toolInputs, metadataProvider, execRoot, artifactPathResolver, digestUtil, ignoredInputs);
return build(tree, digestUtil);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,56 @@ public RemoteOutputsStrategyConverter() {
+ "`all` to print always.")
public ExecutionMessagePrintMode remotePrintExecutionMessages;

@Option(
name = "remote_actionkey_salt",
defaultValue = "",
documentationCategory = OptionDocumentationCategory.REMOTE,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Salt used when hashing remote actionkeys. Used for remote cache hits troubleshooting, or invalidating " +
"entire cache when updating files not tracked by actions but causing different artifact outputs (see " +
"support for xplat artifacts")
public String remoteActionKeySalt;

@Option(
name = "remote_xplat_supported_mnemonics",
converter = Converters.CommaSeparatedOptionListConverter.class,
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.REMOTE,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"Specify the list of mnemonics whose artifact outputs should be treated as platform independant." +
"This is used to support x-platform cache population",
allowMultiple = true)
public List<String> remoteXPlatSupportedMnemonics;

@Option(
name = "remote_xplat_removed_inputs",
converter = Converters.CommaSeparatedOptionListConverter.class,
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.REMOTE,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"Specify the actions inputs that should be removed. They will be removed if their paths contains any " +
"of the specified value here. Removing input from an action can be used to make action key hash " +
"platform independent, at the risk of not rebuilding this action if this input only changes. For such" +
"changes, developers need to bump the salt to re-hash every actions",
allowMultiple = true)
public List<String> remoteXPlatRemovedInputs;

@Option(
name = "remote_xplat_ignored_inputs",
converter = Converters.CommaSeparatedOptionListConverter.class,
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.REMOTE,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"Specify the actions inputs that should be ignored. They will used a default (0) hash as these inputs " +
"exist on different platform, but have different size/hash. Ignoring input from an action can be used to make action key hash " +
"platform independent, at the risk of not rebuilding this action if this input only changes. For such" +
"changes, developers need to bump the salt to re-hash every actions",
allowMultiple = true)
public List<String> remoteXPlatIgnoredInputs;

@Option(
name = "incompatible_remote_downloader_send_all_headers",
defaultValue = "true",
Expand Down