garydgregory commented on code in PR #452:
URL: https://github.com/apache/commons-text/pull/452#discussion_r1300107300


##########
src/main/java/org/apache/commons/text/TextStringBuilder.java:
##########
@@ -1820,17 +1832,84 @@ public boolean endsWith(final String str) {
     /**
      * Tests the capacity and ensures that it is at least the size specified.
      *
+     * <p>Note: This method can be used to minimise memory reallocations during
+     * repeated addition of values by pre-allocating the character buffer.
+     * The method ignores a negative {@code capacity} argument.
+     *
      * @param capacity the capacity to ensure
      * @return this, to enable chaining
+     * @throws OutOfMemoryError if the capacity cannot be allocated
      */
     public TextStringBuilder ensureCapacity(final int capacity) {
-        // checks for overflow
-        if (capacity > 0 && capacity - buffer.length > 0) {
-            reallocate(capacity);
+        if (capacity > 0) {
+            ensureCapacityInternal(capacity);
         }
         return this;
     }
 
+    /**
+     * Ensure that the buffer is at least the size specified. The {@code 
capacity} argument
+     * is treated as an unsigned integer.
+     *
+     * <p>This method will raise an {@link OutOfMemoryError} if the capacity 
is too large
+     * for an array, or cannot be allocated.
+     *
+     * @param capacity the capacity to ensure
+     * @throws OutOfMemoryError if the capacity cannot be allocated
+     */
+    private void ensureCapacityInternal(final int capacity) {
+        // Check for overflow of the current buffer.
+        // Assumes capacity is an unsigned integer up to Integer.MAX_VALUE * 2
+        // (the largest possible addition of two maximum length arrays).
+        if (capacity - buffer.length > 0) {
+            resizeBuffer(capacity);
+        }
+    }
+
+    /**
+     * Resizes the buffer to at least the size specified.
+     *
+     * @param minCapacity the minimum required capacity
+     * @throws OutOfMemoryError if the {@code minCapacity} is negative
+     */
+    private void resizeBuffer(final int minCapacity) {
+        // Overflow-conscious code treats the min and new capacity as unsigned.
+        final int oldCapacity = buffer.length;
+        int newCapacity = oldCapacity * 2;
+        if (Integer.compareUnsigned(newCapacity, minCapacity) < 0) {
+            newCapacity = minCapacity;
+        }
+        if (Integer.compareUnsigned(newCapacity, MAX_BUFFER_SIZE) > 0) {
+            newCapacity = createPositiveCapacity(minCapacity);
+        }
+        reallocate(newCapacity);
+    }
+
+    /**
+     * Create a positive capacity at least as large the minimum required 
capacity.
+     * If the minimum capacity is negative then this throws an 
OutOfMemoryError as no array
+     * can be allocated.
+     *
+     * @param minCapacity the minimum capacity
+     * @return the capacity
+     * @throws OutOfMemoryError if the {@code minCapacity} is negative
+     */
+    private static int createPositiveCapacity(final int minCapacity) {
+        if (minCapacity < 0) {
+            // overflow
+            throw new OutOfMemoryError("Unable to allocate array size: " + 
Integer.toUnsignedString(minCapacity));

Review Comment:
   Hi @aherbert 
   Thank you for this PR :-)
   I think this should be an `IllegalArgumentException` because the JVM has NOT 
in fact run out of memory and a call site up the stack may be able to recover 
from the situation.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to