Skip to content

Commit

Permalink
Internal change
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 169874059
  • Loading branch information
davido authored and katre committed Sep 25, 2017
1 parent 824db7f commit 82859b0
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public Iterable<? extends ActionContext> getActionContexts() {
String buildRequestId = env.getBuildRequestId().toString();
String commandId = env.getCommandId().toString();

if (remoteOptions.experimentalRemoteSpawnCache) {
if (remoteOptions.experimentalRemoteSpawnCache || remoteOptions.experimentalLocalDiskCache) {
RemoteSpawnCache spawnCache =
new RemoteSpawnCache(
env.getExecRoot(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,18 @@ public void beforeCommand(CommandEnvironment env) {
}

try {
boolean restCache = SimpleBlobStoreFactory.isRemoteCacheOptions(remoteOptions);
boolean remoteOrLocalCache = SimpleBlobStoreFactory.isRemoteCacheOptions(remoteOptions);
boolean grpcCache = GrpcRemoteCache.isRemoteCacheOptions(remoteOptions);

Retrier retrier = new Retrier(remoteOptions);
CallCredentials creds = GrpcUtils.newCallCredentials(authAndTlsOptions);
// TODO(davido): The naming is wrong here. "Remote"-prefix in RemoteActionCache class has no
// meaning.
final RemoteActionCache cache;
if (restCache) {
cache = new SimpleBlobStoreActionCache(SimpleBlobStoreFactory.create(remoteOptions));
if (remoteOrLocalCache) {
cache =
new SimpleBlobStoreActionCache(
SimpleBlobStoreFactory.create(remoteOptions, env.getWorkingDirectory()));
} else if (grpcCache || remoteOptions.remoteExecutor != null) {
// If a remote executor but no remote cache is specified, assume both at the same target.
String target = grpcCache ? remoteOptions.remoteCache : remoteOptions.remoteExecutor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.devtools.build.lib.remote;

import com.google.devtools.build.lib.util.OptionsUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
Expand Down Expand Up @@ -229,6 +231,28 @@ public final class RemoteOptions extends OptionsBase {
)
public boolean experimentalRemoteSpawnCache;

// TODO(davido): Find a better place for this and the next option.
@Option(
name = "experimental_local_disk_cache",
defaultValue = "false",
category = "remote",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Whether to use the experimental local disk cache."
)
public boolean experimentalLocalDiskCache;

@Option(
name = "experimental_local_disk_cache_path",
defaultValue = "null",
category = "remote",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
converter = OptionsUtils.PathFragmentConverter.class,
help = "A file path to a local disk cache."
)
public PathFragment experimentalLocalDiskCachePath;

public Platform parseRemotePlatformOverride() {
if (experimentalRemotePlatformOverride != null) {
Platform.Builder platformBuilder = Platform.newBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@

package com.google.devtools.build.lib.remote;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.devtools.build.lib.remote.blobstore.ConcurrentMapBlobStore;
import com.google.devtools.build.lib.remote.blobstore.OnDiskBlobStore;
import com.google.devtools.build.lib.remote.blobstore.RestBlobStore;
import com.google.devtools.build.lib.remote.blobstore.SimpleBlobStore;
import com.google.devtools.build.lib.vfs.Path;
import com.hazelcast.client.HazelcastClient;
import com.hazelcast.client.config.ClientConfig;
import com.hazelcast.client.config.ClientNetworkConfig;
Expand All @@ -26,10 +30,11 @@
import com.hazelcast.core.HazelcastInstance;
import java.io.IOException;
import java.util.concurrent.ConcurrentMap;
import javax.annotation.Nullable;

/**
* A factory class for providing a {@link SimpleBlobStore} to be used with {@link
* SimpleBlobStoreActionCache}. Currently implemented with Hazelcast or REST.
* SimpleBlobStoreActionCache}. Currently implemented with Hazelcast, REST or local.
*/
public final class SimpleBlobStoreFactory {

Expand Down Expand Up @@ -74,20 +79,34 @@ public static SimpleBlobStore createRest(RemoteOptions options) throws IOExcepti
return new RestBlobStore(options.remoteRestCache, options.restCachePoolSize);
}

public static SimpleBlobStore create(RemoteOptions options) throws IOException {
public static SimpleBlobStore createLocalDisk(RemoteOptions options, Path workingDirectory)
throws IOException {
return new OnDiskBlobStore(
workingDirectory.getRelative(checkNotNull(options.experimentalLocalDiskCachePath)));
}

public static SimpleBlobStore create(RemoteOptions options, @Nullable Path workingDirectory)
throws IOException {
if (isHazelcastOptions(options)) {
return createHazelcast(options);
}
if (isRestUrlOptions(options)) {
return createRest(options);
}
if (workingDirectory != null && isLocalDiskCache(options)) {
return createLocalDisk(options, workingDirectory);
}
throw new IllegalArgumentException(
"Unrecognized concurrent map RemoteOptions: must specify "
+ "either Hazelcast or Rest URL options.");
+ "either Hazelcast, Rest URL, or local cache options.");
}

public static boolean isRemoteCacheOptions(RemoteOptions options) {
return isHazelcastOptions(options) || isRestUrlOptions(options);
return isHazelcastOptions(options) || isRestUrlOptions(options) || isLocalDiskCache(options);
}

public static boolean isLocalDiskCache(RemoteOptions options) {
return options.experimentalLocalDiskCache;
}

private static boolean isHazelcastOptions(RemoteOptions options) {
Expand Down
7 changes: 7 additions & 0 deletions src/test/shell/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,13 @@ sh_test(
shard_count = 6,
)

sh_test(
name = "local_action_cache_test",
size = "small",
srcs = ["local_action_cache_test.sh"],
data = [":test-deps"],
)

sh_test(
name = "runfiles_test",
size = "medium",
Expand Down
69 changes: 69 additions & 0 deletions src/test/shell/bazel/local_action_cache_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
#!/bin/bash
#
# Copyright 2017 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.
#
# Test the local action cache
#

# Load the test setup defined in the parent directory
CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
source "${CURRENT_DIR}/../integration_test_setup.sh" \
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }

function test_local_action_cache() {
local cache="${TEST_TMPDIR}/cache"
local execution_file="${TEST_TMPDIR}/run.log"
local input_file="foo.in"
local output_file="bazel-genfiles/foo.txt"
local flags="--experimental_local_disk_cache_path=$cache --experimental_local_disk_cache"

rm -rf $cache
mkdir $cache

touch WORKSPACE
# No sandboxing, side effect is needed to detect action execution
cat > BUILD <<EOF
genrule(
name = "foo",
cmd = "echo run > $execution_file && cat \$< >\$@",
srcs = ["$input_file"],
outs = ["foo.txt"],
tags = ["local"],
)
EOF

# CAS is empty, cache miss
echo 0 >"${execution_file}"
echo 1 >"${input_file}"
bazel build $flags :foo &> $TEST_log || fail "Build failed"
assert_equals "1" $(cat "${output_file}")
assert_equals "run" $(cat "${execution_file}")

# CAS doesn't have output for this input, cache miss
echo 0 >"${execution_file}"
echo 2 >"${input_file}"
bazel build $flags :foo &> $TEST_log || fail "Build failed"
assert_equals "2" $(cat "${output_file}")
assert_equals "run" $(cat "${execution_file}")

# Cache hit, no action run/no side effect
echo 0 >"${execution_file}"
echo 1 >"${input_file}"
bazel build $flags :foo &> $TEST_log || fail "Build failed"
assert_equals "1" $(cat "${output_file}")
assert_equals "0" $(cat "${execution_file}")
}

run_suite "local action cache test"
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public static void main(String[] args) throws Exception {

SimpleBlobStore blobStore =
usingRemoteCache
? SimpleBlobStoreFactory.create(remoteOptions)
? SimpleBlobStoreFactory.create(remoteOptions, null)
: remoteWorkerOptions.casPath != null
? new OnDiskBlobStore(fs.getPath(remoteWorkerOptions.casPath))
: new ConcurrentMapBlobStore(new ConcurrentHashMap<String, byte[]>());
Expand Down

1 comment on commit 82859b0

@damienmg
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit description was unfortunately scrubbed by our export process.

The original code review happened on https://bazel-review.googlesource.com/c/bazel/+/16810

Please sign in to comment.