Bug 532118 - Reduce number of calls to java.security.MessageDigest.update(byte)

MD5Verifier and ChecksumVerifier are responsible for calculating
artifact's checksum and must extend ProcessingStep to fulfill API
requirements. ProcessingStep is effectively an unbuffered OutputStream
with a method to process a single byte of artifact's data.

In respect to MessageDigest, this means updating digest with every new
single byte of data. So, for 16 Kb of data, both classes will call
MessageDigest's update(byte) 16K times while only 1 call is enough.

Introduce MessageDigestProcessingStep that accumulates incoming bytes
in the internal buffer and immediately forwards them to the
destination stream. As soon as buffer is full, it will update
MessageDigest.

AbstractBufferingStep could be used as a base class instead but it is
too risky to adapt it. java.nio.ByteBuffer fits the bill almost
ideally. One caveat is it communicates 'buffer is full' by throwing
BufferOverflowException.

There are no specific reasons for buffer's size of 16K except an
assumption that it should not cause any problems: installing 1000
artifacts with 2 checksums per artifact would increase memory
footprint for about 3200K.

Similar approach has been used already [1], [2], claiming 5x speedup.

This should make [3] less of a problem.

[1] 5b2f061cd8ecf96a37783657e7ee7a0110c9d26d
[2] Bug 405716
[3] Bug 532036

Change-Id: I1ba0d36267b3f07103c22b93f92c41765e33c8cc
Signed-off-by: Mykola Nikishov <mn@mn.com.ua>

3 files changed, 80 insertions, 36 deletions

diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumVerifier.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumVerifier.java
index 2aa55ea9c..ed0430350 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumVerifier.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumVerifier.java
@@ -10,7 +10,6 @@
  *******************************************************************************/
 package org.eclipse.equinox.internal.p2.artifact.processors.checksum;
 
-import java.io.IOException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.Optional;
@@ -18,17 +17,15 @@ import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.equinox.internal.p2.artifact.repository.Activator;
 import org.eclipse.equinox.internal.p2.repository.helpers.ChecksumHelper;
-import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStep;
 import org.eclipse.equinox.p2.core.IProvisioningAgent;
 import org.eclipse.equinox.p2.core.ProvisionException;
 import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor;
 import org.eclipse.equinox.p2.repository.artifact.IProcessingStepDescriptor;
 import org.eclipse.osgi.util.NLS;
 
