From bc1e6311d872a0e808367c373369fd0a3d9aff2f Mon Sep 17 00:00:00 2001 From: Brandon Marc-Aurele Date: Fri, 22 Nov 2024 14:26:22 -0500 Subject: [PATCH 1/4] Landing all conjure-lib API changes ahead of codegen adoption Includes: New conjure collections API for: - unmodifiableList - addAll Optimized add and serialize methods on native containers --- .../palantir/conjure/java/lib/SafeLong.java | 2 +- .../java/lib/internal/ConjureCollections.java | 69 +++++++++++++++++-- .../java/lib/internal/ConjureDoubleList.java | 22 +++++- .../java/lib/internal/ConjureIntegerList.java | 22 +++++- .../lib/internal/ConjureSafeLongList.java | 26 ++++++- 5 files changed, 126 insertions(+), 15 deletions(-) diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java index 4ff79b24e..d70994b38 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java @@ -40,7 +40,7 @@ public long longValue() { return longValue; } - private static long check(long value) { + public static long check(long value) { Preconditions.checkArgument( MIN_SAFE_VALUE <= value && value <= MAX_SAFE_VALUE, "number must be safely representable in javascript i.e. " diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java index 579aaf9b5..1957f4449 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java @@ -19,7 +19,9 @@ import com.palantir.conjure.java.lib.SafeLong; import com.palantir.logsafe.Preconditions; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -37,6 +39,35 @@ private ConjureCollections() { // cannot instantiate } + /* + * This is bizarre. Allow me to explain... + * + * We do _not_ want to expose the Conjure*List types externally + * but we also want the optimizations they provide to make it thru + * to jackson for serialization. So the runtime type needs to be + * preserved while also not exposing the type :phew:. + * + * To achieve this we have to do some gymnastics surrounding the type + * system. We need this to return the type of the list given, but also + * return specific Conjure types when detected. This requires that we + * erase the type info, but we know this is safe because we are directly + * returning the same type which is by definition the identity function. + * Therefore the input List is the same types as the output List. + */ + public static List unmodifiableList(List list) { + // Return the unmodifiable version of the Eclipse types + if (list instanceof ConjureIntegerList) { + return (List) ((ConjureIntegerList) list).asUnmodifiable(); + } else if (list instanceof ConjureDoubleList) { + return (List) ((ConjureDoubleList) list).asUnmodifiable(); + } else if (list instanceof ConjureSafeLongList) { + return (List) ((ConjureSafeLongList) list).asUnmodifiable(); + } else { + // Otherwise use the JDK types + return Collections.unmodifiableList(list); + } + } + @SuppressWarnings("unchecked") public static void addAll(Collection addTo, Iterable elementsToAdd) { Preconditions.checkNotNull(elementsToAdd, "elementsToAdd cannot be null"); @@ -139,6 +170,12 @@ public static Set newNonNullSet(Iterable iterable) { return set; } + /** + * The following Conjure boxed list wrappers for the eclipse-collections [type]ArrayList are temporary (except + * ConjureSafeLongList). In eclipse-collections 12, a BoxedMutable[type]List will be released. Once available, + * Conjure[type]List should be replaced with that. + */ + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static List newNonNullDoubleList() { return new ConjureDoubleList(new DoubleArrayList()); @@ -157,11 +194,14 @@ public static List newNonNullDoubleList(Iterable iterable) { return doubleList; } - /** - * The following Conjure boxed list wrappers for the eclipse-collections [type]ArrayList are temporary (except - * ConjureSafeLongList). In eclipse-collections 12, a BoxedMutable[type]List will be released. Once available, - * Conjure[type]List should be replaced with that. - */ + // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set + public static void addAllToDoubleList(Collection addTo, double[] elementsToAdd) { + if (addTo instanceof ConjureDoubleList) { + ((ConjureDoubleList) addTo).addAll(elementsToAdd); + } else { + addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator()); + } + } // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static List newNonNullIntegerList() { @@ -181,9 +221,19 @@ public static List newNonNullIntegerList(Iterable iterable) { return integerList; } + // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set + public static void addAllToIntegerList(Collection addTo, int[] elementsToAdd) { + if (addTo instanceof ConjureIntegerList) { + ((ConjureIntegerList) addTo).addAll(elementsToAdd); + } else { + addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator()); + } + } + /** * Deprecated, this should only ever be called by a previously generated conjure internal implementation. */ + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static List newNonNullBooleanList() { return newNonNullList(); } @@ -212,4 +262,13 @@ public static List newNonNullSafeLongList(Iterable iterable) return safeLongList; } + + // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set + public static void addAllToSafeLongList(Collection addTo, long[] elementsToAdd) { + if (addTo instanceof ConjureSafeLongList) { + ((ConjureSafeLongList) addTo).addAll(elementsToAdd); + } else { + addAll(addTo, Arrays.stream(elementsToAdd).boxed().map(SafeLong::of).toList()); + } + } } diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java index 58d63bbaa..2e3ac9f5c 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java @@ -16,10 +16,11 @@ package com.palantir.conjure.java.lib.internal; +import com.fasterxml.jackson.annotation.JsonValue; import java.util.AbstractList; import java.util.Collection; import java.util.RandomAccess; -import org.eclipse.collections.impl.list.mutable.primitive.DoubleArrayList; +import org.eclipse.collections.api.list.primitive.MutableDoubleList; import org.eclipse.collections.impl.utility.Iterate; /** @@ -27,9 +28,9 @@ * a BoxedMutableDoubleList will be released. Once available, ConjureDoubleList should be replaced with that. */ final class ConjureDoubleList extends AbstractList implements RandomAccess { - private final DoubleArrayList delegate; + private final MutableDoubleList delegate; - ConjureDoubleList(DoubleArrayList delegate) { + ConjureDoubleList(MutableDoubleList delegate) { this.delegate = delegate; } @@ -55,6 +56,10 @@ public boolean addAll(int index, Collection collection) { return delegate.addAllAtIndex(index, target); } + public void addAll(double... source) { + this.delegate.addAll(source); + } + @Override public Double remove(int index) { return delegate.removeAtIndex(index); @@ -69,4 +74,15 @@ public void clear() { public Double set(int index, Double element) { return delegate.set(index, element); } + + public ConjureDoubleList asUnmodifiable() { + return new ConjureDoubleList(delegate.asUnmodifiable()); + } + + // Cannot be named 'toArray' as that conflicts with the #toArray in AbstractList + // This is a serialization optimization that avoids boxing, but does copy + @JsonValue + double[] jacksonSerialize() { + return delegate.toArray(); + } } diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java index 79ecd32b6..48b476fec 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java @@ -16,10 +16,11 @@ package com.palantir.conjure.java.lib.internal; +import com.fasterxml.jackson.annotation.JsonValue; import java.util.AbstractList; import java.util.Collection; import java.util.RandomAccess; -import org.eclipse.collections.impl.list.mutable.primitive.IntArrayList; +import org.eclipse.collections.api.list.primitive.MutableIntList; import org.eclipse.collections.impl.utility.Iterate; /** @@ -27,9 +28,9 @@ * a BoxedMutableIntList will be released. Once available, ConjureIntegerList should be replaced with that. */ final class ConjureIntegerList extends AbstractList implements RandomAccess { - private final IntArrayList delegate; + private final MutableIntList delegate; - ConjureIntegerList(IntArrayList delegate) { + ConjureIntegerList(MutableIntList delegate) { this.delegate = delegate; } @@ -55,6 +56,10 @@ public boolean addAll(int index, Collection collection) { return delegate.addAllAtIndex(index, target); } + public void addAll(int... source) { + this.delegate.addAll(source); + } + @Override public Integer remove(int index) { return delegate.removeAtIndex(index); @@ -69,4 +74,15 @@ public void clear() { public Integer set(int index, Integer element) { return delegate.set(index, element); } + + public ConjureIntegerList asUnmodifiable() { + return new ConjureIntegerList(delegate.asUnmodifiable()); + } + + // Cannot be named 'toArray' as that conflicts with the #toArray in AbstractList + // This is a serialization optimization that avoids boxing, but does copy + @JsonValue + int[] jacksonSerialize() { + return delegate.toArray(); + } } diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java index 7995d8285..e3d0e50d9 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java @@ -16,11 +16,12 @@ package com.palantir.conjure.java.lib.internal; +import com.fasterxml.jackson.annotation.JsonValue; import com.palantir.conjure.java.lib.SafeLong; import java.util.AbstractList; import java.util.Collection; import java.util.RandomAccess; -import org.eclipse.collections.impl.list.mutable.primitive.LongArrayList; +import org.eclipse.collections.api.list.primitive.MutableLongList; import org.eclipse.collections.impl.utility.Iterate; /** @@ -28,9 +29,9 @@ * with SafeLongs. */ final class ConjureSafeLongList extends AbstractList implements RandomAccess { - private final LongArrayList delegate; + private final MutableLongList delegate; - ConjureSafeLongList(LongArrayList delegate) { + ConjureSafeLongList(MutableLongList delegate) { this.delegate = delegate; } @@ -56,6 +57,14 @@ public boolean addAll(int index, Collection collection) { return delegate.addAllAtIndex(index, target); } + public void addAll(long... source) { + for (long value : source) { + // Doesn't use SafeLong creation because this causes unnecessary boxing + SafeLong.check(value); + } + this.delegate.addAll(source); + } + @Override public SafeLong remove(int index) { return SafeLong.of(delegate.removeAtIndex(index)); @@ -70,4 +79,15 @@ public void clear() { public SafeLong set(int index, SafeLong element) { return SafeLong.of(delegate.set(index, element.longValue())); } + + public ConjureSafeLongList asUnmodifiable() { + return new ConjureSafeLongList(delegate.asUnmodifiable()); + } + + // Cannot be named 'toArray' as that conflicts with the #toArray in AbstractList + // This is a serialization optimization that avoids boxing, but does copy + @JsonValue + long[] jacksonSerialize() { + return delegate.toArray(); + } } From 4efb2d2db9fa8d6bf618e60ada459b88e6e2592b Mon Sep 17 00:00:00 2001 From: Brandon Marc-Aurele Date: Fri, 22 Nov 2024 15:39:01 -0500 Subject: [PATCH 2/4] beginning of CR feedback --- .../palantir/conjure/java/lib/SafeLong.java | 2 +- .../java/lib/internal/ConjureCollections.java | 27 +------------------ .../lib/internal/ConjureSafeLongList.java | 7 ++++- 3 files changed, 8 insertions(+), 28 deletions(-) diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java index d70994b38..4ff79b24e 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java @@ -40,7 +40,7 @@ public long longValue() { return longValue; } - public static long check(long value) { + private static long check(long value) { Preconditions.checkArgument( MIN_SAFE_VALUE <= value && value <= MAX_SAFE_VALUE, "number must be safely representable in javascript i.e. " diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java index 1957f4449..529891bf6 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java @@ -39,33 +39,8 @@ private ConjureCollections() { // cannot instantiate } - /* - * This is bizarre. Allow me to explain... - * - * We do _not_ want to expose the Conjure*List types externally - * but we also want the optimizations they provide to make it thru - * to jackson for serialization. So the runtime type needs to be - * preserved while also not exposing the type :phew:. - * - * To achieve this we have to do some gymnastics surrounding the type - * system. We need this to return the type of the list given, but also - * return specific Conjure types when detected. This requires that we - * erase the type info, but we know this is safe because we are directly - * returning the same type which is by definition the identity function. - * Therefore the input List is the same types as the output List. - */ public static List unmodifiableList(List list) { - // Return the unmodifiable version of the Eclipse types - if (list instanceof ConjureIntegerList) { - return (List) ((ConjureIntegerList) list).asUnmodifiable(); - } else if (list instanceof ConjureDoubleList) { - return (List) ((ConjureDoubleList) list).asUnmodifiable(); - } else if (list instanceof ConjureSafeLongList) { - return (List) ((ConjureSafeLongList) list).asUnmodifiable(); - } else { - // Otherwise use the JDK types - return Collections.unmodifiableList(list); - } + return Collections.unmodifiableList(list); } @SuppressWarnings("unchecked") diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java index e3d0e50d9..03dd07176 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java @@ -18,6 +18,7 @@ import com.fasterxml.jackson.annotation.JsonValue; import com.palantir.conjure.java.lib.SafeLong; +import com.palantir.logsafe.Preconditions; import java.util.AbstractList; import java.util.Collection; import java.util.RandomAccess; @@ -60,7 +61,11 @@ public boolean addAll(int index, Collection collection) { public void addAll(long... source) { for (long value : source) { // Doesn't use SafeLong creation because this causes unnecessary boxing - SafeLong.check(value); + // Mostly copied from SafeLong + Preconditions.checkArgument( + SafeLong.MIN_VALUE.longValue() <= value && value <= SafeLong.MAX_VALUE.longValue(), + "number must be safely representable in javascript i.e. " + + "lie between -9007199254740991 and 9007199254740991"); } this.delegate.addAll(source); } From d70b176fabcea3265e38f64b36e93ab5f3309aa5 Mon Sep 17 00:00:00 2001 From: Brandon Marc-Aurele Date: Fri, 22 Nov 2024 15:46:47 -0500 Subject: [PATCH 3/4] more trimming --- .../java/lib/internal/ConjureCollections.java | 18 ++--------- .../java/lib/internal/ConjureDoubleList.java | 22 ++----------- .../java/lib/internal/ConjureIntegerList.java | 22 ++----------- .../lib/internal/ConjureSafeLongList.java | 31 ++----------------- 4 files changed, 12 insertions(+), 81 deletions(-) diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java index 529891bf6..80870867c 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java @@ -171,11 +171,7 @@ public static List newNonNullDoubleList(Iterable iterable) { // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static void addAllToDoubleList(Collection addTo, double[] elementsToAdd) { - if (addTo instanceof ConjureDoubleList) { - ((ConjureDoubleList) addTo).addAll(elementsToAdd); - } else { - addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator()); - } + addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator()); } // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set @@ -198,11 +194,7 @@ public static List newNonNullIntegerList(Iterable iterable) { // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static void addAllToIntegerList(Collection addTo, int[] elementsToAdd) { - if (addTo instanceof ConjureIntegerList) { - ((ConjureIntegerList) addTo).addAll(elementsToAdd); - } else { - addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator()); - } + addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator()); } /** @@ -240,10 +232,6 @@ public static List newNonNullSafeLongList(Iterable iterable) // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static void addAllToSafeLongList(Collection addTo, long[] elementsToAdd) { - if (addTo instanceof ConjureSafeLongList) { - ((ConjureSafeLongList) addTo).addAll(elementsToAdd); - } else { - addAll(addTo, Arrays.stream(elementsToAdd).boxed().map(SafeLong::of).toList()); - } + addAll(addTo, Arrays.stream(elementsToAdd).boxed().map(SafeLong::of).toList()); } } diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java index 2e3ac9f5c..58d63bbaa 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java @@ -16,11 +16,10 @@ package com.palantir.conjure.java.lib.internal; -import com.fasterxml.jackson.annotation.JsonValue; import java.util.AbstractList; import java.util.Collection; import java.util.RandomAccess; -import org.eclipse.collections.api.list.primitive.MutableDoubleList; +import org.eclipse.collections.impl.list.mutable.primitive.DoubleArrayList; import org.eclipse.collections.impl.utility.Iterate; /** @@ -28,9 +27,9 @@ * a BoxedMutableDoubleList will be released. Once available, ConjureDoubleList should be replaced with that. */ final class ConjureDoubleList extends AbstractList implements RandomAccess { - private final MutableDoubleList delegate; + private final DoubleArrayList delegate; - ConjureDoubleList(MutableDoubleList delegate) { + ConjureDoubleList(DoubleArrayList delegate) { this.delegate = delegate; } @@ -56,10 +55,6 @@ public boolean addAll(int index, Collection collection) { return delegate.addAllAtIndex(index, target); } - public void addAll(double... source) { - this.delegate.addAll(source); - } - @Override public Double remove(int index) { return delegate.removeAtIndex(index); @@ -74,15 +69,4 @@ public void clear() { public Double set(int index, Double element) { return delegate.set(index, element); } - - public ConjureDoubleList asUnmodifiable() { - return new ConjureDoubleList(delegate.asUnmodifiable()); - } - - // Cannot be named 'toArray' as that conflicts with the #toArray in AbstractList - // This is a serialization optimization that avoids boxing, but does copy - @JsonValue - double[] jacksonSerialize() { - return delegate.toArray(); - } } diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java index 48b476fec..79ecd32b6 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java @@ -16,11 +16,10 @@ package com.palantir.conjure.java.lib.internal; -import com.fasterxml.jackson.annotation.JsonValue; import java.util.AbstractList; import java.util.Collection; import java.util.RandomAccess; -import org.eclipse.collections.api.list.primitive.MutableIntList; +import org.eclipse.collections.impl.list.mutable.primitive.IntArrayList; import org.eclipse.collections.impl.utility.Iterate; /** @@ -28,9 +27,9 @@ * a BoxedMutableIntList will be released. Once available, ConjureIntegerList should be replaced with that. */ final class ConjureIntegerList extends AbstractList implements RandomAccess { - private final MutableIntList delegate; + private final IntArrayList delegate; - ConjureIntegerList(MutableIntList delegate) { + ConjureIntegerList(IntArrayList delegate) { this.delegate = delegate; } @@ -56,10 +55,6 @@ public boolean addAll(int index, Collection collection) { return delegate.addAllAtIndex(index, target); } - public void addAll(int... source) { - this.delegate.addAll(source); - } - @Override public Integer remove(int index) { return delegate.removeAtIndex(index); @@ -74,15 +69,4 @@ public void clear() { public Integer set(int index, Integer element) { return delegate.set(index, element); } - - public ConjureIntegerList asUnmodifiable() { - return new ConjureIntegerList(delegate.asUnmodifiable()); - } - - // Cannot be named 'toArray' as that conflicts with the #toArray in AbstractList - // This is a serialization optimization that avoids boxing, but does copy - @JsonValue - int[] jacksonSerialize() { - return delegate.toArray(); - } } diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java index 03dd07176..7995d8285 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java @@ -16,13 +16,11 @@ package com.palantir.conjure.java.lib.internal; -import com.fasterxml.jackson.annotation.JsonValue; import com.palantir.conjure.java.lib.SafeLong; -import com.palantir.logsafe.Preconditions; import java.util.AbstractList; import java.util.Collection; import java.util.RandomAccess; -import org.eclipse.collections.api.list.primitive.MutableLongList; +import org.eclipse.collections.impl.list.mutable.primitive.LongArrayList; import org.eclipse.collections.impl.utility.Iterate; /** @@ -30,9 +28,9 @@ * with SafeLongs. */ final class ConjureSafeLongList extends AbstractList implements RandomAccess { - private final MutableLongList delegate; + private final LongArrayList delegate; - ConjureSafeLongList(MutableLongList delegate) { + ConjureSafeLongList(LongArrayList delegate) { this.delegate = delegate; } @@ -58,18 +56,6 @@ public boolean addAll(int index, Collection collection) { return delegate.addAllAtIndex(index, target); } - public void addAll(long... source) { - for (long value : source) { - // Doesn't use SafeLong creation because this causes unnecessary boxing - // Mostly copied from SafeLong - Preconditions.checkArgument( - SafeLong.MIN_VALUE.longValue() <= value && value <= SafeLong.MAX_VALUE.longValue(), - "number must be safely representable in javascript i.e. " - + "lie between -9007199254740991 and 9007199254740991"); - } - this.delegate.addAll(source); - } - @Override public SafeLong remove(int index) { return SafeLong.of(delegate.removeAtIndex(index)); @@ -84,15 +70,4 @@ public void clear() { public SafeLong set(int index, SafeLong element) { return SafeLong.of(delegate.set(index, element.longValue())); } - - public ConjureSafeLongList asUnmodifiable() { - return new ConjureSafeLongList(delegate.asUnmodifiable()); - } - - // Cannot be named 'toArray' as that conflicts with the #toArray in AbstractList - // This is a serialization optimization that avoids boxing, but does copy - @JsonValue - long[] jacksonSerialize() { - return delegate.toArray(); - } } From 1a4cab210253045de7049993389c4525e5443da0 Mon Sep 17 00:00:00 2001 From: Brandon Marc-Aurele Date: Mon, 25 Nov 2024 12:21:50 -0500 Subject: [PATCH 4/4] address stream overhead feedback --- .../java/lib/internal/ConjureCollections.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java index 80870867c..c8a5a4df8 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java @@ -19,7 +19,6 @@ import com.palantir.conjure.java.lib.SafeLong; import com.palantir.logsafe.Preconditions; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; @@ -171,7 +170,9 @@ public static List newNonNullDoubleList(Iterable iterable) { // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static void addAllToDoubleList(Collection addTo, double[] elementsToAdd) { - addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator()); + for (double el : elementsToAdd) { + addTo.add(el); + } } // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set @@ -194,7 +195,9 @@ public static List newNonNullIntegerList(Iterable iterable) { // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static void addAllToIntegerList(Collection addTo, int[] elementsToAdd) { - addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator()); + for (int el : elementsToAdd) { + addTo.add(el); + } } /** @@ -232,6 +235,8 @@ public static List newNonNullSafeLongList(Iterable iterable) // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static void addAllToSafeLongList(Collection addTo, long[] elementsToAdd) { - addAll(addTo, Arrays.stream(elementsToAdd).boxed().map(SafeLong::of).toList()); + for (long el : elementsToAdd) { + addTo.add(SafeLong.of(el)); + } } }