From 979490ad97e899ffaa12e3aae3a3d35dc99142cb Mon Sep 17 00:00:00 2001 From: Andreas Schildbach Date: Sun, 19 Mar 2023 15:02:58 +0100 Subject: [PATCH] UnsafeByteArrayOutputStream: remove this performance optimisation Migrate all usages of `UnsafeByteArrayOutputStream` to `ByteArrayOutputStream`. --- .../main/java/org/bitcoinj/core/Block.java | 4 +- .../main/java/org/bitcoinj/core/Message.java | 2 +- .../java/org/bitcoinj/core/Transaction.java | 14 +- .../core/UnsafeByteArrayOutputStream.java | 138 ------------------ .../main/java/org/bitcoinj/script/Script.java | 11 +- .../bitcoinj/core/FullBlockTestGenerator.java | 14 +- .../java/org/bitcoinj/script/ScriptTest.java | 4 +- 7 files changed, 24 insertions(+), 163 deletions(-) delete mode 100644 core/src/main/java/org/bitcoinj/core/UnsafeByteArrayOutputStream.java diff --git a/core/src/main/java/org/bitcoinj/core/Block.java b/core/src/main/java/org/bitcoinj/core/Block.java index c52ad6396..c6b4ba9c4 100644 --- a/core/src/main/java/org/bitcoinj/core/Block.java +++ b/core/src/main/java/org/bitcoinj/core/Block.java @@ -392,7 +392,7 @@ public class Block extends Message { // At least one of the two cacheable components is invalid // so fall back to stream write since we can't be sure of the length. - ByteArrayOutputStream stream = new UnsafeByteArrayOutputStream(length == UNKNOWN_LENGTH ? HEADER_SIZE + guessTransactionsLength() : length); + ByteArrayOutputStream stream = new ByteArrayOutputStream(length == UNKNOWN_LENGTH ? HEADER_SIZE + guessTransactionsLength() : length); try { writeHeader(stream); writeTransactions(stream); @@ -462,7 +462,7 @@ public class Block extends Message { */ private Sha256Hash calculateHash() { try { - ByteArrayOutputStream bos = new UnsafeByteArrayOutputStream(HEADER_SIZE); + ByteArrayOutputStream bos = new ByteArrayOutputStream(HEADER_SIZE); writeHeader(bos); return Sha256Hash.wrapReversed(Sha256Hash.hashTwice(bos.toByteArray())); } catch (IOException e) { diff --git a/core/src/main/java/org/bitcoinj/core/Message.java b/core/src/main/java/org/bitcoinj/core/Message.java index 4c02351df..e6e5a8ba3 100644 --- a/core/src/main/java/org/bitcoinj/core/Message.java +++ b/core/src/main/java/org/bitcoinj/core/Message.java @@ -206,7 +206,7 @@ public abstract class Message { } // No cached array available so serialize parts by stream. - ByteArrayOutputStream stream = new UnsafeByteArrayOutputStream(length < 32 ? 32 : length + 32); + ByteArrayOutputStream stream = new ByteArrayOutputStream(length < 32 ? 32 : length + 32); try { bitcoinSerializeToStream(stream); } catch (IOException e) { diff --git a/core/src/main/java/org/bitcoinj/core/Transaction.java b/core/src/main/java/org/bitcoinj/core/Transaction.java index 66d24f545..1e38d1e90 100644 --- a/core/src/main/java/org/bitcoinj/core/Transaction.java +++ b/core/src/main/java/org/bitcoinj/core/Transaction.java @@ -293,7 +293,7 @@ public class Transaction extends ChildMessage { if (!hasWitnesses() && cachedWTxId != null) { cachedTxId = cachedWTxId; } else { - ByteArrayOutputStream stream = new UnsafeByteArrayOutputStream(length < 32 ? 32 : length + 32); + ByteArrayOutputStream stream = new ByteArrayOutputStream(length < 32 ? 32 : length + 32); try { bitcoinSerializeToStream(stream, false); } catch (IOException e) { @@ -339,7 +339,7 @@ public class Transaction extends ChildMessage { public int getWeight() { if (!hasWitnesses()) return getMessageSize() * 4; - try (final ByteArrayOutputStream stream = new UnsafeByteArrayOutputStream(length)) { + try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(length)) { bitcoinSerializeToStream(stream, false); final int baseSize = stream.size(); stream.reset(); @@ -1472,7 +1472,7 @@ public class Transaction extends ChildMessage { byte[] scriptCode, Coin prevValue, byte sigHashType){ - ByteArrayOutputStream bos = new UnsafeByteArrayOutputStream(length == UNKNOWN_LENGTH ? 256 : length + 4); + ByteArrayOutputStream bos = new ByteArrayOutputStream(length == UNKNOWN_LENGTH ? 256 : length + 4); try { byte[] hashPrevouts = new byte[32]; byte[] hashSequence = new byte[32]; @@ -1482,7 +1482,7 @@ public class Transaction extends ChildMessage { boolean signAll = (basicSigHashType != SigHash.SINGLE.value) && (basicSigHashType != SigHash.NONE.value); if (!anyoneCanPay) { - ByteArrayOutputStream bosHashPrevouts = new UnsafeByteArrayOutputStream(256); + ByteArrayOutputStream bosHashPrevouts = new ByteArrayOutputStream(256); for (TransactionInput input : this.inputs) { bosHashPrevouts.write(input.getOutpoint().getHash().getReversedBytes()); uint32ToByteStreamLE(input.getOutpoint().getIndex(), bosHashPrevouts); @@ -1491,7 +1491,7 @@ public class Transaction extends ChildMessage { } if (!anyoneCanPay && signAll) { - ByteArrayOutputStream bosSequence = new UnsafeByteArrayOutputStream(256); + ByteArrayOutputStream bosSequence = new ByteArrayOutputStream(256); for (TransactionInput input : this.inputs) { uint32ToByteStreamLE(input.getSequenceNumber(), bosSequence); } @@ -1499,7 +1499,7 @@ public class Transaction extends ChildMessage { } if (signAll) { - ByteArrayOutputStream bosHashOutputs = new UnsafeByteArrayOutputStream(256); + ByteArrayOutputStream bosHashOutputs = new ByteArrayOutputStream(256); for (TransactionOutput output : this.outputs) { uint64ToByteStreamLE( BigInteger.valueOf(output.getValue().getValue()), @@ -1510,7 +1510,7 @@ public class Transaction extends ChildMessage { } hashOutputs = Sha256Hash.hashTwice(bosHashOutputs.toByteArray()); } else if (basicSigHashType == SigHash.SINGLE.value && inputIndex < outputs.size()) { - ByteArrayOutputStream bosHashOutputs = new UnsafeByteArrayOutputStream(256); + ByteArrayOutputStream bosHashOutputs = new ByteArrayOutputStream(256); uint64ToByteStreamLE( BigInteger.valueOf(this.outputs.get(inputIndex).getValue().getValue()), bosHashOutputs diff --git a/core/src/main/java/org/bitcoinj/core/UnsafeByteArrayOutputStream.java b/core/src/main/java/org/bitcoinj/core/UnsafeByteArrayOutputStream.java deleted file mode 100644 index 726300a84..000000000 --- a/core/src/main/java/org/bitcoinj/core/UnsafeByteArrayOutputStream.java +++ /dev/null @@ -1,138 +0,0 @@ -/* - * Copyright 2011 Steve Coughlan. - * - * Licensed 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.bitcoinj.core; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.OutputStream; - -/** - *

An unsynchronized implementation of ByteArrayOutputStream that will return the backing byte array if its length == size(). - * This avoids unneeded array copy where the BOS is simply being used to extract a byte array of known length from a - * 'serialized to stream' method.

- * - *

Unless the final length can be accurately predicted the only performance this will yield is due to unsynchronized - * methods.

- * - * @author git - */ -public class UnsafeByteArrayOutputStream extends ByteArrayOutputStream { - - public UnsafeByteArrayOutputStream() { - super(32); - } - - public UnsafeByteArrayOutputStream(int size) { - super(size); - } - - /** - * Writes the specified byte to this byte array output stream. - * - * @param b the byte to be written. - */ - @Override - public void write(int b) { - int newcount = count + 1; - if (newcount > buf.length) { - buf = copyOf(buf, Math.max(buf.length << 1, newcount)); - } - buf[count] = (byte) b; - count = newcount; - } - - /** - * Writes {@code len} bytes from the specified byte array - * starting at offset {@code off} to this byte array output stream. - * - * @param b the data. - * @param off the start offset in the data. - * @param len the number of bytes to write. - */ - @Override - public void write(byte[] b, int off, int len) { - if ((off < 0) || (off > b.length) || (len < 0) || - ((off + len) > b.length) || ((off + len) < 0)) { - throw new IndexOutOfBoundsException(); - } else if (len == 0) { - return; - } - int newcount = count + len; - if (newcount > buf.length) { - buf = copyOf(buf, Math.max(buf.length << 1, newcount)); - } - System.arraycopy(b, off, buf, count, len); - count = newcount; - } - - /** - * Writes the complete contents of this byte array output stream to - * the specified output stream argument, as if by calling the output - * stream's write method using {@code out.write(buf, 0, count)}. - * - * @param out the output stream to which to write the data. - * @throws IOException if an I/O error occurs. - */ - @Override - public void writeTo(OutputStream out) throws IOException { - out.write(buf, 0, count); - } - - /** - * Resets the {@code count} field of this byte array output - * stream to zero, so that all currently accumulated output in the - * output stream is discarded. The output stream can be used again, - * reusing the already allocated buffer space. - * - * @see java.io.ByteArrayInputStream#count - */ - @Override - public void reset() { - count = 0; - } - - /** - * Creates a newly allocated byte array. Its size is the current - * size of this output stream and the valid contents of the buffer - * have been copied into it. - * - * @return the current contents of this output stream, as a byte array. - * @see java.io.ByteArrayOutputStream#size() - */ - @Override - public byte toByteArray()[] { - return count == buf.length ? buf : copyOf(buf, count); - } - - /** - * Returns the current size of the buffer. - * - * @return the value of the {@code count} field, which is the number - * of valid bytes in this output stream. - * @see java.io.ByteArrayOutputStream#count - */ - @Override - public int size() { - return count; - } - - private static byte[] copyOf(byte[] in, int length) { - byte[] out = new byte[length]; - System.arraycopy(in, 0, out, 0, Math.min(length, in.length)); - return out; - } -} diff --git a/core/src/main/java/org/bitcoinj/script/Script.java b/core/src/main/java/org/bitcoinj/script/Script.java index 5472608aa..0e6581c3f 100644 --- a/core/src/main/java/org/bitcoinj/script/Script.java +++ b/core/src/main/java/org/bitcoinj/script/Script.java @@ -37,7 +37,6 @@ import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionInput; import org.bitcoinj.core.TransactionOutput; import org.bitcoinj.core.TransactionWitness; -import org.bitcoinj.core.UnsafeByteArrayOutputStream; import org.bitcoinj.core.Utils; import org.bitcoinj.base.VarInt; import org.bitcoinj.core.VerificationException; @@ -489,7 +488,7 @@ public class Script { public static byte[] createInputScript(byte[] signature, byte[] pubkey) { try { // TODO: Do this by creating a Script *first* then having the script reassemble itself into bytes. - ByteArrayOutputStream bits = new UnsafeByteArrayOutputStream(signature.length + pubkey.length + 2); + ByteArrayOutputStream bits = new ByteArrayOutputStream(signature.length + pubkey.length + 2); writeBytes(bits, signature); writeBytes(bits, pubkey); return bits.toByteArray(); @@ -501,7 +500,7 @@ public class Script { public static byte[] createInputScript(byte[] signature) { try { // TODO: Do this by creating a Script *first* then having the script reassemble itself into bytes. - ByteArrayOutputStream bits = new UnsafeByteArrayOutputStream(signature.length + 2); + ByteArrayOutputStream bits = new ByteArrayOutputStream(signature.length + 2); writeBytes(bits, signature); return bits.toByteArray(); } catch (IOException e) { @@ -787,7 +786,7 @@ public class Script { */ public static byte[] removeAllInstancesOf(byte[] inputScript, byte[] chunkToRemove) { // We usually don't end up removing anything - UnsafeByteArrayOutputStream bos = new UnsafeByteArrayOutputStream(inputScript.length); + ByteArrayOutputStream bos = new ByteArrayOutputStream(inputScript.length); int cursor = 0; while (cursor < inputScript.length) { @@ -1522,7 +1521,7 @@ public class Script { byte[] prog = script.getProgram(); byte[] connectedScript = Arrays.copyOfRange(prog, lastCodeSepLocation, prog.length); - UnsafeByteArrayOutputStream outStream = new UnsafeByteArrayOutputStream(sigBytes.length + 1); + ByteArrayOutputStream outStream = new ByteArrayOutputStream(sigBytes.length + 1); try { writeBytes(outStream, sigBytes); } catch (IOException e) { @@ -1599,7 +1598,7 @@ public class Script { byte[] connectedScript = Arrays.copyOfRange(prog, lastCodeSepLocation, prog.length); for (byte[] sig : sigs) { - UnsafeByteArrayOutputStream outStream = new UnsafeByteArrayOutputStream(sig.length + 1); + ByteArrayOutputStream outStream = new ByteArrayOutputStream(sig.length + 1); try { writeBytes(outStream, sig); } catch (IOException e) { diff --git a/core/src/test/java/org/bitcoinj/core/FullBlockTestGenerator.java b/core/src/test/java/org/bitcoinj/core/FullBlockTestGenerator.java index 301908f8f..f37f0af9e 100644 --- a/core/src/test/java/org/bitcoinj/core/FullBlockTestGenerator.java +++ b/core/src/test/java/org/bitcoinj/core/FullBlockTestGenerator.java @@ -675,7 +675,7 @@ public class FullBlockTestGenerator { int b39numP2SHOutputs = 0, b39sigOpsPerOutput = 6; NewBlock b39 = createNextBlock(b35, chainHeadHeight + 12, null, null); { - ByteArrayOutputStream p2shScriptPubKey = new UnsafeByteArrayOutputStream(); + ByteArrayOutputStream p2shScriptPubKey = new ByteArrayOutputStream(); try { Script.writeBytes(p2shScriptPubKey, coinbaseOutKeyPubKey); p2shScriptPubKey.write(OP_2DUP); @@ -695,7 +695,7 @@ public class FullBlockTestGenerator { b39p2shScriptPubKey = p2shScriptPubKey.toByteArray(); byte[] scriptHash = CryptoUtils.sha256hash160(b39p2shScriptPubKey); - UnsafeByteArrayOutputStream scriptPubKey = new UnsafeByteArrayOutputStream(scriptHash.length + 3); + ByteArrayOutputStream scriptPubKey = new ByteArrayOutputStream(scriptHash.length + 3); scriptPubKey.write(OP_HASH160); try { Script.writeBytes(scriptPubKey, scriptHash); @@ -767,12 +767,12 @@ public class FullBlockTestGenerator { // Sign input try { - ByteArrayOutputStream bos = new UnsafeByteArrayOutputStream(73); + ByteArrayOutputStream bos = new ByteArrayOutputStream(73); bos.write(coinbaseOutKey.sign(hash).encodeToDER()); bos.write(SigHash.SINGLE.value); byte[] signature = bos.toByteArray(); - ByteArrayOutputStream scriptSigBos = new UnsafeByteArrayOutputStream(signature.length + b39p2shScriptPubKey.length + 3); + ByteArrayOutputStream scriptSigBos = new ByteArrayOutputStream(signature.length + b39p2shScriptPubKey.length + 3); Script.writeBytes(scriptSigBos, new byte[] {(byte) OP_CHECKSIG}); scriptSigBos.write(Script.createInputScript(signature)); Script.writeBytes(scriptSigBos, b39p2shScriptPubKey); @@ -837,13 +837,13 @@ public class FullBlockTestGenerator { // Sign input try { - ByteArrayOutputStream bos = new UnsafeByteArrayOutputStream( + ByteArrayOutputStream bos = new ByteArrayOutputStream( 73); bos.write(coinbaseOutKey.sign(hash).encodeToDER()); bos.write(SigHash.SINGLE.value); byte[] signature = bos.toByteArray(); - ByteArrayOutputStream scriptSigBos = new UnsafeByteArrayOutputStream( + ByteArrayOutputStream scriptSigBos = new ByteArrayOutputStream( signature.length + b39p2shScriptPubKey.length + 3); Script.writeBytes(scriptSigBos, @@ -1216,7 +1216,7 @@ public class FullBlockTestGenerator { b64Original.solve(); checkState(b64Original.block.getMessageSize() == Block.MAX_BLOCK_SIZE); - UnsafeByteArrayOutputStream stream = new UnsafeByteArrayOutputStream(b64Original.block.getMessageSize() + 8); + ByteArrayOutputStream stream = new ByteArrayOutputStream(b64Original.block.getMessageSize() + 8); b64Original.block.writeHeader(stream); byte[] varIntBytes = new byte[9]; diff --git a/core/src/test/java/org/bitcoinj/script/ScriptTest.java b/core/src/test/java/org/bitcoinj/script/ScriptTest.java index 1f514c49b..55f9ea6e3 100644 --- a/core/src/test/java/org/bitcoinj/script/ScriptTest.java +++ b/core/src/test/java/org/bitcoinj/script/ScriptTest.java @@ -39,7 +39,6 @@ import org.bitcoinj.core.Transaction.SigHash; import org.bitcoinj.core.TransactionInput; import org.bitcoinj.core.TransactionOutPoint; import org.bitcoinj.core.TransactionOutput; -import org.bitcoinj.core.UnsafeByteArrayOutputStream; import org.bitcoinj.core.VerificationException; import org.bitcoinj.crypto.TransactionSignature; import org.bitcoinj.params.MainNetParams; @@ -50,6 +49,7 @@ import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStreamReader; import java.math.BigInteger; @@ -258,7 +258,7 @@ public class ScriptTest { private Script parseScriptString(String string) throws IOException { String[] words = string.split("[ \\t\\n]"); - UnsafeByteArrayOutputStream out = new UnsafeByteArrayOutputStream(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); for(String w : words) { if (w.equals(""))