You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/04/03 10:45:27 UTC
[GitHub] [commons-io] adamretter opened a new pull request #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
adamretter opened a new pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108
This abstract the functionality of `ByteArrayOutputStream` into `AbstractByteArrayOutputStream`. Synchronized and non-synchronized (for performance) implementations are then provided in `ByteArrayOutputStream` and `FastByteArrayOutputStream`.
Tests are also provided.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-610883780
@garydgregory @aherbert Yay! Thanks :-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608611586
@garydgregory Fair points. I have renamed it as you suggested.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608372513
[](https://coveralls.io/builds/29836334)
Coverage increased (+0.1%) to 89.539% when pulling **c2a92e3e913b61243afa4cf626736c909e359ed8 on adamretter:iostreams-sync** into **335808a8295c3d4dc1ec887b79f51699465889a6 on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on a change in pull request
#108: Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r403322127
##########
File path: src/main/java/org/apache/commons/io/output/AbstractByteArrayOutputStream.java
##########
@@ -0,0 +1,391 @@
+/*
+ * 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.output;
+
+import org.apache.commons.io.input.ClosedInputStream;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.SequenceInputStream;
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.commons.io.IOUtils.EOF;
+
+/**
+ * This is the base class for implementing an output stream in which the data
+ * is written into a byte array. The buffer automatically grows as data
+ * is written to it.
+ * <p>
+ * The data can be retrieved using <code>toByteArray()</code> and
+ * <code>toString()</code>.
+ * <p>
+ * Closing an {@code AbstractByteArrayOutputStream} has no effect. The methods in
+ * this class can be called after the stream has been closed without
+ * generating an {@code IOException}.
+ * <p>
+ * This is the base for an alternative implementation of the
+ * {@link java.io.ByteArrayOutputStream} class. The original implementation
+ * only allocates 32 bytes at the beginning. As this class is designed for
+ * heavy duty it starts at 1024 bytes. In contrast to the original it doesn't
+ * reallocate the whole memory block but allocates additional buffers. This
+ * way no buffers need to be garbage collected and the contents don't have
+ * to be copied to the new buffer. This class is designed to behave exactly
+ * like the original. The only exception is the deprecated
+ * {@link java.io.ByteArrayOutputStream#toString(int)} method that has been
+ * ignored.
+ */
+public abstract class AbstractByteArrayOutputStream extends OutputStream {
+
+ static final int DEFAULT_SIZE = 1024;
+
+ /** A singleton empty byte array. */
+ private static final byte[] EMPTY_BYTE_ARRAY = new byte[0];
+
+ /** The list of buffers, which grows and never reduces. */
+ private final List<byte[]> buffers = new ArrayList<>();
+ /** The index of the current buffer. */
+ private int currentBufferIndex;
+ /** The total count of bytes in all the filled buffers. */
+ private int filledBufferSum;
+ /** The current buffer. */
+ private byte[] currentBuffer;
+ /** The total count of bytes written. */
+ protected int count;
+ /** Flag to indicate if the buffers can be reused after reset */
+ private boolean reuseBuffers = true;
+
+ /**
+ * Makes a new buffer available either by allocating
+ * a new one or re-cycling an existing one.
+ *
+ * @param newcount the size of the buffer if one is created
+ */
+ protected void needNewBuffer(final int newcount) {
+ if (currentBufferIndex < buffers.size() - 1) {
+ //Recycling old buffer
+ filledBufferSum += currentBuffer.length;
+
+ currentBufferIndex++;
+ currentBuffer = buffers.get(currentBufferIndex);
+ } else {
+ //Creating new buffer
+ int newBufferSize;
+ if (currentBuffer == null) {
+ newBufferSize = newcount;
+ filledBufferSum = 0;
+ } else {
+ newBufferSize = Math.max(
+ currentBuffer.length << 1,
+ newcount - filledBufferSum);
+ filledBufferSum += currentBuffer.length;
+ }
+
+ currentBufferIndex++;
+ currentBuffer = new byte[newBufferSize];
+ buffers.add(currentBuffer);
+ }
+ }
+
+ /**
+ * Write the bytes to byte array.
Review comment:
"Write" -> "Writes".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-610484095
@garydgregory Sure I see it now. I have just pushed a commit to further improve coverage.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608587144
> @garydgregory Okay I understand. Your preferred option is very long though. How do you feel about:
>
> 1. `UnsyncByteArrayOutputStream`.
The least worst IMO.
> 2. `UByteArrayOutputStream`.
Oh, the horror! ;-) There is no convention that equates "U" to "Unsynchronized", if anything "U" usually stands for "Unsigned".
> 3. `USByteArrayOutputStream`.
Even worse IMO! Same comment as above except US usually means... United States?
> 4. `UnsafeByteArrayOutputStream`.
Nope, same as for "Fast", unsafe compared in what area? Will this be a CVE?
I do not thing there should be an issue spelling out Unsynchronized, if you look around, you'll see similar names spelled out:
- javax.transaction.Synchronization
- java.util.concurrent.locks.AbstractOwnableSynchronizer
- java.util.concurrent.locks.AbstractQueuedLongSynchronizer
- java.util.concurrent.locks.AbstractQueuedSynchronizer
- java.util.Collections.SynchronizedCollection
- java.util.Collections.SynchronizedList
- java.util.Collections.SynchronizedMap
- java.util.Collections.SynchronizedNavigableMap
- java.util.Collections.SynchronizedNavigableSet
- java.util.Collections.SynchronizedRandomAccessList
- java.util.Collections.SynchronizedSet
- java.util.Collections.SynchronizedSortedMap<
- java.util.Collections.SynchronizedSortedSet
- Too many to list in Google Guava.
- Too many to list in our own Apache Commons Collections.
So, IMO, just say what it does.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-609409411
@garydgregory I have incorporated your further feedback now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-610476151
@garydgregory Hmm, from what I read the code coverage after this PR is `+0.07%` so it is a net improvement, no?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-610433745
Hi @adamretter
Thank you for your updates.
JaCoCo reports that the code coverage for `UnsynchronizedByteArrayOutputStream` is less than it is for `ByteArrayOutputStream`.java. May you address this please?
Thank you!
You can run this: `mvn -V clean install site -P jacoco -P japicmp`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608372513
[](https://coveralls.io/builds/29823736)
Coverage increased (+0.07%) to 89.475% when pulling **4db1a3ffc2c5933b75832f436dd6993e628992f8 on adamretter:iostreams-sync** into **335808a8295c3d4dc1ec887b79f51699465889a6 on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608372513
[](https://coveralls.io/builds/29917430)
Coverage increased (+0.2%) to 89.588% when pulling **568e3e7ed59b12215aa8a3fe79f0bc3b9c1a4af3 on adamretter:iostreams-sync** into **335808a8295c3d4dc1ec887b79f51699465889a6 on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
adamretter edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-610476151
@garydgregory Hmm, from what I read in coveralls.io the code coverage after this PR is `+0.07%` so it is a net improvement, no?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on a change in pull request
#108: Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r403322511
##########
File path: src/main/java/org/apache/commons/io/output/AbstractByteArrayOutputStream.java
##########
@@ -0,0 +1,391 @@
+/*
+ * 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.output;
+
+import org.apache.commons.io.input.ClosedInputStream;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.SequenceInputStream;
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.commons.io.IOUtils.EOF;
+
+/**
+ * This is the base class for implementing an output stream in which the data
+ * is written into a byte array. The buffer automatically grows as data
+ * is written to it.
+ * <p>
+ * The data can be retrieved using <code>toByteArray()</code> and
+ * <code>toString()</code>.
+ * <p>
+ * Closing an {@code AbstractByteArrayOutputStream} has no effect. The methods in
+ * this class can be called after the stream has been closed without
+ * generating an {@code IOException}.
+ * <p>
+ * This is the base for an alternative implementation of the
+ * {@link java.io.ByteArrayOutputStream} class. The original implementation
+ * only allocates 32 bytes at the beginning. As this class is designed for
+ * heavy duty it starts at 1024 bytes. In contrast to the original it doesn't
+ * reallocate the whole memory block but allocates additional buffers. This
+ * way no buffers need to be garbage collected and the contents don't have
+ * to be copied to the new buffer. This class is designed to behave exactly
+ * like the original. The only exception is the deprecated
+ * {@link java.io.ByteArrayOutputStream#toString(int)} method that has been
+ * ignored.
+ */
+public abstract class AbstractByteArrayOutputStream extends OutputStream {
+
+ static final int DEFAULT_SIZE = 1024;
+
+ /** A singleton empty byte array. */
+ private static final byte[] EMPTY_BYTE_ARRAY = new byte[0];
+
+ /** The list of buffers, which grows and never reduces. */
+ private final List<byte[]> buffers = new ArrayList<>();
+ /** The index of the current buffer. */
+ private int currentBufferIndex;
+ /** The total count of bytes in all the filled buffers. */
+ private int filledBufferSum;
+ /** The current buffer. */
+ private byte[] currentBuffer;
+ /** The total count of bytes written. */
+ protected int count;
+ /** Flag to indicate if the buffers can be reused after reset */
+ private boolean reuseBuffers = true;
+
+ /**
+ * Makes a new buffer available either by allocating
+ * a new one or re-cycling an existing one.
+ *
+ * @param newcount the size of the buffer if one is created
+ */
+ protected void needNewBuffer(final int newcount) {
+ if (currentBufferIndex < buffers.size() - 1) {
+ //Recycling old buffer
+ filledBufferSum += currentBuffer.length;
+
+ currentBufferIndex++;
+ currentBuffer = buffers.get(currentBufferIndex);
+ } else {
+ //Creating new buffer
+ int newBufferSize;
+ if (currentBuffer == null) {
+ newBufferSize = newcount;
+ filledBufferSum = 0;
+ } else {
+ newBufferSize = Math.max(
+ currentBuffer.length << 1,
+ newcount - filledBufferSum);
+ filledBufferSum += currentBuffer.length;
+ }
+
+ currentBufferIndex++;
+ currentBuffer = new byte[newBufferSize];
+ buffers.add(currentBuffer);
+ }
+ }
+
+ /**
+ * Write the bytes to byte array.
+ * @param b the bytes to write
+ * @param off The start offset
+ * @param len The number of bytes to write
+ */
+ @Override
+ public abstract void write(final byte[] b, final int off, final int len);
+
+ /**
+ * Write the bytes to byte array.
+ * @param b the bytes to write
+ * @param off The start offset
+ * @param len The number of bytes to write
+ */
+ protected void writeImpl(final byte[] b, final int off, final int len) {
+ final int newcount = count + len;
+ int remaining = len;
+ int inBufferPos = count - filledBufferSum;
+ while (remaining > 0) {
+ final int part = Math.min(remaining, currentBuffer.length - inBufferPos);
+ System.arraycopy(b, off + len - remaining, currentBuffer, inBufferPos, part);
+ remaining -= part;
+ if (remaining > 0) {
+ needNewBuffer(newcount);
+ inBufferPos = 0;
+ }
+ }
+ count = newcount;
+ }
+
+ /**
+ * Write a byte to byte array.
+ * @param b the byte to write
+ */
+ @Override
+ public abstract void write(final int b);
+
+ /**
+ * Write a byte to byte array.
+ * @param b the byte to write
+ */
+ protected void writeImpl(final int b) {
+ int inBufferPos = count - filledBufferSum;
+ if (inBufferPos == currentBuffer.length) {
+ needNewBuffer(count + 1);
+ inBufferPos = 0;
+ }
+ currentBuffer[inBufferPos] = (byte) b;
+ count++;
+ }
+
+
+ /**
+ * Writes the entire contents of the specified input stream to this
+ * byte stream. Bytes from the input stream are read directly into the
+ * internal buffers of this streams.
+ *
+ * @param in the input stream to read from
+ * @return total number of bytes read from the input stream
+ * (and written to this stream)
+ * @throws IOException if an I/O error occurs while reading the input stream
+ * @since 1.4
+ */
+ public abstract int write(final InputStream in) throws IOException;
+
+ /**
+ * Writes the entire contents of the specified input stream to this
+ * byte stream. Bytes from the input stream are read directly into the
+ * internal buffers of this streams.
+ *
+ * @param in the input stream to read from
+ * @return total number of bytes read from the input stream
+ * (and written to this stream)
+ * @throws IOException if an I/O error occurs while reading the input stream
+ * @since 2.7
+ */
+ protected int writeImpl(final InputStream in) throws IOException {
+ int readCount = 0;
+ int inBufferPos = count - filledBufferSum;
+ int n = in.read(currentBuffer, inBufferPos, currentBuffer.length - inBufferPos);
+ while (n != EOF) {
+ readCount += n;
+ inBufferPos += n;
+ count += n;
+ if (inBufferPos == currentBuffer.length) {
+ needNewBuffer(currentBuffer.length);
+ inBufferPos = 0;
+ }
+ n = in.read(currentBuffer, inBufferPos, currentBuffer.length - inBufferPos);
+ }
+ return readCount;
+ }
+
+ /**
+ * Return the current size of the byte array.
Review comment:
"Return" -> "Returns".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608581637
@garydgregory Okay I understand. Your preferred option is very long though. How do you feel about:
1. `UnsyncByteArrayOutputStream`.
2. `UByteArrayOutputStream`.
3. `USByteArrayOutputStream`.
4. `UnsafeByteArrayOutputStream`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608372513
[](https://coveralls.io/builds/29822612)
Coverage increased (+0.07%) to 89.475% when pulling **66301700e1da72843a6ac8c44e3a7d0ccc069881 on adamretter:iostreams-sync** into **335808a8295c3d4dc1ec887b79f51699465889a6 on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608576754
> This looks like a nice conversion and maintains the previous support. I'd be wary of using a new dependency just to mark the code as Threadsafe with an annotation on an existing class. Bringing in an annotation framework is a different topic.
>
> Using the name FastXXX is presumptuous but I cannot think of something better. NonSyncXXX is not as friendly. Others may have comments on this.
I am a big -1 on calling a new class "Fast" since it is not fast, it just removes thread-safety. You can hope that it is "faster" than the standard class but that's it.
The JRE has been calling some thread-safe classes `Concurrent` like `ConcurrentHashMap` but other JRE thread-safe classes just have names that reflect behavior like `CopyOnWriteArrayList`, it therefore seem OK to me to call a classes that is not synchronized one of:
- `UnsynchronizedByteArrayOutputStream` (to the point, precise)
- `SimpleByteArrayOutputStream` (not great)
- `BasicByteArrayOutputStream` (not great)
- `DefaultByteArrayOutputStream` (not great)
My preference is `UnsynchroniziedByteArrayOutputStream`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608372513
[](https://coveralls.io/builds/29873225)
Coverage increased (+0.07%) to 89.475% when pulling **70e274135bab3db7863fbbdc9423cacddc33b1be on adamretter:iostreams-sync** into **335808a8295c3d4dc1ec887b79f51699465889a6 on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on a change in pull request
#108: Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r403323945
##########
File path: src/main/java/org/apache/commons/io/output/UnsynchronizedByteArrayOutputStream.java
##########
@@ -0,0 +1,161 @@
+/*
+ * 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.output;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * This class implements a version of {@link AbstractByteArrayOutputStream}
Review comment:
"This class implements..." -> "Implements..."
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on a change in pull request #108:
Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r402980055
##########
File path: pom.xml
##########
@@ -226,6 +230,11 @@ file comparators, endian transformation classes, and much more.
</contributors>
<dependencies>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
Review comment:
Well yes it is to bring in the annotation... but these annotations can also be used by various tools like FindBugs and others to detect issues.
I think it adds value, and jsr305 is tiny (19KB). I can remove it if people prefer? It also adds useful annotations like `@Nullable` and `@NotNull` for similar purposes.
I can remove it though if that is preferred. Should I do that?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608372513
[](https://coveralls.io/builds/29857465)
Coverage increased (+0.2%) to 89.588% when pulling **bcadb75d873b8a8c73e9ee6f04b6bfde8e169b2a on adamretter:iostreams-sync** into **335808a8295c3d4dc1ec887b79f51699465889a6 on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] aherbert commented on a change in pull request #108:
Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r402927109
##########
File path: pom.xml
##########
@@ -226,6 +230,11 @@ file comparators, endian transformation classes, and much more.
</contributors>
<dependencies>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
Review comment:
Is this just to bring in the @javax.annotation.concurrent.ThreadSafe annotation? I don't think this necessary since you already document the thread safety in the javadoc. Nothing else in the code base uses this non-standard annotation support.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on a change in pull request
#108: Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r403323337
##########
File path: src/main/java/org/apache/commons/io/output/AbstractByteArrayOutputStream.java
##########
@@ -0,0 +1,391 @@
+/*
+ * 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.output;
+
+import org.apache.commons.io.input.ClosedInputStream;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.SequenceInputStream;
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.commons.io.IOUtils.EOF;
+
+/**
+ * This is the base class for implementing an output stream in which the data
+ * is written into a byte array. The buffer automatically grows as data
+ * is written to it.
+ * <p>
+ * The data can be retrieved using <code>toByteArray()</code> and
+ * <code>toString()</code>.
+ * <p>
+ * Closing an {@code AbstractByteArrayOutputStream} has no effect. The methods in
+ * this class can be called after the stream has been closed without
+ * generating an {@code IOException}.
+ * <p>
+ * This is the base for an alternative implementation of the
+ * {@link java.io.ByteArrayOutputStream} class. The original implementation
+ * only allocates 32 bytes at the beginning. As this class is designed for
+ * heavy duty it starts at 1024 bytes. In contrast to the original it doesn't
+ * reallocate the whole memory block but allocates additional buffers. This
+ * way no buffers need to be garbage collected and the contents don't have
+ * to be copied to the new buffer. This class is designed to behave exactly
+ * like the original. The only exception is the deprecated
+ * {@link java.io.ByteArrayOutputStream#toString(int)} method that has been
+ * ignored.
+ */
+public abstract class AbstractByteArrayOutputStream extends OutputStream {
+
+ static final int DEFAULT_SIZE = 1024;
+
+ /** A singleton empty byte array. */
+ private static final byte[] EMPTY_BYTE_ARRAY = new byte[0];
+
+ /** The list of buffers, which grows and never reduces. */
+ private final List<byte[]> buffers = new ArrayList<>();
+ /** The index of the current buffer. */
+ private int currentBufferIndex;
+ /** The total count of bytes in all the filled buffers. */
+ private int filledBufferSum;
+ /** The current buffer. */
+ private byte[] currentBuffer;
+ /** The total count of bytes written. */
+ protected int count;
+ /** Flag to indicate if the buffers can be reused after reset */
+ private boolean reuseBuffers = true;
+
+ /**
+ * Makes a new buffer available either by allocating
+ * a new one or re-cycling an existing one.
+ *
+ * @param newcount the size of the buffer if one is created
+ */
+ protected void needNewBuffer(final int newcount) {
+ if (currentBufferIndex < buffers.size() - 1) {
+ //Recycling old buffer
+ filledBufferSum += currentBuffer.length;
+
+ currentBufferIndex++;
+ currentBuffer = buffers.get(currentBufferIndex);
+ } else {
+ //Creating new buffer
+ int newBufferSize;
+ if (currentBuffer == null) {
+ newBufferSize = newcount;
+ filledBufferSum = 0;
+ } else {
+ newBufferSize = Math.max(
+ currentBuffer.length << 1,
+ newcount - filledBufferSum);
+ filledBufferSum += currentBuffer.length;
+ }
+
+ currentBufferIndex++;
+ currentBuffer = new byte[newBufferSize];
+ buffers.add(currentBuffer);
+ }
+ }
+
+ /**
+ * Write the bytes to byte array.
+ * @param b the bytes to write
+ * @param off The start offset
+ * @param len The number of bytes to write
+ */
+ @Override
+ public abstract void write(final byte[] b, final int off, final int len);
+
+ /**
+ * Write the bytes to byte array.
+ * @param b the bytes to write
+ * @param off The start offset
+ * @param len The number of bytes to write
+ */
+ protected void writeImpl(final byte[] b, final int off, final int len) {
+ final int newcount = count + len;
+ int remaining = len;
+ int inBufferPos = count - filledBufferSum;
+ while (remaining > 0) {
+ final int part = Math.min(remaining, currentBuffer.length - inBufferPos);
+ System.arraycopy(b, off + len - remaining, currentBuffer, inBufferPos, part);
+ remaining -= part;
+ if (remaining > 0) {
+ needNewBuffer(newcount);
+ inBufferPos = 0;
+ }
+ }
+ count = newcount;
+ }
+
+ /**
+ * Write a byte to byte array.
+ * @param b the byte to write
+ */
+ @Override
+ public abstract void write(final int b);
+
+ /**
+ * Write a byte to byte array.
+ * @param b the byte to write
+ */
+ protected void writeImpl(final int b) {
+ int inBufferPos = count - filledBufferSum;
+ if (inBufferPos == currentBuffer.length) {
+ needNewBuffer(count + 1);
+ inBufferPos = 0;
+ }
+ currentBuffer[inBufferPos] = (byte) b;
+ count++;
+ }
+
+
+ /**
+ * Writes the entire contents of the specified input stream to this
+ * byte stream. Bytes from the input stream are read directly into the
+ * internal buffers of this streams.
+ *
+ * @param in the input stream to read from
+ * @return total number of bytes read from the input stream
+ * (and written to this stream)
+ * @throws IOException if an I/O error occurs while reading the input stream
+ * @since 1.4
+ */
+ public abstract int write(final InputStream in) throws IOException;
+
+ /**
+ * Writes the entire contents of the specified input stream to this
+ * byte stream. Bytes from the input stream are read directly into the
+ * internal buffers of this streams.
+ *
+ * @param in the input stream to read from
+ * @return total number of bytes read from the input stream
+ * (and written to this stream)
+ * @throws IOException if an I/O error occurs while reading the input stream
+ * @since 2.7
+ */
+ protected int writeImpl(final InputStream in) throws IOException {
+ int readCount = 0;
+ int inBufferPos = count - filledBufferSum;
+ int n = in.read(currentBuffer, inBufferPos, currentBuffer.length - inBufferPos);
+ while (n != EOF) {
+ readCount += n;
+ inBufferPos += n;
+ count += n;
+ if (inBufferPos == currentBuffer.length) {
+ needNewBuffer(currentBuffer.length);
+ inBufferPos = 0;
+ }
+ n = in.read(currentBuffer, inBufferPos, currentBuffer.length - inBufferPos);
+ }
+ return readCount;
+ }
+
+ /**
+ * Return the current size of the byte array.
+ * @return the current size of the byte array
+ */
+ public abstract int size();
+
+ /**
+ * Closing a {@code ByteArrayOutputStream} has no effect. The methods in
+ * this class can be called after the stream has been closed without
+ * generating an {@code IOException}.
+ *
+ * @throws IOException never (this method should not declare this exception
+ * but it has to now due to backwards compatibility)
+ */
+ @Override
+ public void close() throws IOException {
+ //nop
+ }
+
+ /**
+ * @see java.io.ByteArrayOutputStream#reset()
+ */
+ public abstract void reset();
+
+ /**
+ * @see java.io.ByteArrayOutputStream#reset()
+ */
+ protected void resetImpl() {
+ count = 0;
+ filledBufferSum = 0;
+ currentBufferIndex = 0;
+ if (reuseBuffers) {
+ currentBuffer = buffers.get(currentBufferIndex);
+ } else {
+ //Throw away old buffers
+ currentBuffer = null;
+ final int size = buffers.get(0).length;
+ buffers.clear();
+ needNewBuffer(size);
+ reuseBuffers = true;
+ }
+ }
+
+ /**
+ * Writes the entire contents of this byte stream to the
+ * specified output stream.
+ *
+ * @param out the output stream to write to
+ * @throws IOException if an I/O error occurs, such as if the stream is closed
+ * @see java.io.ByteArrayOutputStream#writeTo(OutputStream)
+ */
+ public abstract void writeTo(final OutputStream out) throws IOException;
+
+ /**
+ * Writes the entire contents of this byte stream to the
+ * specified output stream.
+ *
+ * @param out the output stream to write to
+ * @throws IOException if an I/O error occurs, such as if the stream is closed
+ * @see java.io.ByteArrayOutputStream#writeTo(OutputStream)
+ */
+ protected void writeToImpl(final OutputStream out) throws IOException {
+ int remaining = count;
+ for (final byte[] buf : buffers) {
+ final int c = Math.min(buf.length, remaining);
+ out.write(buf, 0, c);
+ remaining -= c;
+ if (remaining == 0) {
+ break;
+ }
+ }
+ }
+
+ /**
+ * Gets the current contents of this byte stream as a Input Stream. The
+ * returned stream is backed by buffers of <code>this</code> stream,
+ * avoiding memory allocation and copy, thus saving space and time.<br>
+ *
+ * @return the current contents of this output stream.
+ * @see java.io.ByteArrayOutputStream#toByteArray()
+ * @see #reset()
+ * @since 2.5
+ */
+ public abstract InputStream toInputStream();
+
+ /**
+ * Gets the current contents of this byte stream as a Input Stream. The
+ * returned stream is backed by buffers of <code>this</code> stream,
+ * avoiding memory allocation and copy, thus saving space and time.<br>
+ *
+ * @return the current contents of this output stream.
+ * @see java.io.ByteArrayOutputStream#toByteArray()
+ * @see #reset()
+ * @since 2.7
+ */
+ protected InputStream toInputStreamImpl() {
+ int remaining = count;
+ if (remaining == 0) {
+ return new ClosedInputStream();
+ }
+ final List<ByteArrayInputStream> list = new ArrayList<>(buffers.size());
+ for (final byte[] buf : buffers) {
+ final int c = Math.min(buf.length, remaining);
+ list.add(new ByteArrayInputStream(buf, 0, c));
+ remaining -= c;
+ if (remaining == 0) {
+ break;
+ }
+ }
+ reuseBuffers = false;
+ return new SequenceInputStream(Collections.enumeration(list));
+ }
+
+ /**
+ * Gets the current contents of this byte stream as a byte array.
+ * The result is independent of this stream.
+ *
+ * @return the current contents of this output stream, as a byte array
+ * @see java.io.ByteArrayOutputStream#toByteArray()
+ */
+ public abstract byte[] toByteArray();
+
+ /**
+ * Gets the current contents of this byte stream as a byte array.
+ * The result is independent of this stream.
+ *
+ * @return the current contents of this output stream, as a byte array
+ * @see java.io.ByteArrayOutputStream#toByteArray()
+ */
+ protected byte[] toByteArrayImpl() {
+ int remaining = count;
+ if (remaining == 0) {
+ return EMPTY_BYTE_ARRAY;
+ }
+ final byte newbuf[] = new byte[remaining];
+ int pos = 0;
+ for (final byte[] buf : buffers) {
+ final int c = Math.min(buf.length, remaining);
+ System.arraycopy(buf, 0, newbuf, pos, c);
+ pos += c;
+ remaining -= c;
+ if (remaining == 0) {
+ break;
+ }
+ }
+ return newbuf;
+ }
+
+ /**
+ * Gets the current contents of this byte stream as a string
+ * using the platform default charset.
+ * @return the contents of the byte array as a String
+ * @see java.io.ByteArrayOutputStream#toString()
+ * @deprecated 2.5 use {@link #toString(String)} instead
Review comment:
I do not think it makes sense to deprecate `toString()` since it is such a basic JRE method that will never go away.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608372513
[](https://coveralls.io/builds/29859359)
Coverage increased (+0.07%) to 89.475% when pulling **b60165d8800c5e1973dd846d9011e7e57e70be96 on adamretter:iostreams-sync** into **335808a8295c3d4dc1ec887b79f51699465889a6 on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on a change in pull request #108:
Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r403013558
##########
File path: pom.xml
##########
@@ -226,6 +230,11 @@ file comparators, endian transformation classes, and much more.
</contributors>
<dependencies>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
Review comment:
Okie dokie. Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on a change in pull request
#108: Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r403321265
##########
File path: src/main/java/org/apache/commons/io/output/AbstractByteArrayOutputStream.java
##########
@@ -0,0 +1,391 @@
+/*
+ * 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.output;
+
+import org.apache.commons.io.input.ClosedInputStream;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.SequenceInputStream;
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.commons.io.IOUtils.EOF;
+
+/**
+ * This is the base class for implementing an output stream in which the data
+ * is written into a byte array. The buffer automatically grows as data
+ * is written to it.
+ * <p>
+ * The data can be retrieved using <code>toByteArray()</code> and
+ * <code>toString()</code>.
+ * <p>
+ * Closing an {@code AbstractByteArrayOutputStream} has no effect. The methods in
+ * this class can be called after the stream has been closed without
+ * generating an {@code IOException}.
+ * <p>
+ * This is the base for an alternative implementation of the
+ * {@link java.io.ByteArrayOutputStream} class. The original implementation
+ * only allocates 32 bytes at the beginning. As this class is designed for
+ * heavy duty it starts at 1024 bytes. In contrast to the original it doesn't
+ * reallocate the whole memory block but allocates additional buffers. This
+ * way no buffers need to be garbage collected and the contents don't have
+ * to be copied to the new buffer. This class is designed to behave exactly
+ * like the original. The only exception is the deprecated
+ * {@link java.io.ByteArrayOutputStream#toString(int)} method that has been
+ * ignored.
+ */
Review comment:
Please Add `@since 2.7`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608372513
[](https://coveralls.io/builds/29824910)
Coverage increased (+0.1%) to 89.523% when pulling **64fbcf74fba5fa2a718342a8b47a4a790cd4b136 on adamretter:iostreams-sync** into **335808a8295c3d4dc1ec887b79f51699465889a6 on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-609717986
@aherbert Good point. I have incorporated the changes as you suggested.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-609422545
@garydgregory Adding the closing `<p>` tags as you suggested breaks the javadoc build, so I will go ahead and remove them again:
```
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 06:02 min
[INFO] Finished at: 2020-04-05T12:41:14Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.1.1:javadoc (default-cli) on project commons-io: An error has occurred in Javadoc report generation:
[ERROR] Exit code: 1 - /home/travis/build/adamretter/commons-io/src/main/java/org/apache/commons/io/output/ByteArrayOutputStream.java:115: error: unexpected end tag: </p>
[ERROR] * </p>
[ERROR] ^
[ERROR] /home/travis/build/adamretter/commons-io/src/main/java/org/apache/commons/io/output/ByteArrayOutputStream.java:142: error: unexpected end tag: </p>
[ERROR] * </p>
[ERROR] ^
[ERROR] /home/travis/build/adamretter/commons-io/src/main/java/org/apache/commons/io/output/UnsynchronizedByteArrayOutputStream.java:112: error: unexpected end tag: </p>
[ERROR] * </p>
[ERROR] ^
[ERROR] /home/travis/build/adamretter/commons-io/src/main/java/org/apache/commons/io/output/UnsynchronizedByteArrayOutputStream.java:138: error: unexpected end tag: </p>
[ERROR] * </p>
[ERROR] ^
[ERROR]
[ERROR] Command line was: /usr/local/lib/jvm/openjdk11/bin/javadoc @options @packages
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory merged pull request #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory edited a comment on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608587144
> @garydgregory Okay I understand. Your preferred option is very long though. How do you feel about:
>
> 1. `UnsyncByteArrayOutputStream`.
The least worst IMO.
> 2. `UByteArrayOutputStream`.
Oh, the horror! ;-) There is no convention that equates "U" to "Unsynchronized", if anything "U" usually stands for "Unsigned".
> 3. `USByteArrayOutputStream`.
Even worse IMO! Same comment as above except US usually means... United States?
> 4. `UnsafeByteArrayOutputStream`.
Nope, same as for "Fast", unsafe compared in what area? Will this be a CVE?
I do not think there should be an issue spelling out Unsynchronized, if you look around, you'll see similar names spelled out:
- javax.transaction.Synchronization
- java.util.concurrent.locks.AbstractOwnableSynchronizer
- java.util.concurrent.locks.AbstractQueuedLongSynchronizer
- java.util.concurrent.locks.AbstractQueuedSynchronizer
- java.util.Collections.SynchronizedCollection
- java.util.Collections.SynchronizedList
- java.util.Collections.SynchronizedMap
- java.util.Collections.SynchronizedNavigableMap
- java.util.Collections.SynchronizedNavigableSet
- java.util.Collections.SynchronizedRandomAccessList
- java.util.Collections.SynchronizedSet
- java.util.Collections.SynchronizedSortedMap<
- java.util.Collections.SynchronizedSortedSet
- Too many to list in Google Guava.
- Too many to list in our own Apache Commons Collections.
So, IMO, just say what it does.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on a change in pull request #108:
Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r402982467
##########
File path: src/test/java/org/apache/commons/io/output/ByteArrayOutputStreamTestCase.java
##########
@@ -211,13 +217,30 @@ public void testStream() throws Exception {
//Make sure that empty ByteArrayOutputStreams really don't create garbage
//on toByteArray()
- final ByteArrayOutputStream baos1 = new ByteArrayOutputStream();
- final ByteArrayOutputStream baos2 = new ByteArrayOutputStream();
+ final AbstractByteArrayOutputStream baos1 = baosFactory.instance();
+ final AbstractByteArrayOutputStream baos2 = baosFactory.instance();
assertSame(baos1.toByteArray(), baos2.toByteArray());
baos1.close();
baos2.close();
baout.close();
baout1.close();
}
+
+ private static Stream<Arguments> baosFactories() {
+ final BAOSFactory syncBaosFactory = size -> new ByteArrayOutputStream(size);
+ final BAOSFactory nonSyncBaos = size -> new FastByteArrayOutputStream(size);
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on a change in pull request #108:
Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r403695497
##########
File path: src/main/java/org/apache/commons/io/output/AbstractByteArrayOutputStream.java
##########
@@ -0,0 +1,391 @@
+/*
+ * 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.output;
+
+import org.apache.commons.io.input.ClosedInputStream;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.SequenceInputStream;
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.commons.io.IOUtils.EOF;
+
+/**
+ * This is the base class for implementing an output stream in which the data
+ * is written into a byte array. The buffer automatically grows as data
+ * is written to it.
+ * <p>
+ * The data can be retrieved using <code>toByteArray()</code> and
+ * <code>toString()</code>.
+ * <p>
+ * Closing an {@code AbstractByteArrayOutputStream} has no effect. The methods in
+ * this class can be called after the stream has been closed without
+ * generating an {@code IOException}.
+ * <p>
+ * This is the base for an alternative implementation of the
+ * {@link java.io.ByteArrayOutputStream} class. The original implementation
+ * only allocates 32 bytes at the beginning. As this class is designed for
+ * heavy duty it starts at 1024 bytes. In contrast to the original it doesn't
+ * reallocate the whole memory block but allocates additional buffers. This
+ * way no buffers need to be garbage collected and the contents don't have
+ * to be copied to the new buffer. This class is designed to behave exactly
+ * like the original. The only exception is the deprecated
+ * {@link java.io.ByteArrayOutputStream#toString(int)} method that has been
+ * ignored.
+ */
+public abstract class AbstractByteArrayOutputStream extends OutputStream {
+
+ static final int DEFAULT_SIZE = 1024;
+
+ /** A singleton empty byte array. */
+ private static final byte[] EMPTY_BYTE_ARRAY = new byte[0];
+
+ /** The list of buffers, which grows and never reduces. */
+ private final List<byte[]> buffers = new ArrayList<>();
+ /** The index of the current buffer. */
+ private int currentBufferIndex;
+ /** The total count of bytes in all the filled buffers. */
+ private int filledBufferSum;
+ /** The current buffer. */
+ private byte[] currentBuffer;
+ /** The total count of bytes written. */
+ protected int count;
+ /** Flag to indicate if the buffers can be reused after reset */
+ private boolean reuseBuffers = true;
+
+ /**
+ * Makes a new buffer available either by allocating
+ * a new one or re-cycling an existing one.
+ *
+ * @param newcount the size of the buffer if one is created
+ */
+ protected void needNewBuffer(final int newcount) {
+ if (currentBufferIndex < buffers.size() - 1) {
+ //Recycling old buffer
+ filledBufferSum += currentBuffer.length;
+
+ currentBufferIndex++;
+ currentBuffer = buffers.get(currentBufferIndex);
+ } else {
+ //Creating new buffer
+ int newBufferSize;
+ if (currentBuffer == null) {
+ newBufferSize = newcount;
+ filledBufferSum = 0;
+ } else {
+ newBufferSize = Math.max(
+ currentBuffer.length << 1,
+ newcount - filledBufferSum);
+ filledBufferSum += currentBuffer.length;
+ }
+
+ currentBufferIndex++;
+ currentBuffer = new byte[newBufferSize];
+ buffers.add(currentBuffer);
+ }
+ }
+
+ /**
+ * Write the bytes to byte array.
+ * @param b the bytes to write
+ * @param off The start offset
+ * @param len The number of bytes to write
+ */
+ @Override
+ public abstract void write(final byte[] b, final int off, final int len);
+
+ /**
+ * Write the bytes to byte array.
+ * @param b the bytes to write
+ * @param off The start offset
+ * @param len The number of bytes to write
+ */
+ protected void writeImpl(final byte[] b, final int off, final int len) {
+ final int newcount = count + len;
+ int remaining = len;
+ int inBufferPos = count - filledBufferSum;
+ while (remaining > 0) {
+ final int part = Math.min(remaining, currentBuffer.length - inBufferPos);
+ System.arraycopy(b, off + len - remaining, currentBuffer, inBufferPos, part);
+ remaining -= part;
+ if (remaining > 0) {
+ needNewBuffer(newcount);
+ inBufferPos = 0;
+ }
+ }
+ count = newcount;
+ }
+
+ /**
+ * Write a byte to byte array.
+ * @param b the byte to write
+ */
+ @Override
+ public abstract void write(final int b);
+
+ /**
+ * Write a byte to byte array.
+ * @param b the byte to write
+ */
+ protected void writeImpl(final int b) {
+ int inBufferPos = count - filledBufferSum;
+ if (inBufferPos == currentBuffer.length) {
+ needNewBuffer(count + 1);
+ inBufferPos = 0;
+ }
+ currentBuffer[inBufferPos] = (byte) b;
+ count++;
+ }
+
+
+ /**
+ * Writes the entire contents of the specified input stream to this
+ * byte stream. Bytes from the input stream are read directly into the
+ * internal buffers of this streams.
+ *
+ * @param in the input stream to read from
+ * @return total number of bytes read from the input stream
+ * (and written to this stream)
+ * @throws IOException if an I/O error occurs while reading the input stream
+ * @since 1.4
+ */
+ public abstract int write(final InputStream in) throws IOException;
+
+ /**
+ * Writes the entire contents of the specified input stream to this
+ * byte stream. Bytes from the input stream are read directly into the
+ * internal buffers of this streams.
+ *
+ * @param in the input stream to read from
+ * @return total number of bytes read from the input stream
+ * (and written to this stream)
+ * @throws IOException if an I/O error occurs while reading the input stream
+ * @since 2.7
+ */
+ protected int writeImpl(final InputStream in) throws IOException {
+ int readCount = 0;
+ int inBufferPos = count - filledBufferSum;
+ int n = in.read(currentBuffer, inBufferPos, currentBuffer.length - inBufferPos);
+ while (n != EOF) {
+ readCount += n;
+ inBufferPos += n;
+ count += n;
+ if (inBufferPos == currentBuffer.length) {
+ needNewBuffer(currentBuffer.length);
+ inBufferPos = 0;
+ }
+ n = in.read(currentBuffer, inBufferPos, currentBuffer.length - inBufferPos);
+ }
+ return readCount;
+ }
+
+ /**
+ * Return the current size of the byte array.
+ * @return the current size of the byte array
+ */
+ public abstract int size();
+
+ /**
+ * Closing a {@code ByteArrayOutputStream} has no effect. The methods in
+ * this class can be called after the stream has been closed without
+ * generating an {@code IOException}.
+ *
+ * @throws IOException never (this method should not declare this exception
+ * but it has to now due to backwards compatibility)
+ */
+ @Override
+ public void close() throws IOException {
+ //nop
+ }
+
+ /**
+ * @see java.io.ByteArrayOutputStream#reset()
+ */
+ public abstract void reset();
+
+ /**
+ * @see java.io.ByteArrayOutputStream#reset()
+ */
+ protected void resetImpl() {
+ count = 0;
+ filledBufferSum = 0;
+ currentBufferIndex = 0;
+ if (reuseBuffers) {
+ currentBuffer = buffers.get(currentBufferIndex);
+ } else {
+ //Throw away old buffers
+ currentBuffer = null;
+ final int size = buffers.get(0).length;
+ buffers.clear();
+ needNewBuffer(size);
+ reuseBuffers = true;
+ }
+ }
+
+ /**
+ * Writes the entire contents of this byte stream to the
+ * specified output stream.
+ *
+ * @param out the output stream to write to
+ * @throws IOException if an I/O error occurs, such as if the stream is closed
+ * @see java.io.ByteArrayOutputStream#writeTo(OutputStream)
+ */
+ public abstract void writeTo(final OutputStream out) throws IOException;
+
+ /**
+ * Writes the entire contents of this byte stream to the
+ * specified output stream.
+ *
+ * @param out the output stream to write to
+ * @throws IOException if an I/O error occurs, such as if the stream is closed
+ * @see java.io.ByteArrayOutputStream#writeTo(OutputStream)
+ */
+ protected void writeToImpl(final OutputStream out) throws IOException {
+ int remaining = count;
+ for (final byte[] buf : buffers) {
+ final int c = Math.min(buf.length, remaining);
+ out.write(buf, 0, c);
+ remaining -= c;
+ if (remaining == 0) {
+ break;
+ }
+ }
+ }
+
+ /**
+ * Gets the current contents of this byte stream as a Input Stream. The
+ * returned stream is backed by buffers of <code>this</code> stream,
+ * avoiding memory allocation and copy, thus saving space and time.<br>
+ *
+ * @return the current contents of this output stream.
+ * @see java.io.ByteArrayOutputStream#toByteArray()
+ * @see #reset()
+ * @since 2.5
+ */
+ public abstract InputStream toInputStream();
+
+ /**
+ * Gets the current contents of this byte stream as a Input Stream. The
+ * returned stream is backed by buffers of <code>this</code> stream,
+ * avoiding memory allocation and copy, thus saving space and time.<br>
+ *
+ * @return the current contents of this output stream.
+ * @see java.io.ByteArrayOutputStream#toByteArray()
+ * @see #reset()
+ * @since 2.7
+ */
+ protected InputStream toInputStreamImpl() {
+ int remaining = count;
+ if (remaining == 0) {
+ return new ClosedInputStream();
+ }
+ final List<ByteArrayInputStream> list = new ArrayList<>(buffers.size());
+ for (final byte[] buf : buffers) {
+ final int c = Math.min(buf.length, remaining);
+ list.add(new ByteArrayInputStream(buf, 0, c));
+ remaining -= c;
+ if (remaining == 0) {
+ break;
+ }
+ }
+ reuseBuffers = false;
+ return new SequenceInputStream(Collections.enumeration(list));
+ }
+
+ /**
+ * Gets the current contents of this byte stream as a byte array.
+ * The result is independent of this stream.
+ *
+ * @return the current contents of this output stream, as a byte array
+ * @see java.io.ByteArrayOutputStream#toByteArray()
+ */
+ public abstract byte[] toByteArray();
+
+ /**
+ * Gets the current contents of this byte stream as a byte array.
+ * The result is independent of this stream.
+ *
+ * @return the current contents of this output stream, as a byte array
+ * @see java.io.ByteArrayOutputStream#toByteArray()
+ */
+ protected byte[] toByteArrayImpl() {
+ int remaining = count;
+ if (remaining == 0) {
+ return EMPTY_BYTE_ARRAY;
+ }
+ final byte newbuf[] = new byte[remaining];
+ int pos = 0;
+ for (final byte[] buf : buffers) {
+ final int c = Math.min(buf.length, remaining);
+ System.arraycopy(buf, 0, newbuf, pos, c);
+ pos += c;
+ remaining -= c;
+ if (remaining == 0) {
+ break;
+ }
+ }
+ return newbuf;
+ }
+
+ /**
+ * Gets the current contents of this byte stream as a string
+ * using the platform default charset.
+ * @return the contents of the byte array as a String
+ * @see java.io.ByteArrayOutputStream#toString()
+ * @deprecated 2.5 use {@link #toString(String)} instead
Review comment:
This was how it was previously in `ByteArrayOutputStream`. I just left it as I found it!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-610479408
> @garydgregory Hmm, from what I read in coveralls.io the code coverage after this PR is `+0.07%` so it is a net improvement, no?
Please run the command I provided and look at the JaCoCo report. You will see that the new class has less coverage than the current (refactored) one. We should strive to give the best possible code coverage for changes and new code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] aherbert commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
aherbert commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-609490381
Looking at the line errors for your `</p>` markers they are all at lines following a `<ul>` element that cannot be nested within a paragraph. Move the `</p>` to before the `<ul>`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] adamretter commented on a change in pull request #108:
Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
adamretter commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r402980055
##########
File path: pom.xml
##########
@@ -226,6 +230,11 @@ file comparators, endian transformation classes, and much more.
</contributors>
<dependencies>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
Review comment:
Well yes it is to bring in the annotation... but these annotations can also be used by various tools like FindBugs and others to detect issues.
I think it adds value, and jsr305 is tiny (19KB). I can remove it if people prefer? It also adds useful annotations like `@Nullable` and `@NotNull` for similar purposes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] aherbert commented on a change in pull request #108:
Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r402993724
##########
File path: pom.xml
##########
@@ -226,6 +230,11 @@ file comparators, endian transformation classes, and much more.
</contributors>
<dependencies>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
Review comment:
I would remove it from this PR. Adding an annotation dependency should be in another PR that perhaps then makes more use of it than a single location. The Guava codebase uses these throughout and in that case they have value for a bug detection tool. I don't think a single annotation of `@ThreadSafe` is likely to detect many (any?) bugs given that synchronized code should be less of a problem than unsynchronized code. If you do have a bug detection tool that flags this class as a problem when used in a concurrent scenario a brief look at the javadoc (which notes this is safe for concurrent use) will allow you to mark the bug as false positive.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls commented on issue #108: Refactor
ByteArrayOutputStream into synchronized and non-synchronized versions
Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#issuecomment-608372513
[](https://coveralls.io/builds/29821075)
Coverage decreased (-0.01%) to 89.394% when pulling **451cf801123dd8e858faf34bacfcc534496d3200 on adamretter:iostreams-sync** into **335808a8295c3d4dc1ec887b79f51699465889a6 on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] aherbert commented on a change in pull request #108:
Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r402927367
##########
File path: src/test/java/org/apache/commons/io/output/ByteArrayOutputStreamTestCase.java
##########
@@ -211,13 +217,30 @@ public void testStream() throws Exception {
//Make sure that empty ByteArrayOutputStreams really don't create garbage
//on toByteArray()
- final ByteArrayOutputStream baos1 = new ByteArrayOutputStream();
- final ByteArrayOutputStream baos2 = new ByteArrayOutputStream();
+ final AbstractByteArrayOutputStream baos1 = baosFactory.instance();
+ final AbstractByteArrayOutputStream baos2 = baosFactory.instance();
assertSame(baos1.toByteArray(), baos2.toByteArray());
baos1.close();
baos2.close();
baout.close();
baout1.close();
}
+
+ private static Stream<Arguments> baosFactories() {
+ final BAOSFactory syncBaosFactory = size -> new ByteArrayOutputStream(size);
+ final BAOSFactory nonSyncBaos = size -> new FastByteArrayOutputStream(size);
Review comment:
rename nonSyncBaosFactory
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on a change in pull request
#108: Refactor ByteArrayOutputStream into synchronized and non-synchronized
versions
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #108: Refactor ByteArrayOutputStream into synchronized and non-synchronized versions
URL: https://github.com/apache/commons-io/pull/108#discussion_r403321750
##########
File path: src/main/java/org/apache/commons/io/output/AbstractByteArrayOutputStream.java
##########
@@ -0,0 +1,391 @@
+/*
+ * 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.output;
+
+import org.apache.commons.io.input.ClosedInputStream;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.SequenceInputStream;
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.commons.io.IOUtils.EOF;
+
+/**
+ * This is the base class for implementing an output stream in which the data
+ * is written into a byte array. The buffer automatically grows as data
+ * is written to it.
+ * <p>
+ * The data can be retrieved using <code>toByteArray()</code> and
+ * <code>toString()</code>.
+ * <p>
Review comment:
Please close HTML tags.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services