From 7a39fe0c376aeefe9b6c6ca33d6b8c129cc5b2ae Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 19 Apr 2024 15:46:29 +0100 Subject: [PATCH 1/2] start Interning instances of SlsVersionMatcher --- sls-versions/build.gradle | 1 + .../palantir/sls/versions/SlsVersionMatcher.java | 6 +++++- .../sls/versions/SlsVersionMatcherTest.java | 16 ++++++++++++++++ versions.lock | 10 +++++----- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/sls-versions/build.gradle b/sls-versions/build.gradle index 3bbbb200..c19effe6 100644 --- a/sls-versions/build.gradle +++ b/sls-versions/build.gradle @@ -7,6 +7,7 @@ dependencies { api 'com.palantir.safe-logging:preconditions' api 'com.palantir.safe-logging:safe-logging' implementation 'com.palantir.safe-logging:logger' + implementation 'com.google.guava:guava' annotationProcessor 'org.immutables:value' compileOnly 'org.immutables:value::annotations' diff --git a/sls-versions/src/main/java/com/palantir/sls/versions/SlsVersionMatcher.java b/sls-versions/src/main/java/com/palantir/sls/versions/SlsVersionMatcher.java index bef24950..e4813142 100644 --- a/sls-versions/src/main/java/com/palantir/sls/versions/SlsVersionMatcher.java +++ b/sls-versions/src/main/java/com/palantir/sls/versions/SlsVersionMatcher.java @@ -21,6 +21,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; +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.logger.SafeLogger; @@ -38,6 +40,8 @@ @ImmutablesStyle public abstract class SlsVersionMatcher { + private static final Interner interner = Interners.newWeakInterner(); + private static final SafeLogger log = SafeLoggerFactory.get(SlsVersionMatcher.class); private static final Comparator EMPTY_IS_GREATER = @@ -96,7 +100,7 @@ static Optional maybeCreate( SafeArg.of("matcher", maybeMatcher)); return Optional.empty(); } - return Optional.of(maybeMatcher); + return Optional.of(interner.intern(maybeMatcher)); } /** diff --git a/sls-versions/src/test/java/com/palantir/sls/versions/SlsVersionMatcherTest.java b/sls-versions/src/test/java/com/palantir/sls/versions/SlsVersionMatcherTest.java index 2e7eac16..8dc2b2cf 100644 --- a/sls-versions/src/test/java/com/palantir/sls/versions/SlsVersionMatcherTest.java +++ b/sls-versions/src/test/java/com/palantir/sls/versions/SlsVersionMatcherTest.java @@ -121,6 +121,22 @@ public void testMatcherComparator_comparesNumbers() { assertMatcherOrder(SlsVersionMatcher.valueOf("2.6.5"), SlsVersionMatcher.valueOf("2.6.6")); } + @Test + void interning_works() { + SlsVersionMatcher instance1 = matcher("1.x.x"); + SlsVersionMatcher instance2 = matcher("1.x.x"); + assertThat(instance1).isSameAs(instance2); + } + + @Test + void surprising_interning_consequences() { + SlsVersionMatcher instance1 = matcher("01.x.x"); + SlsVersionMatcher instance2 = matcher("1.x.x"); + assertThat(instance1).isEqualTo(instance2); + assertThat(instance1).isSameAs(instance2); + assertThat(instance2).hasToString("01.x.x"); + } + private static void assertMatcherOrder(SlsVersionMatcher smaller, SlsVersionMatcher larger) { assertThat(SlsVersionMatcher.MATCHER_COMPARATOR.compare(smaller, larger)) .isLessThan(0); diff --git a/versions.lock b/versions.lock index ba910660..ee0f8687 100644 --- a/versions.lock +++ b/versions.lock @@ -2,11 +2,16 @@ com.fasterxml.jackson.core:jackson-annotations:2.16.1 (2 constraints: c517c771) com.google.code.findbugs:jsr305:3.0.2 (1 constraints: 170aecb4) com.google.errorprone:error_prone_annotations:2.26.1 (5 constraints: 9b497093) +com.google.guava:failureaccess:1.0.2 (1 constraints: 150ae2b4) +com.google.guava:guava:33.1.0-jre (1 constraints: a7066453) +com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava (1 constraints: bd17c918) +com.google.j2objc:j2objc-annotations:3.0.0 (1 constraints: 150aeab4) com.palantir.safe-logging:logger:3.7.0 (1 constraints: 0c050f36) com.palantir.safe-logging:logger-slf4j:3.7.0 (1 constraints: 050e6842) com.palantir.safe-logging:logger-spi:3.7.0 (2 constraints: 191ea27b) com.palantir.safe-logging:preconditions:3.7.0 (1 constraints: 0c050f36) com.palantir.safe-logging:safe-logging:3.7.0 (4 constraints: 92339760) +org.checkerframework:checker-qual:3.42.0 (1 constraints: 4b0a47bf) org.immutables:value:2.10.1 (1 constraints: 3605303b) org.jetbrains:annotations:24.1.0 (1 constraints: 331166d1) org.slf4j:slf4j-api:1.7.36 (2 constraints: 2321cfd3) @@ -14,10 +19,6 @@ org.slf4j:slf4j-api:1.7.36 (2 constraints: 2321cfd3) [Test dependencies] com.fasterxml.jackson.core:jackson-core:2.16.1 (1 constraints: 8a123f21) com.fasterxml.jackson.core:jackson-databind:2.16.1 (1 constraints: 3c05423b) -com.google.guava:failureaccess:1.0.2 (1 constraints: 150ae2b4) -com.google.guava:guava:33.1.0-jre (1 constraints: a7066453) -com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava (1 constraints: bd17c918) -com.google.j2objc:j2objc-annotations:3.0.0 (1 constraints: 150aeab4) net.bytebuddy:byte-buddy:1.14.11 (1 constraints: 7e0bc5ea) net.jqwik:jqwik:1.3.7 (1 constraints: 0d050036) net.jqwik:jqwik-api:1.3.7 (2 constraints: de117b11) @@ -29,7 +30,6 @@ org.apache.logging.log4j:log4j-core:2.23.1 (1 constraints: d41002df) org.apache.logging.log4j:log4j-slf4j-impl:2.23.1 (1 constraints: 3a053d3b) org.apiguardian:apiguardian-api:1.1.2 (8 constraints: d66ee506) org.assertj:assertj-core:3.25.3 (1 constraints: 3f054b3b) -org.checkerframework:checker-qual:3.42.0 (1 constraints: 4b0a47bf) org.junit.jupiter:junit-jupiter:5.10.2 (1 constraints: 3a05433b) org.junit.jupiter:junit-jupiter-api:5.10.2 (3 constraints: f12f0e51) org.junit.jupiter:junit-jupiter-engine:5.10.2 (1 constraints: 350eff49) From 9c80a755a39ac17b4b9463b0d1a032781e86d30c Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 19 Apr 2024 15:56:39 +0100 Subject: [PATCH 2/2] omg its so sneaky --- .../com/palantir/sls/versions/SlsVersionMatcherTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sls-versions/src/test/java/com/palantir/sls/versions/SlsVersionMatcherTest.java b/sls-versions/src/test/java/com/palantir/sls/versions/SlsVersionMatcherTest.java index 8dc2b2cf..0c68c1a1 100644 --- a/sls-versions/src/test/java/com/palantir/sls/versions/SlsVersionMatcherTest.java +++ b/sls-versions/src/test/java/com/palantir/sls/versions/SlsVersionMatcherTest.java @@ -130,11 +130,11 @@ void interning_works() { @Test void surprising_interning_consequences() { - SlsVersionMatcher instance1 = matcher("01.x.x"); - SlsVersionMatcher instance2 = matcher("1.x.x"); + SlsVersionMatcher instance1 = matcher("09876543.x.x"); + SlsVersionMatcher instance2 = matcher("9876543.x.x"); assertThat(instance1).isEqualTo(instance2); assertThat(instance1).isSameAs(instance2); - assertThat(instance2).hasToString("01.x.x"); + assertThat(instance2).hasToString("09876543.x.x"); } private static void assertMatcherOrder(SlsVersionMatcher smaller, SlsVersionMatcher larger) {