Merge pull request #125 from cryptomator/feature/bytebuffer-pooling

Use `BufferPool` for cleartext as well as ciphertext chunks

src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java

@@ -3,9 +3,10 @@
 import com.google.common.base.Preconditions;
 import org.cryptomator.cryptofs.CryptoFileSystemStats;
 import org.cryptomator.cryptofs.EffectiveOpenOptions;
+import org.cryptomator.cryptofs.fh.BufferPool;
 import org.cryptomator.cryptofs.fh.ByteSource;
 import org.cryptomator.cryptofs.fh.ChunkCache;
-import org.cryptomator.cryptofs.fh.ChunkData;
+import org.cryptomator.cryptofs.fh.Chunk;
 import org.cryptomator.cryptofs.fh.ExceptionsDuringWrite;
 import org.cryptomator.cryptofs.fh.OpenFileModifiedDate;
 import org.cryptomator.cryptofs.fh.OpenFileSize;
@@ -42,6 +43,7 @@ public class CleartextFileChannel extends AbstractFileChannel {
 	private final FileHeader fileHeader;
 	private final Cryptor cryptor;
 	private final ChunkCache chunkCache;
+	private final BufferPool bufferPool;
 	private final EffectiveOpenOptions options;
 	private final AtomicLong fileSize;
 	private final AtomicReference<Instant> lastModified;
@@ -52,12 +54,13 @@ public class CleartextFileChannel extends AbstractFileChannel {
 	private boolean mustWriteHeader;
 
 	@Inject
-	public CleartextFileChannel(FileChannel ciphertextFileChannel, FileHeader fileHeader, @MustWriteHeader boolean mustWriteHeader, ReadWriteLock readWriteLock, Cryptor cryptor, ChunkCache chunkCache, EffectiveOpenOptions options, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference<Instant> lastModified, Supplier<BasicFileAttributeView> attrViewProvider, ExceptionsDuringWrite exceptionsDuringWrite, ChannelCloseListener closeListener, CryptoFileSystemStats stats) {
+	public CleartextFileChannel(FileChannel ciphertextFileChannel, FileHeader fileHeader, @MustWriteHeader boolean mustWriteHeader, ReadWriteLock readWriteLock, Cryptor cryptor, ChunkCache chunkCache, BufferPool bufferPool, EffectiveOpenOptions options, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference<Instant> lastModified, Supplier<BasicFileAttributeView> attrViewProvider, ExceptionsDuringWrite exceptionsDuringWrite, ChannelCloseListener closeListener, CryptoFileSystemStats stats) {
 		super(readWriteLock);
 		this.ciphertextFileChannel = ciphertextFileChannel;
 		this.fileHeader = fileHeader;
 		this.cryptor = cryptor;
 		this.chunkCache = chunkCache;
+		this.bufferPool = bufferPool;
 		this.options = options;
 		this.fileSize = fileSize;
 		this.lastModified = lastModified;
@@ -104,9 +107,9 @@ protected int readLocked(ByteBuffer dst, long position) throws IOException {
 			long pos = position + read;
 			long chunkIndex = pos / payloadSize; // floor by int-truncation
 			int offsetInChunk = (int) (pos % payloadSize); // known to fit in int, because payloadSize is int
-			int len = min(dst.remaining(), payloadSize - offsetInChunk); // known to fit in int, because second argument is int
-			final ChunkData chunkData = chunkCache.get(chunkIndex);
-			chunkData.copyDataStartingAt(offsetInChunk).to(dst);
+			ByteBuffer data = chunkCache.get(chunkIndex).data().duplicate().position(offsetInChunk);
+			int len = min(dst.remaining(), data.remaining()); // known to fit in int, because second argument is int
+			dst.put(data.limit(data.position() + len));
 			read += len;
 		}
 		dst.limit(origLimit);
@@ -148,17 +151,21 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti
 			assert len <= cleartextChunkSize;
 			if (offsetInChunk == 0 && len == cleartextChunkSize) {
 				// complete chunk, no need to load and decrypt from file
-				ChunkData chunkData = ChunkData.emptyWithSize(cleartextChunkSize);
-				chunkData.copyData().from(src);
-				chunkCache.set(chunkIndex, chunkData);
+				ByteBuffer cleartextChunkData = bufferPool.getCleartextBuffer();
+				src.copyTo(cleartextChunkData);
+				cleartextChunkData.flip();
+				Chunk chunk = new Chunk(cleartextChunkData, true);
+				chunkCache.set(chunkIndex, chunk);
 			} else {
 				/*
 				 * TODO performance:
 				 * We don't actually need to read the current data into the cache.
 				 * It would suffice if store the written data and do reading when storing the chunk.
 				 */
-				ChunkData chunkData = chunkCache.get(chunkIndex);
-				chunkData.copyDataStartingAt(offsetInChunk).from(src);
+				Chunk chunk = chunkCache.get(chunkIndex);
+				chunk.data().limit(Math.max(chunk.data().limit(), offsetInChunk + len)); // increase limit (if needed)
+				src.copyTo(chunk.data().duplicate().position(offsetInChunk)); // work on duplicate using correct offset
+				chunk.dirty().set(true);
 			}
 			written += len;
 		}
@@ -190,7 +197,9 @@ protected void truncateLocked(long newSize) throws IOException {
 			long indexOfLastChunk = (newSize + cleartextChunkSize - 1) / cleartextChunkSize - 1;
 			int sizeOfIncompleteChunk = (int) (newSize % cleartextChunkSize); // known to fit in int, because cleartextChunkSize is int
 			if (sizeOfIncompleteChunk > 0) {
-				chunkCache.get(indexOfLastChunk).truncate(sizeOfIncompleteChunk);
+				var chunk = chunkCache.get(indexOfLastChunk);
+				chunk.data().limit(sizeOfIncompleteChunk);
+				chunk.dirty().set(true);
 			}
 			long ciphertextFileSize = cryptor.fileHeaderCryptor().headerSize() + cryptor.fileContentCryptor().ciphertextSize(newSize);
 			chunkCache.invalidateAll(); // make sure no chunks _after_ newSize exist that would otherwise be written during the next cache eviction

src/main/java/org/cryptomator/cryptofs/fh/BufferPool.java

@@ -0,0 +1,57 @@
+package org.cryptomator.cryptofs.fh;
+
+import org.cryptomator.cryptofs.CryptoFileSystemScoped;
+import org.cryptomator.cryptolib.api.Cryptor;
+
+import javax.inject.Inject;
+import java.lang.ref.WeakReference;
+import java.nio.ByteBuffer;
+import java.util.Optional;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+/**
+ * A pool of ByteBuffers for cleartext and ciphertext chunks to avoid on-heap allocation.
+ */
+@CryptoFileSystemScoped
+public class BufferPool {
+
+	private final int ciphertextChunkSize;
+	private final int cleartextChunkSize;
+	private final Queue<WeakReference<ByteBuffer>> ciphertextBuffers = new ConcurrentLinkedQueue<>();
+	private final Queue<WeakReference<ByteBuffer>> cleartextBuffers = new ConcurrentLinkedQueue<>();
+
+	@Inject
+	public BufferPool(Cryptor cryptor) {
+		this.ciphertextChunkSize = cryptor.fileContentCryptor().ciphertextChunkSize();
+		this.cleartextChunkSize = cryptor.fileContentCryptor().cleartextChunkSize();
+	}
+
+	private Optional<ByteBuffer> dequeueFrom(Queue<WeakReference<ByteBuffer>> queue) {
+		WeakReference<ByteBuffer> ref;
+		while ((ref = queue.poll()) != null) {
+			ByteBuffer cached = ref.get();
+			if (cached != null) {
+				cached.clear();
+				return Optional.of(cached);
+			}
+		}
+		return Optional.empty();
+	}
+
+	public ByteBuffer getCiphertextBuffer() {
+		return dequeueFrom(ciphertextBuffers).orElseGet(() -> ByteBuffer.allocate(ciphertextChunkSize));
+	}
+
+	public ByteBuffer getCleartextBuffer() {
+		return dequeueFrom(cleartextBuffers).orElseGet(() -> ByteBuffer.allocate(cleartextChunkSize));
+	}
+
+	public void recycle(ByteBuffer buffer) {
+		if (buffer.capacity() == ciphertextChunkSize) {
+			ciphertextBuffers.add(new WeakReference<>(buffer));
+		} else if (buffer.capacity() == cleartextChunkSize) {
+			cleartextBuffers.add(new WeakReference<>(buffer));
+		}
+	}
+}

src/main/java/org/cryptomator/cryptofs/fh/Chunk.java

@@ -0,0 +1,36 @@
+/*******************************************************************************
+ * Copyright (c) 2016 Sebastian Stenzel and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the accompanying LICENSE.txt.
+ *
+ * Contributors:
+ *     Sebastian Stenzel - initial API and implementation
+ *******************************************************************************/
+package org.cryptomator.cryptofs.fh;
+
+import java.nio.ByteBuffer;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static java.lang.String.format;
+
+/**
+ * A chunk of plaintext data. It has these rules:
+ * <ol>
+ *     <li>Capacity of {@code data} is always the cleartext chunk size</li>
+ *     <li>During creation, {@code data}'s limit is the chunk's size (last chunk of a file may be smaller)</li>
+ *     <li>Writes need to adjust the limit and mark this chunk dirty</li>
+ *     <li>Reads need to respect the limit and must not change it</li>
+ *     <li>When no longer used, the cleartext ByteBuffer may be recycled</li>
+ * </ol>
+ */
+public record Chunk(ByteBuffer data, AtomicBoolean dirty) {
+
+	public Chunk(ByteBuffer data, boolean dirty) {
+		this(data, new AtomicBoolean(dirty));
+	}
+
+	public boolean isDirty() {
+		return dirty.get();
+	}
+
+}

src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java

@@ -19,20 +19,22 @@ public class ChunkCache {
 	private final ChunkLoader chunkLoader;
 	private final ChunkSaver chunkSaver;
 	private final CryptoFileSystemStats stats;
-	private final Cache<Long, ChunkData> chunks;
+	private final BufferPool bufferPool;
+	private final Cache<Long, Chunk> chunks;
 
 	@Inject
-	public ChunkCache(ChunkLoader chunkLoader, ChunkSaver chunkSaver, CryptoFileSystemStats stats) {
+	public ChunkCache(ChunkLoader chunkLoader, ChunkSaver chunkSaver, CryptoFileSystemStats stats, BufferPool bufferPool) {
 		this.chunkLoader = chunkLoader;
 		this.chunkSaver = chunkSaver;
 		this.stats = stats;
+		this.bufferPool = bufferPool;
 		this.chunks = CacheBuilder.newBuilder() //
 				.maximumSize(MAX_CACHED_CLEARTEXT_CHUNKS) //
 				.removalListener(this::removeChunk) //
 				.build();
 	}
 
-	private ChunkData loadChunk(long chunkIndex) throws IOException {
+	private Chunk loadChunk(long chunkIndex) throws IOException {
 		stats.addChunkCacheMiss();
 		try {
 			return chunkLoader.load(chunkIndex);
@@ -42,15 +44,16 @@ private ChunkData loadChunk(long chunkIndex) throws IOException {
 		}
 	}
 
-	private void removeChunk(RemovalNotification<Long, ChunkData> removal) {
+	private void removeChunk(RemovalNotification<Long, Chunk> removal) {
 		try {
 			chunkSaver.save(removal.getKey(), removal.getValue());
+			bufferPool.recycle(removal.getValue().data());
 		} catch (IOException e) {
 			throw new UncheckedIOException(e);
 		}
 	}
 
-	public ChunkData get(long chunkIndex) throws IOException {
+	public Chunk get(long chunkIndex) throws IOException {
 		try {
 			stats.addChunkCacheAccess();
 			return chunks.get(chunkIndex, () -> loadChunk(chunkIndex));
@@ -60,7 +63,7 @@ public ChunkData get(long chunkIndex) throws IOException {
 		}
 	}
 
-	public void set(long chunkIndex, ChunkData data) {
+	public void set(long chunkIndex, Chunk data) {
 		chunks.put(chunkIndex, data);
 	}
 

src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java

@@ -15,38 +15,36 @@ class ChunkLoader {
 	private final ChunkIO ciphertext;
 	private final FileHeaderHolder headerHolder;
 	private final CryptoFileSystemStats stats;
+	private final BufferPool bufferPool;
 
 	@Inject
-	public ChunkLoader(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, CryptoFileSystemStats stats) {
+	public ChunkLoader(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, CryptoFileSystemStats stats, BufferPool bufferPool) {
 		this.cryptor = cryptor;
 		this.ciphertext = ciphertext;
 		this.headerHolder = headerHolder;
 		this.stats = stats;
+		this.bufferPool = bufferPool;
 	}
 
-	public ChunkData load(Long chunkIndex) throws IOException, AuthenticationFailedException {
+	public Chunk load(Long chunkIndex) throws IOException, AuthenticationFailedException {
 		stats.addChunkCacheMiss();
-		int payloadSize = cryptor.fileContentCryptor().cleartextChunkSize();
 		int chunkSize = cryptor.fileContentCryptor().ciphertextChunkSize();
 		long ciphertextPos = chunkIndex * chunkSize + cryptor.fileHeaderCryptor().headerSize();
-		ByteBuffer ciphertextBuf = ByteBuffer.allocate(chunkSize);
-		int read = ciphertext.read(ciphertextBuf, ciphertextPos);
-		if (read == -1) {
-			// append
-			return ChunkData.emptyWithSize(payloadSize);
-		} else {
-			ciphertextBuf.flip();
-			ByteBuffer cleartextBuf = cryptor.fileContentCryptor().decryptChunk(ciphertextBuf, chunkIndex, headerHolder.get(), true);
-			stats.addBytesDecrypted(cleartextBuf.remaining());
-			ByteBuffer cleartextBufWhichCanHoldFullChunk;
-			if (cleartextBuf.capacity() < payloadSize) {
-				cleartextBufWhichCanHoldFullChunk = ByteBuffer.allocate(payloadSize);
-				cleartextBufWhichCanHoldFullChunk.put(cleartextBuf);
-				cleartextBufWhichCanHoldFullChunk.flip();
+		ByteBuffer ciphertextBuf = bufferPool.getCiphertextBuffer();
+		ByteBuffer cleartextBuf = bufferPool.getCleartextBuffer();
+		try {
+			int read = ciphertext.read(ciphertextBuf, ciphertextPos);
+			if (read == -1) {
+				cleartextBuf.limit(0);
 			} else {
-				cleartextBufWhichCanHoldFullChunk = cleartextBuf;
+				ciphertextBuf.flip();
+				cryptor.fileContentCryptor().decryptChunk(ciphertextBuf, cleartextBuf, chunkIndex, headerHolder.get(), true);
+				cleartextBuf.flip();
+				stats.addBytesDecrypted(cleartextBuf.remaining());
 			}
-			return ChunkData.wrap(cleartextBufWhichCanHoldFullChunk);
+			return new Chunk(cleartextBuf, false);
+		} finally {
+			bufferPool.recycle(ciphertextBuf);
 		}
 	}