Intern PackageIdentifiers as a memory optimization.
--
MOS_MIGRATED_REVID=104403040
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
index 4e799e3..f7126ae 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -61,7 +61,8 @@
try {
LabelValidator.PackageAndTarget labelParts = LabelValidator.parseAbsoluteLabel(absName);
validate(labelParts.getPackageName(), labelParts.getTargetName());
- return new Label(new PackageIdentifier(repo, new PathFragment(labelParts.getPackageName())),
+ return new Label(
+ PackageIdentifier.create(repo, new PathFragment(labelParts.getPackageName())),
labelParts.getTargetName());
} catch (BadLabelException e) {
throw new LabelSyntaxException(e.getMessage());
@@ -357,7 +358,8 @@
} else {
try {
return new Label(
- new PackageIdentifier(packageIdentifier.getRepository(), relative.getPackageFragment()),
+ PackageIdentifier
+ .create(packageIdentifier.getRepository(), relative.getPackageFragment()),
relative.getName());
} catch (LabelSyntaxException e) {
// We are creating the new label from an existing one which is guaranteed to be valid, so
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
index 65f4258..991544c 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
@@ -20,6 +20,8 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ComparisonChain;
+import com.google.common.collect.Interner;
+import com.google.common.collect.Interners;
import com.google.devtools.build.lib.util.StringCanonicalizer;
import com.google.devtools.build.lib.util.StringUtilities;
import com.google.devtools.build.lib.vfs.Canonicalizer;
@@ -45,6 +47,17 @@
@Immutable
public final class PackageIdentifier implements Comparable<PackageIdentifier>, Serializable {
+ private static final Interner<PackageIdentifier> INTERNER = Interners.newStrongInterner();
+
+ public static PackageIdentifier create(String repository, PathFragment pkgName)
+ throws LabelSyntaxException {
+ return create(RepositoryName.create(repository), pkgName);
+ }
+
+ public static PackageIdentifier create(RepositoryName repository, PathFragment pkgName) {
+ return INTERNER.intern(new PackageIdentifier(repository, pkgName));
+ }
+
/**
* A human-readable name for the repository.
*/
@@ -281,7 +294,7 @@
public static PackageIdentifier createInDefaultRepo(PathFragment name) {
try {
- return new PackageIdentifier(DEFAULT_REPOSITORY, name);
+ return create(DEFAULT_REPOSITORY, name);
} catch (LabelSyntaxException e) {
throw new IllegalArgumentException("could not create package identifier for " + name
+ ": " + e.getMessage());
@@ -297,11 +310,7 @@
/** The name of the package. Canonical (i.e. x.equals(y) <=> x==y). */
private final PathFragment pkgName;
- public PackageIdentifier(String repository, PathFragment pkgName) throws LabelSyntaxException {
- this(RepositoryName.create(repository), pkgName);
- }
-
- public PackageIdentifier(RepositoryName repository, PathFragment pkgName) {
+ private PackageIdentifier(RepositoryName repository, PathFragment pkgName) {
Preconditions.checkNotNull(repository);
Preconditions.checkNotNull(pkgName);
this.repository = repository;
@@ -335,7 +344,7 @@
throw new LabelSyntaxException(error);
}
- return new PackageIdentifier(repo, new PathFragment(packageName));
+ return create(repo, new PathFragment(packageName));
}
public RepositoryName getRepository() {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
index 233a199..460e20b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
@@ -115,7 +115,7 @@
return true;
}
- PackageIdentifier subPackageId = new PackageIdentifier(
+ PackageIdentifier subPackageId = PackageIdentifier.create(
packageId.getRepository(),
packageId.getPackageFragment().getRelative(directory.relativeTo(packageDirectory)));
UnixGlob.FilesystemCalls syscalls = GlobCache.this.syscalls.get();
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java b/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java
index 1cd8294..ab58edd 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java
@@ -448,7 +448,8 @@
Package.Builder builder;
try {
builder = new Package.Builder(
- new PackageIdentifier(packagePb.getRepository(), new PathFragment(packagePb.getName())),
+ PackageIdentifier
+ .create(packagePb.getRepository(), new PathFragment(packagePb.getName())),
null);
} catch (LabelSyntaxException e) {
throw new PackageDeserializationException(e);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunction.java
index 80619e3..6d6511a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunction.java
@@ -43,7 +43,7 @@
if (parentDir == null) {
return ContainingPackageLookupValue.noContainingPackage();
}
- PackageIdentifier parentId = new PackageIdentifier(dir.getRepository(), parentDir);
+ PackageIdentifier parentId = PackageIdentifier.create(dir.getRepository(), parentDir);
return env.getValue(ContainingPackageLookupValue.key(parentId));
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
index 74e155c..db63627 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
@@ -67,7 +67,7 @@
PathFragment globSubdir = glob.getSubdir();
if (!globSubdir.equals(PathFragment.EMPTY_FRAGMENT)) {
PackageLookupValue globSubdirPkgLookupValue = (PackageLookupValue) env.getValue(
- PackageLookupValue.key(new PackageIdentifier(
+ PackageLookupValue.key(PackageIdentifier.create(
glob.getPackageId().getRepository(),
glob.getPackageId().getPackageFragment().getRelative(globSubdir))));
if (globSubdirPkgLookupValue == null) {
@@ -228,7 +228,7 @@
PathFragment directory = glob.getPackageId().getPackageFragment()
.getRelative(glob.getSubdir()).getRelative(fileName);
PackageLookupValue pkgLookupValue = (PackageLookupValue)
- env.getValue(PackageLookupValue.key(new PackageIdentifier(
+ env.getValue(PackageLookupValue.key(PackageIdentifier.create(
glob.getPackageId().getRepository(), directory)));
if (pkgLookupValue == null) {
return;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 7155768..73b5f99 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -745,7 +745,7 @@
Map<Target, SkyKey> targetToKey = new HashMap<>();
for (Target target : pkgBuilder.getTargets()) {
PathFragment dir = target.getLabel().toPathFragment().getParentDirectory();
- PackageIdentifier dirId = new PackageIdentifier(pkgId.getRepository(), dir);
+ PackageIdentifier dirId = PackageIdentifier.create(pkgId.getRepository(), dir);
if (dir.equals(pkgId.getPackageFragment())) {
continue;
}
@@ -756,7 +756,7 @@
Map<Label, SkyKey> subincludeToKey = new HashMap<>();
for (Label subincludeLabel : pkgBuilder.getSubincludeLabels()) {
PathFragment dir = subincludeLabel.toPathFragment().getParentDirectory();
- PackageIdentifier dirId = new PackageIdentifier(pkgId.getRepository(), dir);
+ PackageIdentifier dirId = PackageIdentifier.create(pkgId.getRepository(), dir);
if (dir.equals(pkgId.getPackageFragment())) {
continue;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java
index 9b0e448..1927851 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java
@@ -135,7 +135,7 @@
return getEmptyReturn();
}
- PackageIdentifier packageId = new PackageIdentifier(
+ PackageIdentifier packageId = PackageIdentifier.create(
recursivePkgKey.getRepository(), rootRelativePath);
PackageLookupValue pkgLookupValue;
try {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index 2c0b615..abc44906 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -183,7 +183,7 @@
throws SkylarkImportLookupFunctionException {
ContainingPackageLookupValue containingPackageLookupValue = null;
try {
- PackageIdentifier newPkgId = new PackageIdentifier(repo, file.getParentDirectory());
+ PackageIdentifier newPkgId = PackageIdentifier.create(repo, file.getParentDirectory());
containingPackageLookupValue = (ContainingPackageLookupValue) env.getValueOrThrow(
ContainingPackageLookupValue.key(newPkgId),
BuildFileNotFoundException.class, InconsistentFilesystemException.class);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
index dd6d3c9..dc997a1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
@@ -96,6 +96,6 @@
checkInputArgument(fileToImport);
return new SkyKey(
SkyFunctions.SKYLARK_IMPORTS_LOOKUP,
- new PackageIdentifier(repo, fileToImport));
+ PackageIdentifier.create(repo, fileToImport));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java
index ffbf874..3e3fbd5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java
@@ -49,7 +49,7 @@
PathFragment containingDirectory = label.toPathFragment().getParentDirectory();
ContainingPackageLookupValue containingPackageLookupValue = null;
try {
- PackageIdentifier newPkgId = new PackageIdentifier(
+ PackageIdentifier newPkgId = PackageIdentifier.create(
label.getPackageIdentifier().getRepository(), containingDirectory);
containingPackageLookupValue = (ContainingPackageLookupValue) env.getValueOrThrow(
ContainingPackageLookupValue.key(newPkgId),
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
index 6094468..dea21c3 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
@@ -91,18 +91,18 @@
@Test
public void testToString() throws Exception {
- PackageIdentifier local = new PackageIdentifier("", new PathFragment("bar/baz"));
+ PackageIdentifier local = PackageIdentifier.create("", new PathFragment("bar/baz"));
assertEquals("bar/baz", local.toString());
- PackageIdentifier external = new PackageIdentifier("@foo", new PathFragment("bar/baz"));
+ PackageIdentifier external = PackageIdentifier.create("@foo", new PathFragment("bar/baz"));
assertEquals("@foo//bar/baz", external.toString());
}
@Test
public void testCompareTo() throws Exception {
- PackageIdentifier foo1 = new PackageIdentifier("@foo", new PathFragment("bar/baz"));
- PackageIdentifier foo2 = new PackageIdentifier("@foo", new PathFragment("bar/baz"));
- PackageIdentifier foo3 = new PackageIdentifier("@foo", new PathFragment("bar/bz"));
- PackageIdentifier bar = new PackageIdentifier("@bar", new PathFragment("bar/baz"));
+ PackageIdentifier foo1 = PackageIdentifier.create("@foo", new PathFragment("bar/baz"));
+ PackageIdentifier foo2 = PackageIdentifier.create("@foo", new PathFragment("bar/baz"));
+ PackageIdentifier foo3 = PackageIdentifier.create("@foo", new PathFragment("bar/bz"));
+ PackageIdentifier bar = PackageIdentifier.create("@bar", new PathFragment("bar/baz"));
assertEquals(0, foo1.compareTo(foo2));
assertThat(foo1.compareTo(foo3)).isLessThan(0);
assertThat(foo1.compareTo(bar)).isGreaterThan(0);
@@ -111,12 +111,12 @@
@Test
public void testInvalidPackageName() throws Exception {
// This shouldn't throw an exception, package names aren't validated.
- new PackageIdentifier("@foo", new PathFragment("bar.baz"));
+ PackageIdentifier.create("@foo", new PathFragment("bar.baz"));
}
@Test
public void testSerialization() throws Exception {
- PackageIdentifier inId = new PackageIdentifier("@foo", new PathFragment("bar/baz"));
+ PackageIdentifier inId = PackageIdentifier.create("@foo", new PathFragment("bar/baz"));
ByteArrayOutputStream data = new ByteArrayOutputStream();
ObjectOutputStream out = new ObjectOutputStream(data);
out.writeObject(inId);
@@ -128,8 +128,8 @@
@Test
public void testPackageFragmentEquality() throws Exception {
// Make sure package fragments are canonicalized.
- PackageIdentifier p1 = new PackageIdentifier("@whatever", new PathFragment("foo/bar"));
- PackageIdentifier p2 = new PackageIdentifier("@whatever", new PathFragment("foo/bar"));
+ PackageIdentifier p1 = PackageIdentifier.create("@whatever", new PathFragment("foo/bar"));
+ PackageIdentifier p2 = PackageIdentifier.create("@whatever", new PathFragment("foo/bar"));
assertSame(p2.getPackageFragment(), p1.getPackageFragment());
}
}
