Skip to content

Commit

Permalink
Writes file dependency data when uploading analysis objects.
Browse files Browse the repository at this point in the history
The file dependency data is used to check if a cached analysis object
is fresh.

In this initial phase, we only write the file dependency data without
having clients consume it. Clients skip the file dependency data reference
present in the cached value.

Now that the logic is a bit more complex, splits out the
SelectedEntrySerializer to serialize specific selected values from the
FrontierSerializer, in addition to the file dependency data.

This change adds a couple of test helpers.
* LongVersionGetterTestInjection - to inject LongVersionGetter in tests
* FakeInvalidationDataHelper - to add fake invalidation data to test generated
  cache values.

PiperOrigin-RevId: 718118838
Change-Id: I5de30fb7e4a24e704d5c32e0014a39e2230861f5
  • Loading branch information
aoeui authored and copybara-github committed Jan 22, 2025
1 parent dd37d6e commit 19ecfc4
Show file tree
Hide file tree
Showing 17 changed files with 677 additions and 87 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,8 @@ public void setRemoteAnalysisCachingDependenciesProvider(
* #setRemoteAnalysisCachingDependenciesProvider(RemoteAnalysisCachingDependenciesProvider)} for
* the exact point.
*/
private RemoteAnalysisCachingDependenciesProvider getRemoteAnalysisCachingDependenciesProvider() {
@VisibleForTesting // productionVisibility = Visibility.PRIVATE
public RemoteAnalysisCachingDependenciesProvider getRemoteAnalysisCachingDependenciesProvider() {
return remoteAnalysisCachingDependenciesProvider;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/unsafe:unsafe-provider",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/common/options",
"//src/main/protobuf:file_invalidation_data_java_proto",
"//third_party:auto_value",
"//third_party:caffeine",
"//third_party:error_prone_annotations",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,15 @@ public Object deserializeMemoizedAndBlocking(
public Object deserializeWithSkyframe(
FingerprintValueService fingerprintValueService, ByteString data)
throws SerializationException {
return deserializeWithSkyframe(fingerprintValueService, data.newCodedInput());
}

@Nullable
public Object deserializeWithSkyframe(
FingerprintValueService fingerprintValueService, CodedInputStream codedIn)
throws SerializationException {
return SharedValueDeserializationContext.deserializeWithSkyframe(
getCodecRegistry(), getDependencies(), fingerprintValueService, data);
getCodecRegistry(), getDependencies(), fingerprintValueService, codedIn);
}

static Object deserializeStreamFully(CodedInputStream codedIn, DeserializationContext context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,8 @@ static Object deserializeWithSkyframe(
ObjectCodecRegistry codecRegistry,
ImmutableClassToInstanceMap<Object> dependencies,
FingerprintValueService fingerprintValueService,
ByteString bytes)
CodedInputStream codedIn)
throws SerializationException {
CodedInputStream codedIn = bytes.newCodedInput();
// Enabling aliasing of `codedIn` here might be better for performance but causes deserialized
// values to differ subtly from the input values, complicating testing.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.skyframe.serialization.FingerprintValueStore.MissingFingerprintValueException;
import com.google.devtools.build.lib.skyframe.serialization.analysis.ClientId;
import com.google.devtools.build.lib.skyframe.serialization.proto.DataType;
import com.google.devtools.build.skyframe.IntVersion;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
Expand All @@ -36,8 +37,10 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.protobuf.ByteString;
import com.google.protobuf.CodedInputStream;
import java.io.IOException;
import java.util.Arrays;
import java.util.HexFormat;
import java.util.Optional;
import java.util.concurrent.ExecutionException;

Expand Down Expand Up @@ -207,17 +210,17 @@ public static RetrievalResult tryRetrieve(
Futures.addCallback(
keyWriteStatus, FutureHelpers.FAILURE_REPORTING_CALLBACK, directExecutor());
}
ListenableFuture<byte[]> valueBytes;
ListenableFuture<byte[]> futureValueBytes;
try {
valueBytes =
futureValueBytes =
fingerprintValueService.get(
fingerprintValueService.fingerprint(
frontierNodeVersion.concat(keyBytes.getObject().toByteArray())));
} catch (IOException e) {
throw new SerializationException("key lookup failed for " + key, e);
}
nextState = new WaitingForFutureValueBytes(valueBytes);
switch (futuresShim.dependOnFuture(valueBytes)) {
nextState = new WaitingForFutureValueBytes(futureValueBytes);
switch (futuresShim.dependOnFuture(futureValueBytes)) {
case DONE:
break; // continues to the next state
case NOT_DONE:
Expand All @@ -243,9 +246,40 @@ case WaitingForFutureValueBytes(ListenableFuture<byte[]> futureValueBytes):
nextState = NoCachedData.NO_CACHED_DATA;
break;
}
Object value =
codecs.deserializeWithSkyframe(
fingerprintValueService, ByteString.copyFrom(valueBytes));
var codedIn = CodedInputStream.newInstance(valueBytes);
// Skips over the invalidation data key.
//
// TODO: b/364831651 - this code is a temporary and will eventually be removed when
// this read is performed in the AnalysisCacheService.
try {
int dataTypeOrdinal = codedIn.readInt32();
switch (DataType.forNumber(dataTypeOrdinal)) {
case DATA_TYPE_EMPTY:
break;
case DATA_TYPE_FILE:
// fall through
case DATA_TYPE_LISTING:
{
var unusedKey = codedIn.readString();
break;
}
case DATA_TYPE_NODE:
{
var unusedKey = PackedFingerprint.readFrom(codedIn);
break;
}
default:
throw new SerializationException(
String.format(
"for key=%s, got unexpected data type with ordinal %d from value"
+ " bytes=%s",
key, dataTypeOrdinal, HexFormat.of().formatHex(valueBytes)));
}
} catch (IOException e) {
throw new SerializationException("Error parsing invalidation data key", e);
}

Object value = codecs.deserializeWithSkyframe(fingerprintValueService, codedIn);
if (!(value instanceof ListenableFuture)) {
nextState = new RetrievedValue((SkyValue) value);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,36 @@ java_library(

java_library(
name = "frontier_serializer",
srcs = ["FrontierSerializer.java"],
srcs = [
"FrontierSerializer.java",
"SelectedEntrySerializer.java",
],
deps = [
":dependencies_provider",
":event_listener",
":file_dependency_serializer",
":file_op_node_map",
":invalidation_data_reference_or_future",
":long_version_getter_test_injection",
":options",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator",
"//src/main/java/com/google/devtools/build/lib/skyframe:filesystem_keys",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
"//src/main/java/com/google/devtools/build/lib/util:TestType",
"//src/main/java/com/google/devtools/build/lib/versioning:long_version_getter",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/protobuf:failure_details_java_proto",
"//src/main/protobuf:file_invalidation_data_java_proto",
"//third_party:guava",
"//third_party:jsr305",
"@com_google_protobuf//:protobuf_java",
],
)
Expand Down Expand Up @@ -109,6 +121,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/skyframe:filesystem_keys",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
"//src/main/java/com/google/devtools/build/lib/util:TestType",
"//src/main/java/com/google/devtools/build/lib/versioning:long_version_getter",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
Expand Down Expand Up @@ -194,3 +207,13 @@ java_library(
name = "client_id",
srcs = ["ClientId.java"],
)

java_library(
name = "long_version_getter_test_injection",
srcs = ["LongVersionGetterTestInjection.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/util:TestType",
"//src/main/java/com/google/devtools/build/lib/versioning:long_version_getter",
"//third_party:guava",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static com.google.devtools.build.lib.skyframe.serialization.analysis.InvalidationDataInfoOrFuture.ConstantFileData.CONSTANT_FILE;
import static com.google.devtools.build.lib.skyframe.serialization.analysis.InvalidationDataInfoOrFuture.ConstantListingData.CONSTANT_LISTING;
import static com.google.devtools.build.lib.skyframe.serialization.analysis.InvalidationDataInfoOrFuture.ConstantNodeData.CONSTANT_NODE;
import static com.google.devtools.build.lib.util.TestType.isInTest;
import static com.google.devtools.build.lib.vfs.RootedPath.toRootedPath;
import static java.lang.Math.max;
import static java.nio.charset.StandardCharsets.UTF_8;
Expand Down Expand Up @@ -178,10 +179,11 @@ NodeDataInfoOrFuture registerDependency(AbstractNestedFileOpNodes node) {
FileDataInfoOrFuture populateFutureFileDataInfo(FutureFileDataInfo future) {
FileKey key = future.key();
RootedPath rootedPath = key.argument();
RootedPath parentRootedPath;
// Builtin files don't change.
if (rootedPath.getRoot().getFileSystem() instanceof BundledFileSystem
if ((rootedPath.getRoot().getFileSystem() instanceof BundledFileSystem)
// Assumes that the root folder doesn't change.
|| rootedPath.getRootRelativePath().isEmpty()) {
|| (parentRootedPath = rootedPath.getParentDirectory()) == null) {
return future.completeWith(CONSTANT_FILE);
}

Expand All @@ -199,7 +201,12 @@ FileDataInfoOrFuture populateFutureFileDataInfo(FutureFileDataInfo future) {
return future.failWith(e);
}
}
var uploader = new FileInvalidationDataUploader(rootedPath, realRootedPath, initialMtsv);
var uploader =
new FileInvalidationDataUploader(
/* rootedPath= */ rootedPath,
/* parentRootedPath= */ parentRootedPath,
/* realRootedPath= */ realRootedPath,
initialMtsv);
return future.completeWith(
Futures.transform(
fullyResolvePath(value.isSymlink() ? value.getUnresolvedLinkTarget() : null, uploader),
Expand All @@ -222,9 +229,12 @@ private final class FileInvalidationDataUploader
private long mtsv;

private FileInvalidationDataUploader(
RootedPath rootedPath, RootedPath realRootedPath, long initialMtsv) {
RootedPath rootedPath,
RootedPath parentRootedPath,
RootedPath realRootedPath,
long initialMtsv) {
this.rootedPath = rootedPath;
this.parentRootedPath = rootedPath.getParentDirectory();
this.parentRootedPath = parentRootedPath;
this.realRootedPath = realRootedPath;
this.mtsv = initialMtsv;
}
Expand Down Expand Up @@ -357,6 +367,15 @@ private ListenableFuture<Void> processSymlinks(
Path linkPath,
PathFragment link,
FileInvalidationDataUploader uploader) {
if (link.isAbsolute()) {
if (isInTest()) {
// Test environments may use absolute symlinks, which aren't allowed in production
// environments with analysis caching. Skips further dependency resolution for those.
return immediateVoidFuture();
}
throw new IllegalStateException(
String.format("Absolute symlink not permitted: %s contained %s", linkPath, link));
}
Symlink.Builder symlinkData = uploader.addSymlinksBuilder().setContents(link.getPathString());
PathFragment linkParent = parentRootedPath.getRootRelativePath();
PathFragment unresolvedTarget = linkParent.getRelative(link);
Expand Down
Loading

0 comments on commit 19ecfc4

Please sign in to comment.