From e7a024c846ae1732283bf57802dd3b03a45a0ec2 Mon Sep 17 00:00:00 2001 From: Brandon Marc-Aurele Date: Tue, 5 Nov 2024 13:51:35 -0500 Subject: [PATCH] Remove Primitive Optimization for Boolean Collections (#2390) --- changelog/@unreleased/pr-2390.v2.yml | 5 ++ .../com/palantir/product/ListExample.java | 50 +++++++++++-- .../com/palantir/product/ListExample.java | 49 +++++++++++-- .../java/types/BeanBuilderGenerator.java | 5 +- .../src/test/resources/example-types.yml | 1 + .../java/lib/internal/ConjureBooleanList.java | 72 ------------------- .../java/lib/internal/ConjureCollections.java | 21 +++--- 7 files changed, 107 insertions(+), 96 deletions(-) create mode 100644 changelog/@unreleased/pr-2390.v2.yml delete mode 100644 conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureBooleanList.java diff --git a/changelog/@unreleased/pr-2390.v2.yml b/changelog/@unreleased/pr-2390.v2.yml new file mode 100644 index 000000000..8f0f89c69 --- /dev/null +++ b/changelog/@unreleased/pr-2390.v2.yml @@ -0,0 +1,5 @@ +type: deprecation +deprecation: + description: Remove Primitive Optimization for Boolean Collections + links: + - https://github.com/palantir/conjure-java/pull/2390 diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java index 2a3c0d33f..885402021 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java @@ -27,6 +27,8 @@ public final class ListExample { private final List doubleItems; + private final List booleanItems; + private final List> optionalItems; private final List aliasOptionalItems; @@ -39,13 +41,16 @@ private ListExample( List items, List primitiveItems, List doubleItems, + List booleanItems, List> optionalItems, List aliasOptionalItems, List> nestedItems) { - validateFields(items, primitiveItems, doubleItems, optionalItems, aliasOptionalItems, nestedItems); + validateFields( + items, primitiveItems, doubleItems, booleanItems, optionalItems, aliasOptionalItems, nestedItems); this.items = Collections.unmodifiableList(items); this.primitiveItems = Collections.unmodifiableList(primitiveItems); this.doubleItems = Collections.unmodifiableList(doubleItems); + this.booleanItems = Collections.unmodifiableList(booleanItems); this.optionalItems = Collections.unmodifiableList(optionalItems); this.aliasOptionalItems = Collections.unmodifiableList(aliasOptionalItems); this.nestedItems = Collections.unmodifiableList(nestedItems); @@ -67,6 +72,11 @@ public List getDoubleItems() { return this.doubleItems; } + @JsonProperty("booleanItems") + public List getBooleanItems() { + return this.booleanItems; + } + @JsonProperty("optionalItems") public List> getOptionalItems() { return this.optionalItems; @@ -96,6 +106,7 @@ private boolean equalTo(ListExample other) { return this.items.equals(other.items) && this.primitiveItems.equals(other.primitiveItems) && this.doubleItems.equals(other.doubleItems) + && this.booleanItems.equals(other.booleanItems) && this.optionalItems.equals(other.optionalItems) && this.aliasOptionalItems.equals(other.aliasOptionalItems) && this.nestedItems.equals(other.nestedItems); @@ -109,6 +120,7 @@ public int hashCode() { hash = 31 * hash + this.items.hashCode(); hash = 31 * hash + this.primitiveItems.hashCode(); hash = 31 * hash + this.doubleItems.hashCode(); + hash = 31 * hash + this.booleanItems.hashCode(); hash = 31 * hash + this.optionalItems.hashCode(); hash = 31 * hash + this.aliasOptionalItems.hashCode(); hash = 31 * hash + this.nestedItems.hashCode(); @@ -121,14 +133,15 @@ public int hashCode() { @Override public String toString() { return "ListExample{items: " + items + ", primitiveItems: " + primitiveItems + ", doubleItems: " + doubleItems - + ", optionalItems: " + optionalItems + ", aliasOptionalItems: " + aliasOptionalItems - + ", nestedItems: " + nestedItems + '}'; + + ", booleanItems: " + booleanItems + ", optionalItems: " + optionalItems + ", aliasOptionalItems: " + + aliasOptionalItems + ", nestedItems: " + nestedItems + '}'; } private static void validateFields( List items, List primitiveItems, List doubleItems, + List booleanItems, List> optionalItems, List aliasOptionalItems, List> nestedItems) { @@ -136,6 +149,7 @@ private static void validateFields( missingFields = addFieldIfMissing(missingFields, items, "items"); missingFields = addFieldIfMissing(missingFields, primitiveItems, "primitiveItems"); missingFields = addFieldIfMissing(missingFields, doubleItems, "doubleItems"); + missingFields = addFieldIfMissing(missingFields, booleanItems, "booleanItems"); missingFields = addFieldIfMissing(missingFields, optionalItems, "optionalItems"); missingFields = addFieldIfMissing(missingFields, aliasOptionalItems, "aliasOptionalItems"); missingFields = addFieldIfMissing(missingFields, nestedItems, "nestedItems"); @@ -149,7 +163,7 @@ private static List addFieldIfMissing(List prev, Object fieldVal List missingFields = prev; if (fieldValue == null) { if (missingFields == null) { - missingFields = new ArrayList<>(6); + missingFields = new ArrayList<>(7); } missingFields.add(fieldName); } @@ -170,6 +184,8 @@ public static final class Builder { private List doubleItems = ConjureCollections.newNonNullDoubleList(); + private List booleanItems = ConjureCollections.newNonNullList(); + private List> optionalItems = ConjureCollections.newNonNullList(); private List aliasOptionalItems = ConjureCollections.newNonNullList(); @@ -183,6 +199,7 @@ public Builder from(ListExample other) { items(other.getItems()); primitiveItems(other.getPrimitiveItems()); doubleItems(other.getDoubleItems()); + booleanItems(other.getBooleanItems()); optionalItems(other.getOptionalItems()); aliasOptionalItems(other.getAliasOptionalItems()); nestedItems(other.getNestedItems()); @@ -254,6 +271,28 @@ public Builder doubleItems(double doubleItems) { return this; } + @JsonSetter(value = "booleanItems", nulls = Nulls.SKIP, contentNulls = Nulls.FAIL) + public Builder booleanItems(@Nonnull Iterable booleanItems) { + checkNotBuilt(); + this.booleanItems = ConjureCollections.newNonNullList( + Preconditions.checkNotNull(booleanItems, "booleanItems cannot be null")); + return this; + } + + public Builder addAllBooleanItems(@Nonnull Iterable booleanItems) { + checkNotBuilt(); + ConjureCollections.addAllAndCheckNonNull( + this.booleanItems, Preconditions.checkNotNull(booleanItems, "booleanItems cannot be null")); + return this; + } + + public Builder booleanItems(boolean booleanItems) { + checkNotBuilt(); + Preconditions.checkNotNull(booleanItems, "booleanItems cannot be null"); + this.booleanItems.add(booleanItems); + return this; + } + @JsonSetter(value = "optionalItems", nulls = Nulls.SKIP, contentNulls = Nulls.AS_EMPTY) public Builder optionalItems(@Nonnull Iterable> optionalItems) { checkNotBuilt(); @@ -325,7 +364,8 @@ public Builder nestedItems(List nestedItems) { public ListExample build() { checkNotBuilt(); this._buildInvoked = true; - return new ListExample(items, primitiveItems, doubleItems, optionalItems, aliasOptionalItems, nestedItems); + return new ListExample( + items, primitiveItems, doubleItems, booleanItems, optionalItems, aliasOptionalItems, nestedItems); } private void checkNotBuilt() { diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ListExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ListExample.java index e1dcd4a2d..21a972e86 100644 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ListExample.java +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ListExample.java @@ -28,6 +28,8 @@ public final class ListExample { private final List doubleItems; + private final List booleanItems; + private final List> optionalItems; private final List aliasOptionalItems; @@ -40,13 +42,16 @@ private ListExample( List items, List primitiveItems, List doubleItems, + List booleanItems, List> optionalItems, List aliasOptionalItems, List> nestedItems) { - validateFields(items, primitiveItems, doubleItems, optionalItems, aliasOptionalItems, nestedItems); + validateFields( + items, primitiveItems, doubleItems, booleanItems, optionalItems, aliasOptionalItems, nestedItems); this.items = Collections.unmodifiableList(items); this.primitiveItems = Collections.unmodifiableList(primitiveItems); this.doubleItems = Collections.unmodifiableList(doubleItems); + this.booleanItems = Collections.unmodifiableList(booleanItems); this.optionalItems = Collections.unmodifiableList(optionalItems); this.aliasOptionalItems = Collections.unmodifiableList(aliasOptionalItems); this.nestedItems = Collections.unmodifiableList(nestedItems); @@ -68,6 +73,11 @@ public List getDoubleItems() { return this.doubleItems; } + @JsonProperty("booleanItems") + public List getBooleanItems() { + return this.booleanItems; + } + @JsonProperty("optionalItems") public List> getOptionalItems() { return this.optionalItems; @@ -97,6 +107,7 @@ private boolean equalTo(ListExample other) { return this.items.equals(other.items) && this.primitiveItems.equals(other.primitiveItems) && this.doubleItems.equals(other.doubleItems) + && this.booleanItems.equals(other.booleanItems) && this.optionalItems.equals(other.optionalItems) && this.aliasOptionalItems.equals(other.aliasOptionalItems) && this.nestedItems.equals(other.nestedItems); @@ -110,6 +121,7 @@ public int hashCode() { hash = 31 * hash + this.items.hashCode(); hash = 31 * hash + this.primitiveItems.hashCode(); hash = 31 * hash + this.doubleItems.hashCode(); + hash = 31 * hash + this.booleanItems.hashCode(); hash = 31 * hash + this.optionalItems.hashCode(); hash = 31 * hash + this.aliasOptionalItems.hashCode(); hash = 31 * hash + this.nestedItems.hashCode(); @@ -122,14 +134,15 @@ public int hashCode() { @Override public String toString() { return "ListExample{items: " + items + ", primitiveItems: " + primitiveItems + ", doubleItems: " + doubleItems - + ", optionalItems: " + optionalItems + ", aliasOptionalItems: " + aliasOptionalItems - + ", nestedItems: " + nestedItems + '}'; + + ", booleanItems: " + booleanItems + ", optionalItems: " + optionalItems + ", aliasOptionalItems: " + + aliasOptionalItems + ", nestedItems: " + nestedItems + '}'; } private static void validateFields( List items, List primitiveItems, List doubleItems, + List booleanItems, List> optionalItems, List aliasOptionalItems, List> nestedItems) { @@ -137,6 +150,7 @@ private static void validateFields( missingFields = addFieldIfMissing(missingFields, items, "items"); missingFields = addFieldIfMissing(missingFields, primitiveItems, "primitiveItems"); missingFields = addFieldIfMissing(missingFields, doubleItems, "doubleItems"); + missingFields = addFieldIfMissing(missingFields, booleanItems, "booleanItems"); missingFields = addFieldIfMissing(missingFields, optionalItems, "optionalItems"); missingFields = addFieldIfMissing(missingFields, aliasOptionalItems, "aliasOptionalItems"); missingFields = addFieldIfMissing(missingFields, nestedItems, "nestedItems"); @@ -150,7 +164,7 @@ private static List addFieldIfMissing(List prev, Object fieldVal List missingFields = prev; if (fieldValue == null) { if (missingFields == null) { - missingFields = new ArrayList<>(6); + missingFields = new ArrayList<>(7); } missingFields.add(fieldName); } @@ -172,6 +186,8 @@ public static final class Builder { private List doubleItems = ConjureCollections.newList(); + private List booleanItems = ConjureCollections.newList(); + private List> optionalItems = ConjureCollections.newList(); private List aliasOptionalItems = ConjureCollections.newList(); @@ -185,6 +201,7 @@ public Builder from(ListExample other) { items(other.getItems()); primitiveItems(other.getPrimitiveItems()); doubleItems(other.getDoubleItems()); + booleanItems(other.getBooleanItems()); optionalItems(other.getOptionalItems()); aliasOptionalItems(other.getAliasOptionalItems()); nestedItems(other.getNestedItems()); @@ -252,6 +269,27 @@ public Builder doubleItems(double doubleItems) { return this; } + @JsonSetter(value = "booleanItems", nulls = Nulls.SKIP) + public Builder booleanItems(@Nonnull Iterable booleanItems) { + checkNotBuilt(); + this.booleanItems = + ConjureCollections.newList(Preconditions.checkNotNull(booleanItems, "booleanItems cannot be null")); + return this; + } + + public Builder addAllBooleanItems(@Nonnull Iterable booleanItems) { + checkNotBuilt(); + ConjureCollections.addAll( + this.booleanItems, Preconditions.checkNotNull(booleanItems, "booleanItems cannot be null")); + return this; + } + + public Builder booleanItems(boolean booleanItems) { + checkNotBuilt(); + this.booleanItems.add(booleanItems); + return this; + } + @JsonSetter(value = "optionalItems", nulls = Nulls.SKIP, contentNulls = Nulls.AS_EMPTY) public Builder optionalItems(@Nonnull Iterable> optionalItems) { checkNotBuilt(); @@ -320,7 +358,8 @@ public Builder nestedItems(List nestedItems) { public ListExample build() { checkNotBuilt(); this._buildInvoked = true; - return new ListExample(items, primitiveItems, doubleItems, optionalItems, aliasOptionalItems, nestedItems); + return new ListExample( + items, primitiveItems, doubleItems, booleanItems, optionalItems, aliasOptionalItems, nestedItems); } private void checkNotBuilt() { diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java index d60bb1edf..e791a2298 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java @@ -1044,7 +1044,10 @@ private enum ConjureCollectionType { LIST("List"), DOUBLE_LIST("DoubleList"), INTEGER_LIST("IntegerList"), - BOOLEAN_LIST("BooleanList"), + // Eclipse has a BooleanList type, but this use case implies + // bit mask and it doesn't serialize efficiently as a collection + // so let's just use the "naive" boxed collection + BOOLEAN_LIST("List"), SAFE_LONG_LIST("SafeLongList"), SET("Set"); diff --git a/conjure-java-core/src/test/resources/example-types.yml b/conjure-java-core/src/test/resources/example-types.yml index 932ed26d4..6a09f8d85 100644 --- a/conjure-java-core/src/test/resources/example-types.yml +++ b/conjure-java-core/src/test/resources/example-types.yml @@ -74,6 +74,7 @@ types: safety: safe primitiveItems: list doubleItems: list + booleanItems: list optionalItems: list> aliasOptionalItems: list nestedItems: list> diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureBooleanList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureBooleanList.java deleted file mode 100644 index 6ed117088..000000000 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureBooleanList.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.palantir.conjure.java.lib.internal; - -import java.util.AbstractList; -import java.util.Collection; -import java.util.RandomAccess; -import org.eclipse.collections.impl.list.mutable.primitive.BooleanArrayList; -import org.eclipse.collections.impl.utility.Iterate; - -/** - * ConjureBooleanList is a boxed list wrapper for the eclipse-collections BooleanArrayList. In eclipse-collections 12, - * a BoxedMutableBooleanList will be released. Once available, ConjureBooleanList should be replaced with that. - */ -final class ConjureBooleanList extends AbstractList implements RandomAccess { - private final BooleanArrayList delegate; - - ConjureBooleanList(BooleanArrayList delegate) { - this.delegate = delegate; - } - - @Override - public int size() { - return delegate.size(); - } - - @Override - public void add(int index, Boolean toAdd) { - delegate.addAtIndex(index, toAdd); - } - - @Override - public Boolean get(int index) { - return delegate.get(index); - } - - @Override - public boolean addAll(int index, Collection collection) { - boolean[] target = new boolean[collection.size()]; - Iterate.forEachWithIndex(collection, (each, parameter) -> target[parameter] = each.booleanValue()); - return delegate.addAllAtIndex(index, target); - } - - @Override - public Boolean remove(int index) { - return delegate.removeAtIndex(index); - } - - @Override - public void clear() { - delegate.clear(); - } - - @Override - public Boolean set(int index, Boolean element) { - return delegate.set(index, element); - } -} 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 92b3b7b81..579aaf9b5 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 @@ -23,7 +23,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Set; -import org.eclipse.collections.impl.list.mutable.primitive.BooleanArrayList; import org.eclipse.collections.impl.list.mutable.primitive.DoubleArrayList; import org.eclipse.collections.impl.list.mutable.primitive.IntArrayList; import org.eclipse.collections.impl.list.mutable.primitive.LongArrayList; @@ -182,22 +181,18 @@ public static List newNonNullIntegerList(Iterable iterable) { return integerList; } - // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set + /** + * Deprecated, this should only ever be called by a previously generated conjure internal implementation. + */ public static List newNonNullBooleanList() { - return new ConjureBooleanList(new BooleanArrayList()); + return newNonNullList(); } - // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set + /** + * Deprecated, this should only ever be called by a previously generated conjure internal implementation. + */ public static List newNonNullBooleanList(Iterable iterable) { - List booleanList; - if (iterable instanceof Collection) { - booleanList = new ConjureBooleanList(new BooleanArrayList(((Collection) iterable).size())); - } else { - booleanList = new ConjureBooleanList(new BooleanArrayList()); - } - addAll(booleanList, iterable); - - return booleanList; + return newNonNullList(iterable); } // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set