Read the compared contents once, in the background

team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java

@@ -893,7 +893,11 @@ private IDocument createDocument() {
 
 				try {
 					String encoding = internalGetEncoding();
-					s = Utilities.readString(sca, encoding);
+					// The background job may have read the contents already; decoding is
+					// all that is left then.
+					byte[] prefetched = CompareUIPlugin.takePrefetchedContents(fElement);
+					s = prefetched != null ? Utilities.readString(prefetched, encoding)
+							: Utilities.readString(sca, encoding);
 				} catch (CoreException ex) {
 					this.fViewer.setError(fLeg, ex.getMessage());
 				}

team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java

@@ -17,6 +17,8 @@
 package org.eclipse.compare.internal;
 
 import java.io.BufferedInputStream;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.ref.Reference;
@@ -285,6 +287,30 @@ private static final class SniffedElement {
 		}
 	}
 
+	/**
+	 * Contents read ahead while preparing an input. They are written in the
+	 * background job and read on the UI thread. An entry is dropped as soon as it
+	 * became a document, and the element is held weakly so that entries nobody asked
+	 * for go away with the editor that prepared them.
+	 */
+	private static final List<PrefetchedElement> fPrefetchedContents= new ArrayList<>(3);
+
+	/** Contents larger than this are read by whoever needs them instead. */
+	private static final int PREFETCH_LIMIT= 8 * 1024 * 1024;
+
+	/** Enough for the content type describers and the text heuristic. */
+	private static final int PREFETCH_PROBE_SIZE= 8 * 1024;
+
+	private static final class PrefetchedElement {
+		final Reference<ITypedElement> element;
+		final byte[] contents;
+
+		PrefetchedElement(ITypedElement element, byte[] contents) {
+			this.element= new WeakReference<>(element);
+			this.contents= contents;
+		}
+	}
+
 	public static final int NO_DIFFERENCE = 10000;
 
 	/**
@@ -904,6 +930,9 @@ public IStatus prepareInput(CompareEditorInput input, IProgressMonitor monitor)
 			if (input.getCompareResult() == null) {
 				return new Status(IStatus.ERROR, CompareUIPlugin.PLUGIN_ID, NO_DIFFERENCE, Utilities.getString("CompareUIPlugin.noDifferences"), null); //$NON-NLS-1$
 			}
+			// Still off the UI thread here, so this is the place to read the contents
+			// that the viewer would otherwise read while the user waits.
+			prefetchContents(input.getCompareResult(), monitor);
 			return Status.OK_STATUS;
 		} catch (InterruptedException e) {
 			throw new OperationCanceledException();
@@ -1569,6 +1598,134 @@ private static IContentType getContentType(ITypedElement element) {
 		return sniffed.contentType;
 	}
 
+	/**
+	 * Reads the contents of the given input's elements once, so that the sniffing
+	 * and the merge viewer do not have to open the streams again on the UI thread.
+	 * Elements that come with a shared document are left alone: those are served
+	 * from the file buffer and never read a stream.
+	 */
+	public static void prefetchContents(Object compareResult, IProgressMonitor monitor) {
+		if (!(compareResult instanceof ICompareInput input)) {
+			return;
+		}
+		prefetchContents(input.getAncestor(), monitor);
+		prefetchContents(input.getLeft(), monitor);
+		prefetchContents(input.getRight(), monitor);
+	}
+
+	private static void prefetchContents(ITypedElement element, IProgressMonitor monitor) {
+		if (!(element instanceof IStreamContentAccessor accessor) || hasSharedDocument(element)) {
+			return;
+		}
+		try (InputStream stream= accessor.getContents()) {
+			if (stream == null) {
+				return;
+			}
+			// Binary contents are neither shown by the text merge viewer nor read in
+			// full by the binary one, so nothing would use them.
+			byte[] probe= read(stream, PREFETCH_PROBE_SIZE, monitor);
+			if (probe == null || !looksLikeText(probe)) {
+				return;
+			}
+			byte[] rest= read(stream, PREFETCH_LIMIT - probe.length + 1, monitor);
+			if (rest == null || probe.length + rest.length > PREFETCH_LIMIT) {
+				return; // too large to be worth holding on to
+			}
+			byte[] contents= new byte[probe.length + rest.length];
+			System.arraycopy(probe, 0, contents, 0, probe.length);
+			System.arraycopy(rest, 0, contents, probe.length, rest.length);
+			putPrefetchedContents(element, contents);
+		} catch (CoreException | IOException e) {
+			// not fatal: whoever needs the contents reads the stream itself
+		}
+	}
+
+	/** Reads up to the given number of bytes, or <code>null</code> when cancelled. */
+	private static byte[] read(InputStream stream, int limit, IProgressMonitor monitor) throws IOException {
+		ByteArrayOutputStream buffer= new ByteArrayOutputStream(Math.min(limit, 8192));
+		byte[] chunk= new byte[8192];
+		while (buffer.size() < limit) {
+			if (monitor != null && monitor.isCanceled()) {
+				return null;
+			}
+			int read= stream.read(chunk, 0, Math.min(chunk.length, limit - buffer.size()));
+			if (read == -1) {
+				break;
+			}
+			buffer.write(chunk, 0, read);
+		}
+		return buffer.toByteArray();
+	}
+
+	/** The heuristic of {@link #computeGuessType(ITypedElement)}, on bytes at hand. */
+	private static boolean looksLikeText(byte[] contents) {
+		int lineLength= 0;
+		int lines= 0;
+		for (int i= 0; i < contents.length && lines < 10; i++) {
+			byte c= contents[i];
+			if (c == '\n' || c == '\r') {
+				lineLength= 0;
+				lines++;
+			} else if (++lineLength > 1000) {
+				return false;
+			}
+		}
+		return true;
+	}
+
+	private static boolean hasSharedDocument(ITypedElement element) {
+		ISharedDocumentAdapter adapter= SharedDocumentAdapterWrapper.getAdapter(element);
+		return adapter != null && adapter.getDocumentKey(element) != null;
+	}
+
+	private static void putPrefetchedContents(ITypedElement element, byte[] contents) {
+		synchronized (fPrefetchedContents) {
+			fPrefetchedContents.removeIf(prefetched -> {
+				ITypedElement held= prefetched.element.get();
+				return held == null || held == element;
+			});
+			fPrefetchedContents.add(new PrefetchedElement(element, contents));
+		}
+	}
+
+	/**
+	 * Returns the prefetched contents of the given element and forgets them, or
+	 * <code>null</code> if nothing was read ahead for it.
+	 */
+	public static byte[] takePrefetchedContents(Object element) {
+		synchronized (fPrefetchedContents) {
+			for (Iterator<PrefetchedElement> it= fPrefetchedContents.iterator(); it.hasNext();) {
+				PrefetchedElement prefetched= it.next();
+				ITypedElement held= prefetched.element.get();
+				if (held == null) {
+					it.remove();
+				} else if (held == element) {
+					it.remove();
+					return prefetched.contents;
+				}
+			}
+		}
+		return null;
+	}
+
+	private static byte[] prefetchedContents(ITypedElement element) {
+		synchronized (fPrefetchedContents) {
+			for (PrefetchedElement prefetched : fPrefetchedContents) {
+				if (prefetched.element.get() == element) {
+					return prefetched.contents;
+				}
+			}
+		}
+		return null;
+	}
+
+	/** The contents of the given element, read ahead if that already happened. */
+	private static InputStream contentsOf(ITypedElement element, IStreamContentAccessor accessor)
+			throws CoreException {
+		byte[] prefetched= prefetchedContents(element);
+		return prefetched != null ? new ByteArrayInputStream(prefetched) : accessor.getContents();
+	}
+
 	private static IContentType computeContentType(ITypedElement element) {
 		String name= element.getName();
 		IContentType ct= null;
@@ -1585,7 +1742,7 @@ private static IContentType computeContentType(ITypedElement element) {
 		}
 		if (element instanceof IStreamContentAccessor isa) {
 			try {
-				InputStream is= isa.getContents();
+				InputStream is= contentsOf(element, isa);
 				if (is != null) {
 					try (InputStream bis = new BufferedInputStream(is)) {
 						ct= fgContentTypeManager.findContentTypeFor(is, name);
@@ -1718,7 +1875,7 @@ private static String computeGuessType(ITypedElement input) {
 		if (input instanceof IStreamContentAccessor sca) {
 			InputStream is= null;
 			try {
-				is= sca.getContents();
+				is= contentsOf(input, sca);
 				if (is == null) {
 					return null;
 				}

team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/Utilities.java

@@ -15,6 +15,7 @@
 package org.eclipse.compare.internal;
 
 import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -582,14 +583,28 @@ private static IStatus addStatus(IStatus status, IStatus entry) {
 
 	// encoding
 
+	/** Opens the contents to decode, once per decoding attempt. */
+	private interface ContentsSupplier {
+		InputStream get() throws CoreException;
+	}
+
+	/** Decodes contents that have already been read. */
+	public static String readString(byte[] contents, String encoding) throws CoreException {
+		return decode(() -> new ByteArrayInputStream(contents), encoding);
+	}
+
 	public static String readString(IStreamContentAccessor sca, String encoding) throws CoreException {
+		return decode(sca::getContents, encoding);
+	}
+
+	private static String decode(ContentsSupplier contents, String encoding) throws CoreException {
 		String s = null;
 		try {
 			try {
-				s= Utilities.readString(sca.getContents(), encoding);
+				s= Utilities.readString(contents.get(), encoding);
 			} catch (UnsupportedEncodingException e) {
 				if (!encoding.equals(ResourcesPlugin.getEncoding())) {
-					s = Utilities.readString(sca.getContents(), ResourcesPlugin.getEncoding());
+					s = Utilities.readString(contents.get(), ResourcesPlugin.getEncoding());
 				}
 			}
 		} catch (IOException e) {

team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/CompareOpenEfficiencyTest.java

@@ -19,9 +19,13 @@
 import static org.junit.jupiter.api.Assertions.fail;
 
 import java.io.ByteArrayInputStream;
+import java.io.FilterInputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.BooleanSupplier;
 
 import org.eclipse.compare.CompareConfiguration;
@@ -55,31 +59,68 @@ public class CompareOpenEfficiencyTest {
 
 	/**
 	 * Upper bound for the {@code getContents()} calls per side on one compare editor
-	 * open: content-type detection, text heuristic, and the document itself.
+	 * open: the background job reads the contents once and everything else is served
+	 * from it.
 	 */
-	private static final int MAX_GET_CONTENTS_PER_SIDE = 3;
+	private static final int MAX_GET_CONTENTS_PER_SIDE = 1;
+
+	private static final int BINARY_SIZE = 4 * 1024 * 1024;
+
+	/**
+	 * Only the probe that tells binary from text may be read per side, plus a little
+	 * slack for the content type describers.
+	 */
+	private static final long MAX_BINARY_BYTES_PER_SIDE = 64 * 1024;
 
 	private static final long TIMEOUT_MILLIS = 30_000;
 
 	private boolean originalUnifiedDiff;
 
-	/** A text element that counts every {@link #getContents()} call. */
+	/**
+	 * An element that counts every {@link #getContents()} call and every byte
+	 * consumed from the streams it hands out.
+	 */
 	private static final class CountingElement
 			implements ITypedElement, IEncodedStreamContentAccessor {
 
 		private final String name;
+		private final String type;
 		private final byte[] bytes;
 		private final AtomicInteger contentReads = new AtomicInteger();
+		private final AtomicLong bytesRead = new AtomicLong();
 
 		CountingElement(String name, String content) {
+			this(name, TEXT_TYPE, content.getBytes(StandardCharsets.UTF_8));
+		}
+
+		CountingElement(String name, String type, byte[] content) {
 			this.name = name;
-			this.bytes = content.getBytes(StandardCharsets.UTF_8);
+			this.type = type;
+			this.bytes = content;
 		}
 
 		@Override
 		public InputStream getContents() {
 			contentReads.incrementAndGet();
-			return new ByteArrayInputStream(bytes);
+			return new FilterInputStream(new ByteArrayInputStream(bytes)) {
+				@Override
+				public int read() throws IOException {
+					int b = super.read();
+					if (b != -1) {
+						bytesRead.incrementAndGet();
+					}
+					return b;
+				}
+
+				@Override
+				public int read(byte[] b, int off, int len) throws IOException {
+					int n = super.read(b, off, len);
+					if (n > 0) {
+						bytesRead.addAndGet(n);
+					}
+					return n;
+				}
+			};
 		}
 
 		@Override
@@ -94,14 +135,18 @@ public String getName() {
 
 		@Override
 		public String getType() {
-			return TEXT_TYPE;
+			return type;
 		}
 
 		@Override
 		public Image getImage() {
 			return null;
 		}
 
+		long bytes() {
+			return bytesRead.get();
+		}
+
 		int reads() {
 			return contentReads.get();
 		}
@@ -188,6 +233,36 @@ public void testGetContentsCallCountPerSide() throws Exception {
 		assertReadsWithinBound("right", rightElement.reads()); //$NON-NLS-1$
 	}
 
+	/**
+	 * Binary contents must not be read ahead: the binary viewer stops at the first
+	 * difference, so buffering the whole thing would be both slower and a lot of
+	 * memory held for nothing.
+	 */
+	@Test
+	public void testBinaryContentsAreNotReadAhead() throws Exception {
+		store().setValue(ComparePreferencePage.UNIFIED_DIFF, false);
+		byte[] leftBytes = new byte[BINARY_SIZE];
+		Arrays.fill(leftBytes, (byte) 1);
+		byte[] rightBytes = leftBytes.clone();
+		rightBytes[0] = 2; // differs immediately, so the viewer reads one byte
+		CountingElement leftElement = new CountingElement("left.bin", ITypedElement.UNKNOWN_TYPE, leftBytes); //$NON-NLS-1$
+		CountingElement rightElement = new CountingElement("right.bin", ITypedElement.UNKNOWN_TYPE, rightBytes); //$NON-NLS-1$
+		CountingCompareEditorInput input = new CountingCompareEditorInput(true, leftElement, rightElement);
+
+		CompareUI.openCompareEditor(input);
+		pumpUntil(() -> input.getCompareResult() != null && contentPane(input) != null,
+				"compare editor did not finish opening"); //$NON-NLS-1$
+
+		assertBytesWithinBound("left", leftElement.bytes()); //$NON-NLS-1$
+		assertBytesWithinBound("right", rightElement.bytes()); //$NON-NLS-1$
+	}
+
+	private static void assertBytesWithinBound(String side, long bytes) {
+		assertTrue(bytes <= MAX_BINARY_BYTES_PER_SIDE,
+				"read " + bytes + " bytes from the " + side + " side of a " + BINARY_SIZE //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+						+ " byte binary, exceeding the bound of " + MAX_BINARY_BYTES_PER_SIDE); //$NON-NLS-1$
+	}
+
 	private static void assertReadsWithinBound(String side, int reads) {
 		assertTrue(reads >= 1, "expected at least one getContents() call on the " + side + " side"); //$NON-NLS-1$ //$NON-NLS-2$
 		assertTrue(reads <= MAX_GET_CONTENTS_PER_SIDE,