Intern PackageIdentifiers as a memory optimization.

--
MOS_MIGRATED_REVID=104403040

11 files changed, 32 insertions, 20 deletions

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 4e799e3d57..f7126ae769 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 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin
     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 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin
     } 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 65f4258b55..991544c0d2 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.CacheBuilder;
 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 @@ import javax.annotation.concurrent.Immutable;
 @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 final class PackageIdentifier implements Comparable<PackageIdentifier>, S
 
   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 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S
   /** 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 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S
       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 233a199ec5..460e20b08d 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 @@ public class GlobCache {
           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 1cd8294e5a..ab58edd603 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 @@ public class PackageDeserializer {
     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 80619e3997..6d6511af55 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 @@ public class ContainingPackageLookupFunction implements SkyFunction {
     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 74e155c89c..db63627be7 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 @@ public final class GlobFunction implements SkyFunction {
     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 @@ public final class GlobFunction implements SkyFunction {
         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 7155768357..73b5f997be 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 @@ public class PackageFunction implements SkyFunction {
     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 @@ public class PackageFunction implements SkyFunction {
     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 9b0e44817b..1927851d40 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 @@ abstract class RecursiveDirectoryTraversalFunction
       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 2c0b615f67..abc44906b9 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 @@ public class SkylarkImportLookupFunction implements SkyFunction {
       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 dd6d3c9eda..dc997a1959 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 @@ public class SkylarkImportLookupValue implements SkyValue {
     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 ffbf874508..3e3fbd5fb5 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 @@ public final class TargetMarkerFunction implements SkyFunction {
       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),