Copilot commented on code in PR #801:
URL: https://github.com/apache/commons-io/pull/801#discussion_r2439773895
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -132,6 +132,94 @@ public class IOUtils {
// Writer. Each method should take at least one of these as a parameter,
// or return one of them.
+ /**
+ * Holder for per-thread internal scratch buffers.
+ *
+ * <p>Buffers are created lazily and reused within the same thread to
reduce allocation overhead. In the rare case of reentrant access, a temporary
buffer
+ * is allocated to avoid data corruption.</p>
+ *
+ * <p>Typical usage:</p>
+ *
+ * <pre>{@code
+ * final byte[] buffer = ScratchBufferHolder.getScratchByteArray();
+ * try {
+ * // use the buffer
+ * } finally {
+ * ScratchBufferHolder.releaseScratchByteArray(buffer);
+ * }
+ * }</pre>
+ */
+ static final class ScratchBufferHolder {
+
+ /**
+ * Holder for internal byte array buffer.
+ */
+ private static final ThreadLocal<Object[]> SCRATCH_BYTE_BUFFER_HOLDER
= ThreadLocal.withInitial(() -> new Object[] { false, byteArray() });
+
+ /**
+ * Holder for internal char array buffer.
+ */
+ private static final ThreadLocal<Object[]> SCRATCH_CHAR_BUFFER_HOLDER
= ThreadLocal.withInitial(() -> new Object[] { false, charArray() });
+
+
+ /**
+ * Gets the internal byte array buffer.
+ *
+ * @return the internal byte array buffer.
+ */
+ static byte[] getScratchByteArray() {
+ final Object[] holder = SCRATCH_BYTE_BUFFER_HOLDER.get();
+ // If already in use, return a new array
+ if ((boolean) holder[0]) {
+ return byteArray();
+ }
+ holder[0] = true;
+ return (byte[]) holder[1];
+ }
+
+ /**
+ * Gets the char byte array buffer.
+ *
+ * @return the char byte array buffer.
+ */
+ static char[] getScratchCharArray() {
+ final Object[] holder = SCRATCH_CHAR_BUFFER_HOLDER.get();
+ // If already in use, return a new array
+ if ((boolean) holder[0]) {
+ return charArray();
+ }
+ holder[0] = true;
+ return (char[]) holder[1];
+ }
+
+
+ /**
+ * If the argument is the internal byte array, release it for reuse.
+ *
+ * @param array the byte array to release.
+ */
+ static void releaseScratchByteArray(byte[] array) {
+ final Object[] holder = SCRATCH_BYTE_BUFFER_HOLDER.get();
+ if (array == holder[1]) {
+ Arrays.fill(array, (byte) 0);
+ holder[0] = false;
+ }
+ }
+
+ /**
+ * If the argument is the internal char array, release it for reuse.
+ *
+ * @param array the byte array to release.
Review Comment:
The parameter documentation should be 'the char array to release' instead of
'the byte array to release' since this method takes a char array parameter.
```suggestion
* @param array the char array to release.
```
##########
src/test/java/org/apache/commons/io/IOCaseTest.java:
##########
@@ -296,34 +297,50 @@ void test_getName() {
@Test
void test_getScratchByteArray() {
- final byte[] array = IOUtils.getScratchByteArray();
- assert0(array);
- Arrays.fill(array, (byte) 1);
- assert0(IOUtils.getScratchCharArray());
- }
-
- @Test
- void test_getScratchByteArrayWriteOnly() {
- final byte[] array = IOUtils.getScratchByteArrayWriteOnly();
- assert0(array);
- Arrays.fill(array, (byte) 1);
- assert0(IOUtils.getScratchCharArray());
+ final byte[] array = IOUtils.ScratchBufferHolder.getScratchByteArray();
+ try {
+ assert0(array);
+ Arrays.fill(array, (byte) 1);
+ // Get another array, while the first is still in use
+ final byte[] array2 =
IOUtils.ScratchBufferHolder.getScratchByteArray();
+ assert0(array2);
+ assertNotSame(array, array2);
+ } finally {
+ // Release first array
+ IOUtils.ScratchBufferHolder.releaseScratchByteArray(array);
+ }
+ // The first array should be reset and reusable
+ final byte[] array3 =
IOUtils.ScratchBufferHolder.getScratchByteArray();
+ try {
+ assert0(array3);
+ assertSame(array, array3);
+ } finally {
+ IOUtils.ScratchBufferHolder.releaseScratchByteArray(array3);
+ }
}
@Test
void test_getScratchCharArray() {
- final char[] array = IOUtils.getScratchCharArray();
- assert0(array);
- Arrays.fill(array, (char) 1);
- assert0(IOUtils.getScratchCharArray());
- }
-
- @Test
- void test_getScratchCharArrayWriteOnly() {
- final char[] array = IOUtils.getScratchCharArrayWriteOnly();
- assert0(array);
- Arrays.fill(array, (char) 1);
- assert0(IOUtils.getScratchCharArray());
+ final char[] array = IOUtils.ScratchBufferHolder.getScratchCharArray();
+ try {
+ assert0(array);
+ Arrays.fill(array, (char) 1);
+ // Get another array, while the first is still in use
+ final char[] array2 =
IOUtils.ScratchBufferHolder.getScratchCharArray();
+ assert0(array2);
+ assertNotSame(array, array2);
+ } finally {
+ // Release first array
+ IOUtils.ScratchBufferHolder.releaseScratchCharArray(array);
+ }
+ // The first array should be reset and reusable
+ final char[] array3 =
IOUtils.ScratchBufferHolder.getScratchCharArray();
+ try {
+ assert0(array3);
+ assertSame(array, array3);
Review Comment:
[nitpick] Extra space between 'array,' and 'array3' should be removed for
consistent formatting.
```suggestion
assertSame(array, array3);
```
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -132,6 +132,94 @@ public class IOUtils {
// Writer. Each method should take at least one of these as a parameter,
// or return one of them.
+ /**
+ * Holder for per-thread internal scratch buffers.
+ *
+ * <p>Buffers are created lazily and reused within the same thread to
reduce allocation overhead. In the rare case of reentrant access, a temporary
buffer
+ * is allocated to avoid data corruption.</p>
+ *
+ * <p>Typical usage:</p>
+ *
+ * <pre>{@code
+ * final byte[] buffer = ScratchBufferHolder.getScratchByteArray();
+ * try {
+ * // use the buffer
+ * } finally {
+ * ScratchBufferHolder.releaseScratchByteArray(buffer);
+ * }
+ * }</pre>
+ */
+ static final class ScratchBufferHolder {
+
+ /**
+ * Holder for internal byte array buffer.
+ */
+ private static final ThreadLocal<Object[]> SCRATCH_BYTE_BUFFER_HOLDER
= ThreadLocal.withInitial(() -> new Object[] { false, byteArray() });
+
+ /**
+ * Holder for internal char array buffer.
+ */
+ private static final ThreadLocal<Object[]> SCRATCH_CHAR_BUFFER_HOLDER
= ThreadLocal.withInitial(() -> new Object[] { false, charArray() });
+
+
+ /**
+ * Gets the internal byte array buffer.
+ *
+ * @return the internal byte array buffer.
+ */
+ static byte[] getScratchByteArray() {
+ final Object[] holder = SCRATCH_BYTE_BUFFER_HOLDER.get();
+ // If already in use, return a new array
+ if ((boolean) holder[0]) {
+ return byteArray();
+ }
+ holder[0] = true;
+ return (byte[]) holder[1];
+ }
+
+ /**
+ * Gets the char byte array buffer.
+ *
+ * @return the char byte array buffer.
Review Comment:
The comment should be 'Gets the char array buffer' instead of 'Gets the char
byte array buffer' since this method returns a char array, not a byte array.
```suggestion
* Gets the char array buffer.
*
* @return the char array buffer.
```
##########
src/test/java/org/apache/commons/io/IOCaseTest.java:
##########
@@ -296,34 +297,50 @@ void test_getName() {
@Test
void test_getScratchByteArray() {
- final byte[] array = IOUtils.getScratchByteArray();
- assert0(array);
- Arrays.fill(array, (byte) 1);
- assert0(IOUtils.getScratchCharArray());
- }
-
- @Test
- void test_getScratchByteArrayWriteOnly() {
- final byte[] array = IOUtils.getScratchByteArrayWriteOnly();
- assert0(array);
- Arrays.fill(array, (byte) 1);
- assert0(IOUtils.getScratchCharArray());
+ final byte[] array = IOUtils.ScratchBufferHolder.getScratchByteArray();
+ try {
+ assert0(array);
+ Arrays.fill(array, (byte) 1);
+ // Get another array, while the first is still in use
+ final byte[] array2 =
IOUtils.ScratchBufferHolder.getScratchByteArray();
+ assert0(array2);
+ assertNotSame(array, array2);
+ } finally {
+ // Release first array
+ IOUtils.ScratchBufferHolder.releaseScratchByteArray(array);
+ }
+ // The first array should be reset and reusable
+ final byte[] array3 =
IOUtils.ScratchBufferHolder.getScratchByteArray();
+ try {
+ assert0(array3);
+ assertSame(array, array3);
Review Comment:
[nitpick] Extra space between 'array,' and 'array3' should be removed for
consistent formatting.
```suggestion
assertSame(array, array3);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
