aherbert commented on code in PR #452:
URL: https://github.com/apache/commons-text/pull/452#discussion_r1300134930
##########
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:
We throw an OOM in the code I adapted from Commons Codec. Disclaimer: I
wrote that code so I am only copying my own precedent. However this is the
behaviour of JDK's StringBuilder. If you try to add to an array such that it
would not fit in a single array you will get an OutOfMemoryError. Since we are
billing the TextStringBuilder as a replacement for that then it seems
appropriate to copy the behaviour.
Regarding recovery by the user. This would be a situation where you are
adding to the StringBuilder and then you get an out-of-memory error. So you
have a situation where a builder cannot create a single String to hold all the
data. The recovery is to either try to process smaller chunks of the full
chars; or accept that you have to truncate the single String. If we start
throwing e.g. IAE here then a user will not be able to use the same try-catch
with our object that they use with a JDK StringBuilder. The catch must be
modified and we lose the drop-in-replacement pattern.
Note: I just updated my extra tests to use JDK's StringBuilder. They pass.
--
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
