diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobDescriptor.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobDescriptor.java index c9f26f36add2c4..e3b15f6b72cdb5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobDescriptor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobDescriptor.java @@ -14,31 +14,43 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Preconditions; +import com.google.common.collect.Interner; import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.cmdline.PackageIdentifierCodec; +import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; +import com.google.devtools.build.lib.skyframe.serialization.SerializationException; +import com.google.devtools.build.lib.skyframe.serialization.strings.StringCodecs; import com.google.devtools.build.lib.util.StringCanonicalizer; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathCodec; import com.google.devtools.build.lib.vfs.PathFragment; -import java.io.Serializable; -import java.util.Objects; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.protobuf.CodedInputStream; +import com.google.protobuf.CodedOutputStream; +import java.io.IOException; /** - * A descriptor for a glob request. + * A descriptor for a glob request, used as the {@link SkyKey} for {@link GlobFunction}. * *

{@code subdir} must be empty or point to an existing directory.

* *

{@code pattern} must be valid, as indicated by {@code UnixGlob#checkPatternForError}. */ @ThreadSafe -public final class GlobDescriptor implements Serializable { - final PackageIdentifier packageId; - final Path packageRoot; - final PathFragment subdir; - final String pattern; - final boolean excludeDirs; +public final class GlobDescriptor implements SkyKey { + + private static final Interner interner = BlazeInterners.newWeakInterner(); + + /** Creates and returns a new {@link ObjectCodec} for {@link GlobDescriptor}s. */ + public static ObjectCodec getCodec(PathCodec pathCodec) { + return new GlobDescriptorCodec(pathCodec); + } /** - * Constructs a GlobDescriptor. + * Returns interned instance based on the parameters. * * @param packageId the name of the owner package (must be an existing package) * @param packageRoot the package root of {@code packageId} @@ -48,7 +60,24 @@ public final class GlobDescriptor implements Serializable { * @param pattern a valid glob pattern * @param excludeDirs true if directories should be excluded from results */ - public GlobDescriptor(PackageIdentifier packageId, Path packageRoot, PathFragment subdir, + public static GlobDescriptor create( + PackageIdentifier packageId, + Path packageRoot, + PathFragment subdir, + String pattern, + boolean excludeDirs) { + return interner.intern( + new GlobDescriptor(packageId, packageRoot, subdir, pattern, excludeDirs)); + + } + + private final PackageIdentifier packageId; + private final Path packageRoot; + private final PathFragment subdir; + private final String pattern; + private final boolean excludeDirs; + + private GlobDescriptor(PackageIdentifier packageId, Path packageRoot, PathFragment subdir, String pattern, boolean excludeDirs) { this.packageId = Preconditions.checkNotNull(packageId); this.packageRoot = Preconditions.checkNotNull(packageRoot); @@ -104,11 +133,6 @@ public boolean excludeDirs() { return excludeDirs; } - @Override - public int hashCode() { - return Objects.hash(packageId, subdir, pattern, excludeDirs); - } - @Override public boolean equals(Object obj) { if (this == obj) { @@ -122,4 +146,60 @@ public boolean equals(Object obj) { && subdir.equals(other.subdir) && pattern.equals(other.pattern) && excludeDirs == other.excludeDirs; } + + @Override + public int hashCode() { + // Generated instead of Objects.hashCode to avoid intermediate array required for latter. + final int prime = 31; + int result = 1; + result = prime * result + (excludeDirs ? 1231 : 1237); + result = prime * result + packageId.hashCode(); + result = prime * result + packageRoot.hashCode(); + result = prime * result + pattern.hashCode(); + result = prime * result + subdir.hashCode(); + return result; + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.GLOB; + } + + private static class GlobDescriptorCodec implements ObjectCodec { + + private final PackageIdentifierCodec packageIdCodec = new PackageIdentifierCodec(); + private final PathCodec pathCodec; + private final ObjectCodec stringCodec = StringCodecs.asciiOptimized(); + + private GlobDescriptorCodec(PathCodec pathCodec) { + this.pathCodec = pathCodec; + } + + @Override + public Class getEncodedClass() { + return GlobDescriptor.class; + } + + @Override + public void serialize(GlobDescriptor globDesc, CodedOutputStream codedOut) + throws IOException, SerializationException { + packageIdCodec.serialize(globDesc.getPackageId(), codedOut); + pathCodec.serialize(globDesc.getPackageRoot(), codedOut); + PathFragment.CODEC.serialize(globDesc.getSubdir(), codedOut); + stringCodec.serialize(globDesc.getPattern(), codedOut); + codedOut.writeBoolNoTag(globDesc.excludeDirs()); + } + + @Override + public GlobDescriptor deserialize(CodedInputStream codedIn) + throws SerializationException, IOException { + PackageIdentifier packageId = packageIdCodec.deserialize(codedIn); + Path packageRoot = pathCodec.deserialize(codedIn); + PathFragment pathFragment = PathFragment.CODEC.deserialize(codedIn); + String pattern = stringCodec.deserialize(codedIn); + boolean excludeDirs = codedIn.readBool(); + return GlobDescriptor.create(packageId, packageRoot, pathFragment, pattern, excludeDirs); + } + } + } \ No newline at end of file diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java index 6d71a63cb27f90..eb00cb9f205d40 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.UnixGlob; -import com.google.devtools.build.skyframe.LegacySkyKey; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; @@ -104,9 +103,7 @@ public static SkyKey key(PackageIdentifier packageId, Path packageRoot, String p @ThreadSafe static SkyKey internalKey(PackageIdentifier packageId, Path packageRoot, PathFragment subdir, String pattern, boolean excludeDirs) { - return LegacySkyKey.create( - SkyFunctions.GLOB, - new GlobDescriptor(packageId, packageRoot, subdir, pattern, excludeDirs)); + return GlobDescriptor.create(packageId, packageRoot, subdir, pattern, excludeDirs); } /** @@ -116,8 +113,12 @@ static SkyKey internalKey(PackageIdentifier packageId, Path packageRoot, PathFra */ @ThreadSafe static SkyKey internalKey(GlobDescriptor glob, String subdirName) { - return internalKey(glob.packageId, glob.packageRoot, glob.subdir.getRelative(subdirName), - glob.pattern, glob.excludeDirs); + return internalKey( + glob.getPackageId(), + glob.getPackageRoot(), + glob.getSubdir().getRelative(subdirName), + glob.getPattern(), + glob.excludeDirs()); } /** diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD index a92f87161bc373..a4b01da48195ad 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD @@ -78,6 +78,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/rules/cpp", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/main/java/com/google/devtools/build/skyframe", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobDescriptorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobDescriptorTest.java new file mode 100644 index 00000000000000..3601eaa0b02f11 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skyframe/GlobDescriptorTest.java @@ -0,0 +1,79 @@ +// 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. +package com.google.devtools.build.lib.skyframe; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.skyframe.serialization.testutils.FsUtils; +import com.google.devtools.build.lib.skyframe.serialization.testutils.ObjectCodecTester; +import com.google.devtools.build.lib.vfs.PathCodec; +import com.google.devtools.build.lib.vfs.PathFragment; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link GlobDescriptor}. */ +@RunWith(JUnit4.class) +public class GlobDescriptorTest { + + @Test + public void testSerialization() throws Exception { + ObjectCodecTester.newBuilder(GlobDescriptor.getCodec(new PathCodec(FsUtils.TEST_FILESYSTEM))) + .addSubjects( + GlobDescriptor.create( + PackageIdentifier.create("@foo", PathFragment.create("//bar")), + FsUtils.TEST_FILESYSTEM.getPath("/packageRoot"), + PathFragment.create("subdir"), + "pattern", + /*excludeDirs=*/ false), + GlobDescriptor.create( + PackageIdentifier.create("@bar", PathFragment.create("//foo")), + FsUtils.TEST_FILESYSTEM.getPath("/anotherPackageRoot"), + PathFragment.create("anotherSubdir"), + "pattern", + /*excludeDirs=*/ true)) + .verificationFunction( + (orig, deserialized) -> assertThat(deserialized).isSameAs(orig)) + .buildAndRunTests(); + } + + @Test + public void testCreateReturnsInternedInstances() throws LabelSyntaxException { + GlobDescriptor original = GlobDescriptor.create( + PackageIdentifier.create("@foo", PathFragment.create("//bar")), + FsUtils.TEST_FILESYSTEM.getPath("/packageRoot"), + PathFragment.create("subdir"), + "pattern", + /*excludeDirs=*/ false); + + GlobDescriptor sameCopy = GlobDescriptor.create( + original.getPackageId(), + original.getPackageRoot(), + original.getSubdir(), + original.getPattern(), + original.excludeDirs()); + assertThat(sameCopy).isSameAs(original); + + GlobDescriptor diffCopy = GlobDescriptor.create( + original.getPackageId(), + original.getPackageRoot(), + original.getSubdir(), + original.getPattern(), + !original.excludeDirs()); + assertThat(diffCopy).isNotEqualTo(original); + } + +}