Patch 5 from Steves lazy parsing patchset:

Optimise BitcoinSerialiser for Transactions.  When calculating checksum on deserialize use it prepopulate the transaction's hash.  Likewise on serialize check if the Transaction already has a hash and use that to write checksum bytes.  This yields performance improvesment up to 400% by saving on a double hash.

Don't parse all the subcomponents of a Transaction purely to calculate its length, instead do the minimal work possible.

Recaching on a call to bitcoinSerialise().  Prevents double serialization of transactions and inputs/outputs when calculating a merkleroot during block serialization.  Also makes it more likely the original larger byte array can be GC'd

src/com/google/bitcoin/core/BitcoinSerializer.java

@@ -24,6 +24,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.UnsupportedEncodingException;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -159,8 +160,22 @@ public class BitcoinSerializer {
         Utils.uint32ToByteArrayLE(payload.length, header, 4 + COMMAND_LEN);
 
         if (usesChecksumming) {
-            byte[] hash = doubleDigest(payload);
+            Sha256Hash msgHash = message.getHash();
-            System.arraycopy(hash, 0, header, 4 + COMMAND_LEN + 4, 4);
+        	if (msgHash != null && message instanceof Transaction) {
+        		//if the message happens to have a precalculated hash use it.
+        		//reverse copying 4 bytes is about 1600 times faster than
+        		//calculating a new hash
+        		//this is only possible for transactions as block hashes
+        		//are hashes of the header only
+        		byte[] hash = msgHash.getBytes();
+        		int start = 4 + COMMAND_LEN + 4;
+                for (int i = start; i < start + 4; i++)
+                    header[i] = hash[31 - i + start];
+                
+        	} else {
+        		byte[] hash = doubleDigest(payload);
+        		System.arraycopy(hash, 0, header, 4 + COMMAND_LEN + 4, 4);
+        	}
         }
 
         out.write(header);
@@ -249,8 +264,9 @@ public class BitcoinSerializer {
         }
 
         // Verify the checksum.
+        byte[] hash = null;
         if (usesChecksumming) {
-            byte[] hash = doubleDigest(payloadBytes);
+        	hash = doubleDigest(payloadBytes);
             if (header.checksum[0] != hash[0] || header.checksum[1] != hash[1] ||
                     header.checksum[2] != hash[2] || header.checksum[3] != hash[3]) {
                 throw new ProtocolException("Checksum failed to verify, actual " +
@@ -268,24 +284,27 @@ public class BitcoinSerializer {
         }
 
         try {
-            return makeMessage(header.command, header.size, payloadBytes);
+            return makeMessage(header.command, header.size, payloadBytes, hash);
         } catch (Exception e) {
             throw new ProtocolException("Error deserializing message " + Utils.bytesToHexString(payloadBytes) + "\n", e);
         }
     }
 
-    private Message makeMessage(String command, int length, byte[] payloadBytes) throws ProtocolException {
+    private Message makeMessage(String command, int length, byte[] payloadBytes, byte[] hash) throws ProtocolException {
         // We use an if ladder rather than reflection because reflection is very slow on Android.
         if (command.equals("version")) {
             return new VersionMessage(params, payloadBytes);
         } else if (command.equals("inv")) {
             return new InventoryMessage(params, payloadBytes, parseLazy, parseRetain, length);
         } else if (command.equals("block")) {
-            return new Block(params, payloadBytes, parseLazy, parseRetain, length);
+        	return new Block(params, payloadBytes, parseLazy, parseRetain, length);
         } else if (command.equals("getdata")) {
             return new GetDataMessage(params, payloadBytes, parseLazy, parseRetain, length);
         } else if (command.equals("tx")) {
-            return new Transaction(params, payloadBytes, null, parseLazy, parseRetain, length);
+        	Transaction tx = new Transaction(params, payloadBytes, null, parseLazy, parseRetain, length);
+        	if (hash != null)
+        		tx.setHash(new Sha256Hash(Utils.reverseBytes(hash)));
+        	return tx;
         } else if (command.equals("addr")) {
             return new AddressMessage(params, payloadBytes, parseLazy, parseRetain, length);
         } else if (command.equals("ping")) {

src/com/google/bitcoin/core/Message.java

@@ -53,6 +53,7 @@ public abstract class Message implements Serializable {
     protected transient byte[] bytes;
     
     protected transient boolean parsed = false;
+    protected transient boolean recached = false;
     protected transient final boolean parseLazy;
     protected transient final boolean parseRetain;
 
@@ -203,11 +204,9 @@ public abstract class Message implements Serializable {
     	 *     to keep track of whether the cache is valid or not.
     	 */
 
-    	
+    	checkParse();
-    	if (parseRetain) {
+    	bytes = null;
-    		checkParse();
+    	recached = false;
-    		bytes = null;
-    	}
     }
     
     /**
@@ -225,6 +224,10 @@ public abstract class Message implements Serializable {
     	return bytes != null;
     }
     
+    public boolean isRecached() {
+    	return recached;
+    }
+    
     /**
      * Serialize this message to a byte array that conforms to the bitcoin wire protocol.
      * <br/>
@@ -251,9 +254,9 @@ public abstract class Message implements Serializable {
     			return bytes;
     		}
     		
-    		int len = cursor - offset;
+    		//int len = cursor - offset;
-    		byte[] buf = new byte[len];
+    		byte[] buf = new byte[length];
-    		System.arraycopy(bytes, offset, buf, 0, len);
+    		System.arraycopy(bytes, offset, buf, 0, length);
     		return buf;
     	}
     	
@@ -266,6 +269,24 @@ public abstract class Message implements Serializable {
         } catch (IOException e) {
             // Cannot happen, we are serializing to a memory stream.
         }
+        
+        if (parseRetain) {
+        	//a free set of steak knives!
+        	//If there happens to be a call to this method we gain an opportunity to recache
+        	//the byte array and in this case it contains no bytes from parent messages.
+        	//This give a dual benefit.  Releasing references to the larger byte array so that it
+        	//it is more likely to be GC'd.  A preventing double serializations.  E.g. calculating
+        	//merkle root calls this method.  It is will frequently happen prior to serializing the block
+        	//which means another call to bitcoinSerialize is coming.  If we didn't recache then internal
+        	//serialization would occur a 2nd time and every subsequent time the message is serialized.
+        	bytes = stream.toByteArray();
+        	cursor = cursor - offset;
+        	offset = 0;
+        	recached = true;
+        	length = bytes.length;
+        	return bytes;
+        }
+        
         return stream.toByteArray();
     }
     
@@ -291,6 +312,15 @@ public abstract class Message implements Serializable {
     	log.debug("Warning: {} class has not implemented bitcoinSerializeToStream method.  Generating message with no payload", getClass());
     }
     
+    /**
+     * This method is a NOP for all classes except Block and Transaction.  It is only declared in Message
+     * so BitcoinSerializer can avoid 2 instanceof checks + a casting.
+     * @return
+     */
+    public Sha256Hash getHash() {
+    	return null;
+    }
+    
     /**
      * This should be overidden to extract correct message size in the case of lazy parsing.  Until this method is
      * implemented in a subclass of ChildMessage lazy parsing will have no effect.
@@ -339,7 +369,7 @@ public abstract class Message implements Serializable {
     }
     
     long readVarInt(int offset) {
-        VarInt varint = new VarInt(bytes, cursor + offset);
+    	VarInt varint = new VarInt(bytes, cursor + offset);
         cursor += offset + varint.getSizeInBytes();
         return varint.value;
     }

src/com/google/bitcoin/core/Transaction.java

@@ -122,6 +122,16 @@ public class Transaction extends ChildMessage implements Serializable {
         }
         return hash;
     }
+    
+	/**
+	 * Used by BitcoinSerializer.  The serializer has to calculate a hash for checksumming so to
+	 * avoid wasting the considerable effort a set method is provided so the serializer can set it.
+	 * 
+	 * No verification is performed on this hash.
+	 */
+	void setHash(Sha256Hash hash) {
+		this.hash = hash;
+	}
 
     public String getHashAsString() {
         return getHash().toString();
@@ -132,7 +142,8 @@ public class Transaction extends ChildMessage implements Serializable {
      * include spent outputs or not.
      */
     BigInteger getValueSentToMe(Wallet wallet, boolean includeSpent) {
-        // This is tested in WalletTest.
+        checkParse();
+    	// This is tested in WalletTest.
         BigInteger v = BigInteger.ZERO;
         for (TransactionOutput o : outputs) {
             if (!o.isMine(wallet)) continue;
@@ -181,7 +192,8 @@ public class Transaction extends ChildMessage implements Serializable {
      * @return sum in nanocoins.
      */
     public BigInteger getValueSentFromMe(Wallet wallet) throws ScriptException {
-        // This is tested in WalletTest.
+    	checkParse();
+    	// This is tested in WalletTest.
         BigInteger v = BigInteger.ZERO;
         for (TransactionInput input : inputs) {
             // This input is taking value from an transaction in our wallet. To discover the value,
@@ -204,6 +216,7 @@ public class Transaction extends ChildMessage implements Serializable {
 
     boolean disconnectInputs() {
         boolean disconnected = false;
+        checkParse();
         for (TransactionInput input : inputs) {
             disconnected |= input.disconnect();
         }
@@ -215,7 +228,8 @@ public class Transaction extends ChildMessage implements Serializable {
      * null on success.
      */
     TransactionInput connectForReorganize(Map<Sha256Hash, Transaction> transactions) {
-        for (TransactionInput input : inputs) {
+        checkParse();
+    	for (TransactionInput input : inputs) {
             // Coinbase transactions, by definition, do not have connectable inputs.
             if (input.isCoinBase()) continue;
             TransactionInput.ConnectionResult result = input.connect(transactions, false);
@@ -235,7 +249,8 @@ public class Transaction extends ChildMessage implements Serializable {
      * @return true if every output is marked as spent.
      */
     public boolean isEveryOutputSpent() {
-        for (TransactionOutput output : outputs) {
+        checkParse();
+    	for (TransactionOutput output : outputs) {
             if (output.isAvailableForSpending())
                 return false;
         }
@@ -281,6 +296,11 @@ public class Transaction extends ChildMessage implements Serializable {
         NONE,        // 2
         SINGLE,      // 3
     }
+    
+    protected void unCache() {
+    	super.unCache();
+    	hash = null;
+    }
 
     protected void parseLite() throws ProtocolException {
     	
@@ -297,16 +317,51 @@ public class Transaction extends ChildMessage implements Serializable {
     		//of the various components gains us the ability to cache the backing bytearrays
     		//so that only those subcomponents that have changed will need to be reserialized.
     	
-    		parse();
+    		//parse();
-    		parsed = true;
+    		//parsed = true;
+    		length = calcLength(bytes, cursor, offset);
+    		cursor = offset + length;
     	}
     }
     
+    protected static int calcLength(byte[] buf, int cursor, int offset) {
+    	VarInt varint;
+    	cursor = offset + 4;
+    	
+    	int i;
+    	long scriptLen;
+    	
+    	varint = new VarInt(buf, cursor);
+    	long txInCount = varint.value;
+    	cursor += varint.getSizeInBytes();
+    	
+    	for (i = 0; i < txInCount; i++) {
+    		cursor += 36;
+    		varint = new VarInt(buf, cursor);
+    		scriptLen = varint.value;
+    		cursor += scriptLen + 4 + varint.getSizeInBytes();
+    	}
+    	
+    	varint = new VarInt(buf, cursor);
+    	long txOutCount = varint.value;
+    	cursor += varint.getSizeInBytes();
+    	
+    	for (i = 0; i < txOutCount; i++) {
+    		cursor += 8;
+    		varint = new VarInt(buf, cursor);
+    		scriptLen = varint.value;
+    		cursor += scriptLen + varint.getSizeInBytes();
+    	}
+    	return cursor - offset + 4;
+    }
+    
     void parse() throws ProtocolException {
     	
     	if (parsed)
     		return;
     	
+    	cursor = offset;
+    	
     	version = readUint32();
     	int marker = cursor;
     	
@@ -337,7 +392,8 @@ public class Transaction extends ChildMessage implements Serializable {
      * position in a block but by the data in the inputs.
      */
     public boolean isCoinBase() {
-        return inputs.get(0).isCoinBase();
+        checkParse();
+    	return inputs.get(0).isCoinBase();
     }
 
     /**

src/com/google/bitcoin/core/TransactionInput.java

@@ -108,7 +108,8 @@ public class TransactionInput extends ChildMessage implements Serializable {
      * Coinbase transactions have special inputs with hashes of zero. If this is such an input, returns true.
      */
     public boolean isCoinBase() {
-        return outpoint.getHash().equals(Sha256Hash.ZERO_HASH);
+        checkParse();
+    	return outpoint.getHash().equals(Sha256Hash.ZERO_HASH);
     }
 
     /**

tests/com/google/bitcoin/core/LazyParseByteCacheTest.java

@@ -219,12 +219,16 @@ public class LazyParseByteCacheTest {
     	if (b1.getTransactions().size() > 0) {
     		assertTrue(b1.isParsedTransactions());
     		Transaction tx1 = b1.getTransactions().get(0);
-    		if (lazy == tx1.isParsed())
+    		
-    			System.out.print("");
     		//this will always be true for all children of a block once they are retrieved.
     		//the tx child inputs/outputs may not be parsed however.
-    		assertEquals(true, tx1.isParsed());
+    		
-        	assertEquals(retain, tx1.isCached());	
+    		//no longer forced to parse if length not provided.
+    		//assertEquals(true, tx1.isParsed());
+        	if (tx1.isParsed())
+        		assertEquals(retain, tx1.isCached());
+        	else
+        		assertTrue(tx1.isCached());
         	
         	//does it still match ref block?
         	serDeser(bs, b1, bos.toByteArray(), null, null);
@@ -259,8 +263,14 @@ public class LazyParseByteCacheTest {
     	if (b1.getTransactions().size() > 0) {
     		assertTrue(b1.isParsedTransactions());
     		Transaction tx1 = b1.getTransactions().get(0);
-    		assertEquals(true, tx1.isParsed());
+    		
-        	assertEquals(retain, tx1.isCached());	
+    		//no longer forced to parse if length not provided.
+    		//assertEquals(true, tx1.isParsed());
+        	
+        	if (tx1.isParsed())
+        		assertEquals(retain, tx1.isCached());
+        	else
+        		assertTrue(tx1.isCached());	
         }
     	//does it still match ref block?
     	serDeser(bs, b1, bos.toByteArray(), null, null);
@@ -310,6 +320,8 @@ public class LazyParseByteCacheTest {
         	serDeser(bs, b1, bos.toByteArray(), null, null);
         }
     	
+    	if (lazy && retain)
+    		System.out.print("");
     	//refresh block
     	b1 = (Block) bs.deserialize(new ByteArrayInputStream(blockBytes));
     	bRef = (Block) bsRef.deserialize(new ByteArrayInputStream(blockBytes));
@@ -337,7 +349,7 @@ public class LazyParseByteCacheTest {
         		}
         		
         		//this has to be false. Altering a tx invalidates the merkle root.
-        		//when we have seperate merkle caching then the entire won't need to be
+        		//when we have seperate merkle caching then the entire header won't need to be
         		//invalidated.
         		assertFalse(b1.isHeaderBytesValid());
         		
@@ -367,7 +379,7 @@ public class LazyParseByteCacheTest {
     		Transaction tx2 = b2.getTransactions().get(0);
     		
         	if (tx1.getInputs().size() > 0) {
-        		if (lazy && retain)
+        		if (lazy && !retain)
         			System.out.print("");
         		TransactionInput fromTx1 = tx1.getInputs().get(0);
         		tx2.addInput(fromTx1);
@@ -463,7 +475,7 @@ public class LazyParseByteCacheTest {
     	
     	//add an input
     	if (t1.getInputs().size() > 0) {
-    		if (lazy && !retain)
+    		if (lazy && retain)
     			System.out.print("");
     		
     		t1.addInput(t1.getInputs().get(0));
@@ -495,7 +507,8 @@ public class LazyParseByteCacheTest {
 		if (!message.equals(m2)) {
 			System.out.print("");
 		}
-    	assertEquals(message, m2);
+    	
+		assertEquals(message, m2);
  
     	bos.reset();
     	bs.serialize(m2, bos);

tests/com/google/bitcoin/core/SpeedTest.java

@@ -73,16 +73,20 @@ public class SpeedTest {
 	private byte[] tx2BytesWithHeader;
 
 	List<SerializerEntry> bss;
+	List<SerializerEntry> singleBs;
 	List<Manipulator<Transaction>> txMans = new ArrayList();
 	List<Manipulator<Block>> blockMans = new ArrayList();
 	List<Manipulator<AddressMessage>> addrMans = new ArrayList();
+	List<Manipulator> genericMans = new ArrayList();
 
 	public static void main(String[] args) throws Exception {
 		SpeedTest test = new SpeedTest();
 		test.setUp();
-		test.start(10000, 10000, false);
+		test.start(50000, 50000, false);
 	}
 
+	public static final boolean RECACHE = false;
+	
 	public void start(int warmupIterations, int iterations, boolean pauseForKeyPress) {
 
 		if (pauseForKeyPress) {
@@ -113,6 +117,16 @@ public class SpeedTest {
 
 		junk = null;
 
+		System.out.println("******************************");
+		System.out.println("***      Generic Tests     ***");
+		System.out.println("******************************");
+		for (Manipulator<AddressMessage> man : genericMans) {
+			testManipulator(man, "warmup", warmupIterations * 10, singleBs, null, b1Bytes);
+		}
+		for (Manipulator<AddressMessage> man : genericMans) {
+			testManipulator(man, "main test", iterations * 10, singleBs, null, b1Bytes);
+		}
+		
 		System.out.println("******************************");
 		System.out.println("***      WARMUP PHASE      ***");
 		System.out.println("******************************");
@@ -193,6 +207,7 @@ public class SpeedTest {
 
 		bss = new ArrayList();
 		bss.add(new SerializerEntry(bs, "Standard (Non-lazy, No cached)"));
+		singleBs = new ArrayList(bss);
 		// add 2 because when profiling the first seems to take a lot longer
 		// than usual.
 		// bss.add(new SerializerEntry(bs, "Standard (Non-lazy, No cached)"));
@@ -204,6 +219,72 @@ public class SpeedTest {
 	}
 
 	private void buildManipulators() {
+		
+		Manipulator reverseBytes = new Manipulator<AddressMessage>() {
+
+			byte[] bytes = new byte[32];
+			
+			@Override
+			public void manipulate(BitcoinSerializer bs, AddressMessage message) throws Exception {
+				Utils.reverseBytes(bytes);
+			}
+
+			@Override
+			public void manipulate(BitcoinSerializer bs, byte[] bytes) throws Exception {
+			}
+
+			@Override
+			public String getDescription() {
+				return "Reverse 32 bytes";
+			}
+
+		};
+		genericMans.add(reverseBytes);
+		
+		Manipulator doubleDigest32Bytes = new Manipulator<AddressMessage>() {
+
+			byte[] bytes = new byte[32];
+			
+			@Override
+			public void manipulate(BitcoinSerializer bs, AddressMessage message) throws Exception {
+				Utils.doubleDigest(bytes);
+			}
+
+			@Override
+			public void manipulate(BitcoinSerializer bs, byte[] bytes) throws Exception {
+			}
+
+			@Override
+			public String getDescription() {
+				return "Double Digest 32 bytes";
+			}
+
+		};
+		genericMans.add(doubleDigest32Bytes);
+		
+		Manipulator doubleDigestBytes = new Manipulator<AddressMessage>() {
+
+			int len = -1;
+			
+			@Override
+			public void manipulate(BitcoinSerializer bs, AddressMessage message) throws Exception {
+				
+			}
+
+			@Override
+			public void manipulate(BitcoinSerializer bs, byte[] bytes) throws Exception {
+				if (len == -1)
+					len = bytes.length;
+				Utils.doubleDigest(bytes);
+			}
+
+			@Override
+			public String getDescription() {
+				return "Double Digest " + len + " bytes";
+				
+			}
+		};
+		genericMans.add(doubleDigestBytes);
 
 		Manipulator seralizeAddr = new Manipulator<AddressMessage>() {
 
@@ -369,6 +450,55 @@ public class SpeedTest {
 
 		};
 		txMans.add(deSeralizeTx);
+		
+		Manipulator serDeeralizeTx_1 = new Manipulator<Transaction>() {
+
+			ByteArrayOutputStream bos = new ByteArrayOutputStream();
+			
+			@Override
+			public void manipulate(BitcoinSerializer bs, Transaction message) throws Exception {
+			}
+
+			@Override
+			public void manipulate(BitcoinSerializer bs, byte[] bytes) throws Exception {
+				ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+				Transaction tx = (Transaction) bs.deserialize(bis);
+				bos.reset();
+				bs.serialize(tx, bos);
+			}
+
+			@Override
+			public String getDescription() {
+				return "Deserialize Transaction, Serialize";
+			}
+
+		};
+		txMans.add(serDeeralizeTx_1);
+		
+		Manipulator serDeeralizeTx = new Manipulator<Transaction>() {
+
+			ByteArrayOutputStream bos = new ByteArrayOutputStream();
+			
+			@Override
+			public void manipulate(BitcoinSerializer bs, Transaction message) throws Exception {
+			}
+
+			@Override
+			public void manipulate(BitcoinSerializer bs, byte[] bytes) throws Exception {
+				ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+				Transaction tx = (Transaction) bs.deserialize(bis);
+				tx.addInput(tx.getInputs().get(0));
+				bos.reset();
+				bs.serialize(tx, bos);
+			}
+
+			@Override
+			public String getDescription() {
+				return "Deserialize Transaction, modify, Serialize";
+			}
+
+		};
+		txMans.add(serDeeralizeTx);
 
 		Manipulator deSeralizeTx_1 = new Manipulator<Transaction>() {
 
@@ -728,7 +858,7 @@ public class SpeedTest {
 			M message, byte[] bytes) {
 		long allStart = System.currentTimeMillis();
 		System.out.println("Beginning " + phaseName + " run for manipulator: [" + man.getDescription() + "]");
-		int pause = iterations / 20;
+		int pause = iterations / 100;
 		pause = pause < 200 ? 200 : pause;
 		pause = pause > 1000 ? 1000 : pause;
 		long bestTime = Long.MAX_VALUE;