Weakly intern normalized OrderableSlsVersion instances

changelog/@unreleased/pr-720.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Weakly intern normalized OrderableSlsVersion instances
+  links:
+  - https://github.com/palantir/sls-version-java/pull/720

sls-versions/build.gradle

@@ -6,6 +6,8 @@ dependencies {
     api 'com.fasterxml.jackson.core:jackson-annotations'
     api 'com.palantir.safe-logging:preconditions'
     api 'com.palantir.safe-logging:safe-logging'
+
+    implementation 'com.google.guava:guava'
     implementation 'com.palantir.safe-logging:logger'
 
     annotationProcessor 'org.immutables:value'

sls-versions/src/main/java/com/palantir/sls/versions/OrderableSlsVersion.java

@@ -19,7 +19,11 @@
 import static com.palantir.logsafe.Preconditions.checkArgument;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
+import com.google.common.collect.Interner;
+import com.google.common.collect.Interners;
+import com.palantir.logsafe.SafeArg;
 import com.palantir.logsafe.UnsafeArg;
+import com.palantir.logsafe.exceptions.SafeIllegalStateException;
 import java.util.Optional;
 import org.immutables.value.Value;
 
@@ -31,6 +35,8 @@
 @ImmutablesStyle
 public abstract class OrderableSlsVersion extends SlsVersion implements Comparable<OrderableSlsVersion> {
 
+    private static final Interner<OrderableSlsVersion> interner = Interners.newWeakInterner();
+
     private static final SlsVersionType[] ORDERED_VERSION_TYPES = {
         SlsVersionType.RELEASE,
         SlsVersionType.RELEASE_CANDIDATE,
@@ -76,7 +82,37 @@ private static OrderableSlsVersion construct(SlsVersionType type, String value,
             orderableSlsVersion.secondSequenceVersionNumber(groups.groupAsInt(5));
         }
 
-        return orderableSlsVersion.build();
+        OrderableSlsVersion version = orderableSlsVersion.build();
+        if (version.isNormalized()) {
+            // only intern fully normalized values where there are no leading zeros or git commits
+            return interner.intern(version);
+        }
+        return version;
+    }
+
+    private boolean isNormalized() {
+        return getValue().equals(normalizedVersionString());
+    }
+
+    private String normalizedVersionString() {
+        switch (getType()) {
+            case RELEASE:
+                return getMajorVersionNumber() + "." + getMinorVersionNumber() + '.' + getPatchVersionNumber();
+            case RELEASE_CANDIDATE:
+                return getMajorVersionNumber() + "." + getMinorVersionNumber() + '.' + getPatchVersionNumber() + "-rc"
+                        + firstSequenceVersionNumber().orElseThrow();
+            case RELEASE_CANDIDATE_SNAPSHOT:
+                return getMajorVersionNumber() + "." + getMinorVersionNumber() + '.' + getPatchVersionNumber() + "-rc"
+                        + firstSequenceVersionNumber().orElseThrow()
+                        + "-" + secondSequenceVersionNumber().orElseThrow();
+            case RELEASE_SNAPSHOT:
+                return getMajorVersionNumber() + "." + getMinorVersionNumber() + '.' + getPatchVersionNumber() + '-'
+                        + firstSequenceVersionNumber().orElseThrow();
+            case NON_ORDERABLE:
+            default:
+                throw new SafeIllegalStateException(
+                        "Invalid type", SafeArg.of("type", getType()), SafeArg.of("version", this));
+        }
     }
 
     /** Returns true iff the given coordinate has a version which can be parsed into a valid orderable SLS version. */

sls-versions/src/main/java/com/palantir/sls/versions/VersionComparator.java

@@ -16,36 +16,56 @@
 
 package com.palantir.sls.versions;
 
-import static com.palantir.logsafe.Preconditions.checkArgument;
-
 import com.palantir.logsafe.SafeArg;
+import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
 import java.util.Comparator;
-import java.util.OptionalInt;
 
 /** Compares {@link OrderableSlsVersion}s by "newness", i.e., "1.4.0" is greater/newer/later than "1.2.1", etc.. */
 public enum VersionComparator implements Comparator<OrderableSlsVersion> {
     INSTANCE;
 
+    private static final Comparator<OrderableSlsVersion> majorMinorPatchComparator = Comparator.comparing(
+                    OrderableSlsVersion::getMajorVersionNumber)
+            .thenComparing(OrderableSlsVersion::getMinorVersionNumber)
+            .thenComparing(OrderableSlsVersion::getPatchVersionNumber);
+
+    private static final Comparator<OrderableSlsVersion> typePriority =
+            Comparator.comparingInt(v -> v.getType().getPriority());
+    private static final Comparator<OrderableSlsVersion> firstSequence =
+            Comparator.comparingInt(v -> v.firstSequenceVersionNumber()
+                    .orElseThrow(() -> new SafeIllegalArgumentException(
+                            "Expected to find a first sequence number for version",
+                            SafeArg.of("version", v.getValue()))));
+
+    private static final Comparator<OrderableSlsVersion> secondSequence =
+            // Substitute -1 if not present because snapshots are greater than non-snapshots.
+            Comparator.comparingInt(v -> v.secondSequenceVersionNumber().orElse(-1));
+
+    public static Comparator<OrderableSlsVersion> majorMinorPatch() {
+        return majorMinorPatchComparator;
+    }
+
     @Override
     public int compare(OrderableSlsVersion left, OrderableSlsVersion right) {
         if (left.getValue().equals(right.getValue())) {
             return 0;
         }
 
-        int mainVersionComparison = compareMainVersion(left, right);
+        int mainVersionComparison = majorMinorPatch().compare(left, right);
         if (mainVersionComparison != 0) {
             return mainVersionComparison;
         }
 
         if ((left.getType() == SlsVersionType.RELEASE) || (right.getType() == SlsVersionType.RELEASE)) {
             // Releases always compare correctly just by type now we know base version matches.
-            return Integer.compare(left.getType().getPriority(), right.getType().getPriority());
+            return typePriority.compare(left, right);
         }
 
         if ((left.getType() == SlsVersionType.RELEASE_SNAPSHOT)
                 && (right.getType() == SlsVersionType.RELEASE_SNAPSHOT)) {
             // If both are snapshots, compare snapshot number.
-            return compareFirstSequenceVersions(left, right);
+
+            return firstSequence.compare(left, right);
         }
 
         if (left.getType() == SlsVersionType.RELEASE_SNAPSHOT) {
@@ -61,50 +81,17 @@ public int compare(OrderableSlsVersion left, OrderableSlsVersion right) {
         return compareSuffix(left, right);
     }
 
-    private int compareMainVersion(OrderableSlsVersion left, OrderableSlsVersion right) {
-        if (left.getMajorVersionNumber() != right.getMajorVersionNumber()) {
-            return left.getMajorVersionNumber() > right.getMajorVersionNumber() ? 1 : -1;
-        }
-
-        if (left.getMinorVersionNumber() != right.getMinorVersionNumber()) {
-            return left.getMinorVersionNumber() > right.getMinorVersionNumber() ? 1 : -1;
-        }
-
-        if (left.getPatchVersionNumber() != right.getPatchVersionNumber()) {
-            return left.getPatchVersionNumber() > right.getPatchVersionNumber() ? 1 : -1;
-        }
-
-        return 0;
-    }
-
     private int compareSuffix(OrderableSlsVersion left, OrderableSlsVersion right) {
         // We know by this point that both are RCs or RC-snapshots.
         // Compare RC number first.
-        int rcCompare = compareFirstSequenceVersions(left, right);
+
+        int rcCompare = firstSequence.compare(left, right);
         if (rcCompare != 0) {
             return rcCompare;
         }
 
         // RC number is the same, compare snapshot versions.
         // Substitute -1 if not present because snapshots are greater than non-snapshots.
-        OptionalInt leftInt = left.secondSequenceVersionNumber();
-        OptionalInt rightInt = right.secondSequenceVersionNumber();
-        return Integer.compare(leftInt.orElse(-1), rightInt.orElse(-1));
-    }
-
-    private int compareFirstSequenceVersions(OrderableSlsVersion left, OrderableSlsVersion right) {
-        OptionalInt leftInt = left.firstSequenceVersionNumber();
-        OptionalInt rightInt = right.firstSequenceVersionNumber();
-
-        checkArgument(
-                leftInt.isPresent(),
-                "Expected to find a first sequence number for version",
-                SafeArg.of("version", left.getValue()));
-        checkArgument(
-                rightInt.isPresent(),
-                "Expected to find a first sequence number for version",
-                SafeArg.of("version", right.getValue()));
-
-        return Integer.compare(leftInt.getAsInt(), rightInt.getAsInt());
+        return secondSequence.compare(left, right);
     }
 }