-final public class ChecksumVerifier extends ProcessingStep {
+final public class ChecksumVerifier extends MessageDigestProcessingStep {
 
 	private String expectedChecksum;
-	private MessageDigest messageDigest;
 	private String algorithmName;
 	private String algorithmId;
 
@@ -66,21 +63,11 @@ final public class ChecksumVerifier extends ProcessingStep {
 	}
 
 	@Override
-	final public void write(int b) throws IOException {
-		messageDigest.update((byte) b);
-		getDestination().write(b);
-	}
-
-	@Override
-	final public void close() throws IOException {
-		byte[] hashBytes = messageDigest.digest();
-		String hash = ChecksumHelper.toHexString(hashBytes);
-
+	final protected void onClose(String digestString) {
 		// if the hashes don't line up set the status to error.
-		if (!hash.equals(expectedChecksum))
+		if (!digestString.equals(expectedChecksum))
 			// TODO like ProvisionException.ARTIFACT_MD5_NOT_MATCH but for any checksum
-			setStatus(new Status(IStatus.ERROR, Activator.ID, ProvisionException.ARTIFACT_MD5_NOT_MATCH, NLS.bind(Messages.Error_unexpected_checksum, new Object[] {algorithmName, expectedChecksum, hash}), null));
-		super.close();
+			setStatus(new Status(IStatus.ERROR, Activator.ID, ProvisionException.ARTIFACT_MD5_NOT_MATCH, NLS.bind(Messages.Error_unexpected_checksum, new Object[] {algorithmName, expectedChecksum, digestString}), null));
 	}
 
 }
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/MessageDigestProcessingStep.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/MessageDigestProcessingStep.java
new file mode 100644
index 000000000..3fef57d03
--- /dev/null
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/MessageDigestProcessingStep.java
@@ -0,0 +1,69 @@
+/*******************************************************************************
+ * Copyright (c) 2018, 2018 Mykola Nikishov.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    Mykola Nikishov - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.equinox.internal.p2.artifact.processors.checksum;
+
+import java.io.IOException;
+import java.nio.BufferOverflowException;
+import java.nio.ByteBuffer;
+import java.security.MessageDigest;
+import org.eclipse.equinox.internal.p2.repository.helpers.ChecksumHelper;
+import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStep;
+
+/**
+ * @noreference This class is not intended to be referenced by clients.
+ */
+public abstract class MessageDigestProcessingStep extends ProcessingStep {
+
+	protected MessageDigest messageDigest;
+	private static final int BUFFER_SIZE = 16 * 1024;
+	private ByteBuffer buffer = ByteBuffer.allocate(BUFFER_SIZE);
+
+	@Override
+	// TODO template method, should be final but MD5Verifier prevents this
+	public void write(int b) throws IOException {
+		getDestination().write(b);
+
+		// TODO exceptions are expensive
+		try {
+			buffer.put((byte) b);
+		} catch (BufferOverflowException e) {
+			processBufferredBytes();
+			buffer.put((byte) b);
+		}
+	}
+
+	private void processBufferredBytes() {
+		buffer.flip();
+		updateDigest();
+		buffer.clear();
+	}
+
+	private void updateDigest() {
+		messageDigest.update(buffer);
+	}
+
+	@Override
+	// TODO should be final but MD5Verifier prevents this
+	public void close() throws IOException {
+		processBufferredBytes();
+		String digestString = digest();
+		onClose(digestString);
+		super.close();
+	}
+
+	private String digest() {
+		byte[] digestBytes = messageDigest.digest();
+		return ChecksumHelper.toHexString(digestBytes);
+	}
+
+	protected abstract void onClose(String digestString);
+
+}
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/MD5Verifier.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/MD5Verifier.java
index 50d2ff0d3..3d867b135 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/MD5Verifier.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/MD5Verifier.java
@@ -8,17 +8,16 @@
  * Contributors:
  * 	compeople AG (Stefan Liebig) - initial API and implementation
  * 	IBM Corporation - ongoing development
+ * 	Mykola Nikishov - ongoing development
  *******************************************************************************/
 package org.eclipse.equinox.internal.p2.artifact.processors.md5;
 
-import java.io.IOException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
+import org.eclipse.equinox.internal.p2.artifact.processors.checksum.MessageDigestProcessingStep;
 import org.eclipse.equinox.internal.p2.artifact.repository.Activator;
-import org.eclipse.equinox.internal.p2.repository.helpers.ChecksumHelper;
-import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStep;
 import org.eclipse.equinox.p2.core.IProvisioningAgent;
 import org.eclipse.equinox.p2.core.ProvisionException;
 import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor;
@@ -26,10 +25,9 @@ import org.eclipse.equinox.p2.repository.artifact.IProcessingStepDescriptor;
 import org.eclipse.osgi.util.NLS;
 
 @Deprecated
-public class MD5Verifier extends ProcessingStep {
+public class MD5Verifier extends MessageDigestProcessingStep {
 
 	protected String expectedMD5;
-	private MessageDigest md5;
 
 	public MD5Verifier() {
 		super();
@@ -60,26 +58,16 @@ public class MD5Verifier extends ProcessingStep {
 		if (expectedMD5 == null || expectedMD5.length() != 32)
 			setStatus(new Status(code, Activator.ID, NLS.bind(Messages.Error_invalid_hash, expectedMD5)));
 		try {
-			md5 = MessageDigest.getInstance("MD5"); //$NON-NLS-1$
+			messageDigest = MessageDigest.getInstance("MD5"); //$NON-NLS-1$
 		} catch (NoSuchAlgorithmException e) {
 			setStatus(new Status(code, Activator.ID, Messages.Error_MD5_unavailable, e));
 		}
 	}
 
 	@Override
-	public void write(int b) throws IOException {
-		md5.update((byte) b);
-		getDestination().write(b);
-	}
-
-	@Override
-	public void close() throws IOException {
-		byte[] digest = md5.digest();
-		String buf = ChecksumHelper.toHexString(digest);
-
+	protected void onClose(String digestString) {
 		// if the hashes don't line up set the status to error.
-		if (!buf.equals(expectedMD5))
-			setStatus(new Status(IStatus.ERROR, Activator.ID, ProvisionException.ARTIFACT_MD5_NOT_MATCH, NLS.bind(Messages.Error_unexpected_hash, expectedMD5, buf), null));
-		super.close();
+		if (!digestString.equals(expectedMD5))
+			setStatus(new Status(IStatus.ERROR, Activator.ID, ProvisionException.ARTIFACT_MD5_NOT_MATCH, NLS.bind(Messages.Error_unexpected_hash, expectedMD5, digestString), null));
 	}
 }