This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-io.git
The following commit(s) were added to refs/heads/master by this push:
new c12eaff7 Don't use thread local buffers for write only operations
c12eaff7 is described below
commit c12eaff7f747353a7a9a97df735fd3301f42e313
Author: Gary Gregory <[email protected]>
AuthorDate: Fri May 12 10:37:49 2023 -0400
Don't use thread local buffers for write only operations
- Use a shared static byte array for write only throw away data
- Keep using thread locals for read-write buffers
- Allow tests to clear thread locals to ensure proper re-initialization
---
src/main/java/org/apache/commons/io/IO.java | 31 +++++++++
src/main/java/org/apache/commons/io/IOUtils.java | 80 +++++++++++++++++-----
.../java/org/apache/commons/io/IOUtilsTest.java | 9 +++
3 files changed, 101 insertions(+), 19 deletions(-)
diff --git a/src/main/java/org/apache/commons/io/IO.java
b/src/main/java/org/apache/commons/io/IO.java
new file mode 100644
index 00000000..d9db4246
--- /dev/null
+++ b/src/main/java/org/apache/commons/io/IO.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.commons.io;
+
+/**
+ * Component-wide operations on Apache Commons IO.
+ */
+class IO {
+
+ /**
+ * Clears any state, throughout Apache Commons IO. Handy for tests.
+ */
+ static void clear() {
+ IOUtils.clear();
+ }
+
+}
diff --git a/src/main/java/org/apache/commons/io/IOUtils.java
b/src/main/java/org/apache/commons/io/IOUtils.java
index 58fb5521..cca9a798 100644
--- a/src/main/java/org/apache/commons/io/IOUtils.java
+++ b/src/main/java/org/apache/commons/io/IOUtils.java
@@ -193,14 +193,24 @@ public class IOUtils {
public static final String LINE_SEPARATOR_WINDOWS =
StandardLineSeparator.CRLF.getString();
/**
- * Internal byte array buffer.
+ * Internal byte array buffer, intended for both reading and writing.
*/
- private static final ThreadLocal<byte[]> SCRATCH_BYTE_BUFFER =
ThreadLocal.withInitial(IOUtils::byteArray);
+ private static final ThreadLocal<byte[]> SCRATCH_BYTE_BUFFER_RW =
ThreadLocal.withInitial(IOUtils::byteArray);
/**
- * Internal byte array buffer.
+ * Internal byte array buffer, intended for write only operations.
*/
- private static final ThreadLocal<char[]> SCRATCH_CHAR_BUFFER =
ThreadLocal.withInitial(IOUtils::charArray);
+ private static final byte[] SCRATCH_BYTE_BUFFER_WO = byteArray();
+
+ /**
+ * Internal char array buffer, intended for both reading and writing.
+ */
+ private static final ThreadLocal<char[]> SCRATCH_CHAR_BUFFER_RW =
ThreadLocal.withInitial(IOUtils::charArray);
+
+ /**
+ * Internal char array buffer, intended for write only operations.
+ */
+ private static final char[] SCRATCH_CHAR_BUFFER_WO = charArray();
/**
* Returns the given InputStream if it is already a {@link
BufferedInputStream}, otherwise creates a
@@ -377,6 +387,21 @@ public class IOUtils {
return new char[size];
}
+ /**
+ * Clears any state.
+ * <ul>
+ * <li>Removes the current thread's value for thread-local variables.</li>
+ * <li>Sets static scratch arrays to 0s.</li>
+ * </ul>
+ * @see IO#clear()
+ */
+ static void clear() {
+ SCRATCH_BYTE_BUFFER_RW.remove();
+ SCRATCH_CHAR_BUFFER_RW.remove();
+ Arrays.fill(SCRATCH_BYTE_BUFFER_WO, (byte) 0);
+ Arrays.fill(SCRATCH_CHAR_BUFFER_WO, (char) 0);
+ }
+
/**
* Closes the given {@link Closeable} as a null-safe operation.
*
@@ -1660,21 +1685,39 @@ public class IOUtils {
}
/**
- * Gets the thread local byte array.
+ * Gets the internal byte array buffer, intended for both reading and
writing.
+ *
+ * @return the internal byte array buffer, intended for both reading and
writing.
+ */
+ private static byte[] getScratchByteArray() {
+ return SCRATCH_BYTE_BUFFER_RW.get();
+ }
+
+ /**
+ * Gets the internal byte array intended for write only operations.
*
- * @return the thread local byte array.
+ * @return the internal byte array intended for write only operations.
*/
- static byte[] getScratchByteArray() {
- return SCRATCH_BYTE_BUFFER.get();
+ private static byte[] getScratchByteArrayWriteOnly() {
+ return SCRATCH_BYTE_BUFFER_WO;
}
/**
- * Gets the thread local char array.
+ * Gets the char byte array buffer, intended for both reading and writing.
*
- * @return the thread local char array.
+ * @return the char byte array buffer, intended for both reading and
writing.
*/
static char[] getScratchCharArray() {
- return SCRATCH_CHAR_BUFFER.get();
+ return SCRATCH_CHAR_BUFFER_RW.get();
+ }
+
+ /**
+ * Gets the internal char array intended for write only operations.
+ *
+ * @return the internal char array intended for write only operations.
+ */
+ private static char[] getScratchCharArrayWriteOnly() {
+ return SCRATCH_CHAR_BUFFER_WO;
}
/**
@@ -2291,16 +2334,15 @@ public class IOUtils {
if (toSkip < 0) {
throw new IllegalArgumentException("Skip count must be
non-negative, actual: " + toSkip);
}
- /*
- * N.B. no need to synchronize access to SKIP_BYTE_BUFFER: - we don't
care if the buffer is created multiple
- * times (the data is ignored) - we always use the same size buffer,
so if it is recreated it will still be
- * OK (if the buffer size were variable, we would need to synch. to
ensure some other thread did not create a
- * smaller one)
- */
+ //
+ // No need to synchronize access to SCRATCH_BYTE_BUFFER_WO: We don't
care if the buffer is written multiple
+ // times or in parallel since the data is ignored. We reuse the same
buffer, if the buffer size were variable or read-write,
+ // we would need to synch or use a thread local to ensure some other
thread safety.
+ //
long remain = toSkip;
while (remain > 0) {
// See https://issues.apache.org/jira/browse/IO-203 for why we use
read() rather than delegating to skip()
- final byte[] byteArray = getScratchByteArray();
+ final byte[] byteArray = getScratchByteArrayWriteOnly();
final long n = input.read(byteArray, 0, (int) Math.min(remain,
byteArray.length));
if (n < 0) { // EOF
break;
@@ -2368,7 +2410,7 @@ public class IOUtils {
long remain = toSkip;
while (remain > 0) {
// See https://issues.apache.org/jira/browse/IO-203 for why we use
read() rather than delegating to skip()
- final char[] charArray = getScratchCharArray();
+ final char[] charArray = getScratchCharArrayWriteOnly();
final long n = reader.read(charArray, 0, (int) Math.min(remain,
charArray.length));
if (n < 0) { // EOF
break;
diff --git a/src/test/java/org/apache/commons/io/IOUtilsTest.java
b/src/test/java/org/apache/commons/io/IOUtilsTest.java
index dd988b2e..395b8e8f 100644
--- a/src/test/java/org/apache/commons/io/IOUtilsTest.java
+++ b/src/test/java/org/apache/commons/io/IOUtilsTest.java
@@ -77,6 +77,8 @@ import org.apache.commons.io.output.StringBuilderWriter;
import org.apache.commons.io.test.TestUtils;
import org.apache.commons.io.test.ThrowOnCloseReader;
import org.apache.commons.lang3.StringUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
@@ -127,6 +129,13 @@ public class IOUtilsTest {
assertArrayEquals(b0, b1, "Content not equal according to
java.util.Arrays#equals()");
}
+ @BeforeAll
+ @AfterAll
+ public static void beforeAll() {
+ // Not required, just to exercise the method and make sure there are
no adverse side-effect when recycling thread locals.
+ IO.clear();
+ }
+
@BeforeEach
public void setUp() {
try {
