From 48e873abe51484478783e63bc296b3b10519a477 Mon Sep 17 00:00:00 2001 From: Rishabh Kumar Date: Thu, 12 Sep 2024 22:32:19 +0530 Subject: [PATCH] OAK-11112 : added ensure capacity method to collectionutils to create collections with expected capacity --- .../commons/collections/CollectionUtils.java | 30 +++++++++++++++---- .../collections/CollectionUtilsTest.java | 22 +++++++++++++- .../oak/security/user/GroupImpl.java | 2 +- .../oak/security/user/UserImporter.java | 3 +- .../oak/segment/SegmentIdTable.java | 3 +- .../oak/segment/file/tar/GraphLoader.java | 3 +- 6 files changed, 53 insertions(+), 10 deletions(-) diff --git a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java index eebc3ac5eb1..626aefc3a44 100644 --- a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java +++ b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java @@ -34,6 +34,8 @@ */ public class CollectionUtils { + private static final int MAX_CAPACITY = 1 << 30; + private CollectionUtils() { // no instances for you } @@ -131,12 +133,12 @@ public static Set toSet(@NotNull final T... elements) { public static Iterable toIterable(@NotNull final Iterator iterator) { Objects.requireNonNull(iterator); - Iterable delegate = new Iterable() { + return new Iterable<>() { private boolean consumed = false; @Override - public Iterator iterator() { + public @NotNull Iterator iterator() { if (consumed) { throw new IllegalStateException("Iterator already returned once"); } else { @@ -145,8 +147,6 @@ public Iterator iterator() { } } }; - - return delegate; } /** @@ -170,7 +170,27 @@ public static Stream toStream(@NotNull Iterable iterable) { * iterator to convert * @return the stream (representing the remaining elements in the iterator) */ - public static Stream toStream(Iterator iterator) { + @NotNull + public static Stream toStream(@NotNull Iterator iterator) { return StreamSupport.stream(toIterable(iterator).spliterator(), false); } + + /** + * Ensure the capacity of a map or set given the expected number of elements. + * + * @param capacity the expected number of elements + * @return the capacity to use to avoid rehashing & collisions + */ + public static int ensureCapacity(final int capacity) { + + if (capacity < 0) { + throw new IllegalArgumentException("Capacity must be non-negative"); + } + + if (capacity > MAX_CAPACITY) { + return MAX_CAPACITY; + } + + return 1 + (int) (capacity / 0.75f); + } } \ No newline at end of file diff --git a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java index 4722d04d3e1..6b943c02bc1 100644 --- a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java +++ b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java @@ -30,6 +30,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.junit.Assert.fail; + public class CollectionUtilsTest { final List data = Arrays.asList("one", "two", "three", null); @@ -98,7 +100,7 @@ public void iteratorToIIteratable() { Assert.assertFalse(testit.hasNext()); try { testit = iterable.iterator(); - Assert.fail("should only work once"); + fail("should only work once"); } catch (IllegalStateException expected) { // that's what we want } @@ -112,4 +114,22 @@ public void iteratorToStream() { List result = stream.collect(Collectors.toList()); Assert.assertEquals(input.toString(), result.toString()); } + + @Test + public void ensureCapacity() { + int capacity = CollectionUtils.ensureCapacity(8); + Assert.assertEquals(11, capacity); + } + + @Test + public void ensureCapacityWithMaxValue() { + int capacity = CollectionUtils.ensureCapacity(1073741825); + Assert.assertEquals(1073741824, capacity); + } + + @Test(expected = IllegalArgumentException.class) + public void ensureCapacityWithNegativeValue() { + int capacity = CollectionUtils.ensureCapacity(-8); + fail("Should throw IllegalArgumentException"); + } } \ No newline at end of file diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java index 7dc575da12e..dc4dcdfc906 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java @@ -298,7 +298,7 @@ private Set updateMembers(boolean isRemove, @NotNull String... memberIds } // calculate the contentID for each memberId and remember ids that cannot be processed - Map updateMap = new HashMap<>((int)Math.ceil(memberIds.length / 0.75)); + Map updateMap = new HashMap<>(CollectionUtils.ensureCapacity(memberIds.length)); MembershipProvider mp = getMembershipProvider(); for (String memberId : memberIds) { if (Strings.isNullOrEmpty(memberId)) { diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java index 9f1d48816ba..fdcdb554664 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java @@ -33,6 +33,7 @@ import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.commons.collections.CollectionUtils; import org.apache.jackrabbit.oak.namepath.NamePathMapper; import org.apache.jackrabbit.oak.plugins.identifier.IdentifierManager; import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; @@ -657,7 +658,7 @@ void process() throws RepositoryException { @NotNull Map getAuthorizablesToAdd(@NotNull Group gr, @NotNull Map toRemove, @NotNull Map nonExisting) throws RepositoryException { - Map toAdd = new HashMap<>(members.size()); + Map toAdd = new HashMap<>(CollectionUtils.ensureCapacity(members.size())); for (String contentId : members) { // NOTE: no need to check for re-mapped uuids with the referenceTracker because // ImportUUIDBehavior.IMPORT_UUID_CREATE_NEW is not supported for user/group imports (see line 189) diff --git a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentIdTable.java b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentIdTable.java index e8cc33cf6e9..9723e0e741e 100644 --- a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentIdTable.java +++ b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentIdTable.java @@ -30,6 +30,7 @@ import java.util.Set; import java.util.UUID; +import org.apache.jackrabbit.oak.commons.collections.CollectionUtils; import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -130,7 +131,7 @@ void collectReferencedIds(Collection ids) { private synchronized Collection refresh() { int size = references.size(); - Map> ids = new HashMap<>((int)Math.ceil(size / 0.75)); + Map> ids = new HashMap<>(CollectionUtils.ensureCapacity(size)); boolean hashCollisions = false; boolean emptyReferences = false; diff --git a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tar/GraphLoader.java b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tar/GraphLoader.java index d81af3e5fe7..9c4ceaac22b 100644 --- a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tar/GraphLoader.java +++ b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tar/GraphLoader.java @@ -29,6 +29,7 @@ import java.util.zip.CRC32; import org.apache.jackrabbit.oak.commons.Buffer; +import org.apache.jackrabbit.oak.commons.collections.CollectionUtils; import org.apache.jackrabbit.oak.segment.util.ReaderAtEnd; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -93,7 +94,7 @@ public static Buffer loadGraph(ReaderAtEnd readerAtEnd) throws IOException { public static Map> parseGraph(Buffer buffer) { int nEntries = buffer.getInt(buffer.limit() - 12); - Map> graph = new HashMap<>((int)Math.ceil(nEntries / 0.75)); + Map> graph = new HashMap<>(CollectionUtils.ensureCapacity(nEntries)); for (int i = 0; i < nEntries; i++) { long msb = buffer.getLong();