Browse Source

improved directory name caching (>95% hitrate now)

Sebastian Stenzel 10 years ago
parent
commit
49646aae41

+ 3 - 31
main/core/src/main/java/org/cryptomator/webdav/jackrabbit/CryptoResourceFactory.java

@@ -1,16 +1,10 @@
 package org.cryptomator.webdav.jackrabbit;
 
 import java.io.IOException;
-import java.nio.ByteBuffer;
-import java.nio.channels.FileChannel;
-import java.nio.channels.FileLock;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.StandardOpenOption;
-import java.util.UUID;
 import java.util.concurrent.ExecutorService;
 
 import org.apache.commons.httpclient.HttpStatus;
@@ -120,7 +114,7 @@ public class CryptoResourceFactory implements DavResourceFactory, FileConstants
 		final Path parent = createEncryptedDirectoryPath(parentCleartextPath);
 		final String cleartextFilename = FilenameUtils.getName(relativeCleartextPath);
 		try {
-			final String encryptedFilename = filenameTranslator.getEncryptedDirName(cleartextFilename);
+			final String encryptedFilename = filenameTranslator.getEncryptedDirFileName(cleartextFilename);
 			return parent.resolve(encryptedFilename);
 		} catch (IOException e) {
 			throw new DavException(DavServletResponse.SC_INTERNAL_SERVER_ERROR, e);
@@ -143,15 +137,9 @@ public class CryptoResourceFactory implements DavResourceFactory, FileConstants
 				final String parentCleartextPath = FilenameUtils.getPathNoEndSeparator(relativeCleartextPath);
 				final Path parent = createEncryptedDirectoryPath(parentCleartextPath);
 				final String cleartextFilename = FilenameUtils.getName(relativeCleartextPath);
-				final String encryptedFilename = filenameTranslator.getEncryptedDirName(cleartextFilename);
+				final String encryptedFilename = filenameTranslator.getEncryptedDirFileName(cleartextFilename);
 				final Path directoryFile = parent.resolve(encryptedFilename);
-				final String directoryId;
-				if (Files.exists(directoryFile)) {
-					directoryId = new String(readAllBytesAtomically(directoryFile), StandardCharsets.UTF_8);
-				} else {
-					directoryId = UUID.randomUUID().toString();
-					writeAllBytesAtomically(directoryFile, directoryId.getBytes(StandardCharsets.UTF_8));
-				}
+				final String directoryId = filenameTranslator.getDirectoryId(directoryFile, true);
 				final String directory = cryptor.encryptDirectoryPath(directoryId, FileSystems.getDefault().getSeparator());
 				result = dataRoot.resolve(directory);
 			}
@@ -194,20 +182,4 @@ public class CryptoResourceFactory implements DavResourceFactory, FileConstants
 		return new NonExistingNode(this, locator, session, lockManager, cryptor, filePath, dirFilePath);
 	}
 
-	/* IO support */
-
-	private void writeAllBytesAtomically(Path path, byte[] bytes) throws IOException {
-		try (final FileChannel c = FileChannel.open(path, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DSYNC); final FileLock lock = c.lock()) {
-			c.write(ByteBuffer.wrap(bytes));
-		}
-	}
-
-	private byte[] readAllBytesAtomically(Path path) throws IOException {
-		try (final FileChannel c = FileChannel.open(path, StandardOpenOption.READ, StandardOpenOption.DSYNC); final FileLock lock = c.lock(0L, Long.MAX_VALUE, true)) {
-			final ByteBuffer buffer = ByteBuffer.allocate((int) c.size());
-			c.read(buffer);
-			return buffer.array();
-		}
-	}
-
 }

+ 7 - 32
main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedDir.java

@@ -70,12 +70,8 @@ class EncryptedDir extends AbstractEncryptedNode implements FileConstants {
 	 */
 	protected synchronized String getDirectoryId() {
 		if (directoryId == null) {
-			try (final FileChannel c = FileChannel.open(filePath, StandardOpenOption.READ, StandardOpenOption.DSYNC); final FileLock lock = c.lock(0L, Long.MAX_VALUE, true)) {
-				final ByteBuffer buffer = ByteBuffer.allocate((int) c.size());
-				c.read(buffer);
-				directoryId = new String(buffer.array(), StandardCharsets.UTF_8);
-			} catch (FileNotFoundException e) {
-				directoryId = null;
+			try {
+				directoryId = filenameTranslator.getDirectoryId(filePath, false);
 			} catch (IOException e) {
 				throw new IORuntimeException(e);
 			}
@@ -139,21 +135,9 @@ class EncryptedDir extends AbstractEncryptedNode implements FileConstants {
 		}
 		try {
 			final String cleartextDirName = FilenameUtils.getName(childLocator.getResourcePath());
-			final String ciphertextDirName = filenameTranslator.getEncryptedDirName(cleartextDirName);
+			final String ciphertextDirName = filenameTranslator.getEncryptedDirFileName(cleartextDirName);
 			final Path dirFilePath = dirPath.resolve(ciphertextDirName);
-			final String directoryId;
-			if (Files.exists(dirFilePath)) {
-				try (final FileChannel c = FileChannel.open(dirFilePath, StandardOpenOption.READ, StandardOpenOption.DSYNC); final FileLock lock = c.lock(0L, Long.MAX_VALUE, true)) {
-					final ByteBuffer buffer = ByteBuffer.allocate((int) c.size());
-					c.read(buffer);
-					directoryId = new String(buffer.array(), StandardCharsets.UTF_8);
-				}
-			} else {
-				directoryId = UUID.randomUUID().toString();
-				try (final FileChannel c = FileChannel.open(dirFilePath, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW, StandardOpenOption.DSYNC); final FileLock lock = c.lock()) {
-					c.write(ByteBuffer.wrap(directoryId.getBytes(StandardCharsets.UTF_8)));
-				}
-			}
+			final String directoryId = filenameTranslator.getDirectoryId(dirFilePath, true);
 			final Path directoryPath = filenameTranslator.getEncryptedDirectoryPath(directoryId);
 			Files.createDirectories(directoryPath);
 		} catch (SecurityException e) {
@@ -257,7 +241,7 @@ class EncryptedDir extends AbstractEncryptedNode implements FileConstants {
 				if (subDirPath != null) {
 					Files.deleteIfExists(subDirPath);
 				}
-				ciphertextFilename = filenameTranslator.getEncryptedDirName(cleartextFilename);
+				ciphertextFilename = filenameTranslator.getEncryptedDirFileName(cleartextFilename);
 			} else {
 				ciphertextFilename = filenameTranslator.getEncryptedFilename(cleartextFilename);
 			}
@@ -333,17 +317,8 @@ class EncryptedDir extends AbstractEncryptedNode implements FileConstants {
 					Files.copy(srcChildPath, dstChildPath, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
 				}
 			} else if (StringUtils.endsWithIgnoreCase(childName, DIR_EXT)) {
-				final String srcSubdirId;
-				try (final FileChannel c = FileChannel.open(srcChildPath, StandardOpenOption.READ, StandardOpenOption.DSYNC); final FileLock lock = c.lock(0L, Long.MAX_VALUE, true)) {
-					final ByteBuffer buffer = ByteBuffer.allocate((int) c.size());
-					c.read(buffer);
-					srcSubdirId = new String(buffer.array(), StandardCharsets.UTF_8);
-				}
-				final String dstSubdirId = UUID.randomUUID().toString();
-				try (final FileChannel c = FileChannel.open(dstChildPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DSYNC);
-						final FileLock lock = c.lock()) {
-					c.write(ByteBuffer.wrap(dstSubdirId.getBytes(StandardCharsets.UTF_8)));
-				}
+				final String srcSubdirId = filenameTranslator.getDirectoryId(srcChildPath, false);
+				final String dstSubdirId = filenameTranslator.getDirectoryId(dstChildPath, true);
 				copyDirectoryContents(srcSubdirId, dstSubdirId);
 			}
 		}

+ 1 - 1
main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFilePart.java

@@ -138,7 +138,7 @@ class EncryptedFilePart extends EncryptedFile {
 			}
 		} catch (EOFException e) {
 			if (LOG.isDebugEnabled()) {
-				LOG.debug("Unexpected end of stream during delivery of partial content (client hung up).");
+				LOG.trace("Unexpected end of stream during delivery of partial content (client hung up).");
 			}
 		} catch (DecryptFailedException e) {
 			throw new IOException("Error decrypting file " + filePath.toString(), e);

+ 52 - 10
main/core/src/main/java/org/cryptomator/webdav/jackrabbit/FilenameTranslator.java

@@ -1,16 +1,24 @@
 package org.cryptomator.webdav.jackrabbit;
 
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
+import java.nio.file.attribute.FileTime;
+import java.util.Map;
 import java.util.UUID;
 
+import org.apache.commons.collections4.map.LRUMap;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
 import org.cryptomator.crypto.Cryptor;
 import org.cryptomator.crypto.exceptions.DecryptFailedException;
 
@@ -18,10 +26,13 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 
 class FilenameTranslator implements FileConstants {
 
+	private static final int MAX_CACHED_DIRECTORY_IDS = 5000;
+
 	private final Cryptor cryptor;
 	private final Path dataRoot;
 	private final Path metadataRoot;
 	private final ObjectMapper objectMapper = new ObjectMapper();
+	private final Map<Pair<Path, FileTime>, String> directoryIdCache = new LRUMap<>(MAX_CACHED_DIRECTORY_IDS); // <directoryFile, directoryId>
 
 	public FilenameTranslator(Cryptor cryptor, Path vaultRoot) {
 		this.cryptor = cryptor;
@@ -31,6 +42,28 @@ class FilenameTranslator implements FileConstants {
 
 	/* file and directory name en/decryption */
 
+	public String getDirectoryId(Path directoryFile, boolean createIfNonexisting) throws IOException {
+		try {
+			final Pair<Path, FileTime> key = ImmutablePair.of(directoryFile, Files.getLastModifiedTime(directoryFile));
+			String directoryId = directoryIdCache.get(key);
+			if (directoryId == null) {
+				directoryId = new String(readAllBytesAtomically(directoryFile), StandardCharsets.UTF_8);
+				directoryIdCache.put(key, directoryId);
+			}
+			return directoryId;
+		} catch (FileNotFoundException | NoSuchFileException e) {
+			if (createIfNonexisting) {
+				final String directoryId = UUID.randomUUID().toString();
+				writeAllBytesAtomically(directoryFile, directoryId.getBytes(StandardCharsets.UTF_8));
+				final Pair<Path, FileTime> key = ImmutablePair.of(directoryFile, Files.getLastModifiedTime(directoryFile));
+				directoryIdCache.put(key, directoryId);
+				return directoryId;
+			} else {
+				return null;
+			}
+		}
+	}
+
 	public Path getEncryptedDirectoryPath(String directoryId) {
 		final String encrypted = cryptor.encryptDirectoryPath(directoryId, FileSystems.getDefault().getSeparator());
 		return dataRoot.resolve(encrypted);
@@ -40,7 +73,7 @@ class FilenameTranslator implements FileConstants {
 		return getEncryptedFilename(cleartextFilename, FILE_EXT, LONG_FILE_EXT);
 	}
 
-	public String getEncryptedDirName(String cleartextDirName) throws IOException {
+	public String getEncryptedDirFileName(String cleartextDirName) throws IOException {
 		return getEncryptedFilename(cleartextDirName, DIR_EXT, LONG_DIR_EXT);
 	}
 
@@ -94,10 +127,8 @@ class FilenameTranslator implements FileConstants {
 		final Path metadataDir = metadataRoot.resolve(metadataGroup.substring(0, 2));
 		Files.createDirectories(metadataDir);
 		final Path metadataFile = metadataDir.resolve(metadataGroup.substring(2));
-		try (final FileChannel c = FileChannel.open(metadataFile, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DSYNC); final FileLock lock = c.lock()) {
-			byte[] bytes = objectMapper.writeValueAsBytes(metadata);
-			c.write(ByteBuffer.wrap(bytes));
-		}
+		final byte[] metadataContent = objectMapper.writeValueAsBytes(metadata);
+		writeAllBytesAtomically(metadataFile, metadataContent);
 	}
 
 	private LongFilenameMetadata readMetadata(String metadataGroup) throws IOException {
@@ -106,11 +137,22 @@ class FilenameTranslator implements FileConstants {
 		if (!Files.isReadable(metadataFile)) {
 			return new LongFilenameMetadata();
 		} else {
-			try (final FileChannel c = FileChannel.open(metadataFile, StandardOpenOption.READ, StandardOpenOption.DSYNC); final FileLock lock = c.lock(0L, Long.MAX_VALUE, true)) {
-				final ByteBuffer buffer = ByteBuffer.allocate((int) c.size());
-				c.read(buffer);
-				return objectMapper.readValue(buffer.array(), LongFilenameMetadata.class);
-			}
+			final byte[] metadataContent = readAllBytesAtomically(metadataFile);
+			return objectMapper.readValue(metadataContent, LongFilenameMetadata.class);
+		}
+	}
+
+	private void writeAllBytesAtomically(Path path, byte[] bytes) throws IOException {
+		try (final FileChannel c = FileChannel.open(path, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DSYNC); final FileLock lock = c.lock()) {
+			c.write(ByteBuffer.wrap(bytes));
+		}
+	}
+
+	private byte[] readAllBytesAtomically(Path path) throws IOException {
+		try (final FileChannel c = FileChannel.open(path, StandardOpenOption.READ, StandardOpenOption.DSYNC); final FileLock lock = c.lock(0L, Long.MAX_VALUE, true)) {
+			final ByteBuffer buffer = ByteBuffer.allocate((int) c.size());
+			c.read(buffer);
+			return buffer.array();
 		}
 	}
 

+ 2 - 2
main/crypto-api/src/main/java/org/cryptomator/crypto/AbstractCryptorDecorator.java

@@ -33,8 +33,8 @@ public class AbstractCryptorDecorator implements Cryptor {
 	}
 
 	@Override
-	public String encryptDirectoryPath(String cleartextPath, String nativePathSep) {
-		return cryptor.encryptDirectoryPath(cleartextPath, nativePathSep);
+	public String encryptDirectoryPath(String cleartextDirectoryId, String nativePathSep) {
+		return cryptor.encryptDirectoryPath(cleartextDirectoryId, nativePathSep);
 	}
 
 	@Override

+ 1 - 1
main/crypto-api/src/main/java/org/cryptomator/crypto/Cryptor.java

@@ -45,7 +45,7 @@ public interface Cryptor extends Destroyable {
 	/**
 	 * Encrypts a given plaintext path representing a directory structure. See {@link #encryptFilename(String, CryptorMetadataSupport)} for contents inside directories.
 	 * 
-	 * @param cleartextDirectoryId A relative path (UTF-8 encoded), whose path components are separated by '/'
+	 * @param cleartextDirectoryId A unique directory id
 	 * @param nativePathSep Path separator like "/" used on local file system. Must not be null, even if cleartextPath is a sole file name without any path separators.
 	 * @return Encrypted path.
 	 */

+ 8 - 21
main/crypto-api/src/main/java/org/cryptomator/crypto/PathCachingCryptorDecorator.java

@@ -12,7 +12,7 @@ public class PathCachingCryptorDecorator extends AbstractCryptorDecorator {
 	private static final int MAX_CACHED_PATHS = 5000;
 	private static final int MAX_CACHED_NAMES = 5000;
 
-	private final Map<String, String> pathCache = new LRUMap<>(MAX_CACHED_PATHS); // <cleartextPath, ciphertextPath>
+	private final Map<String, String> pathCache = new LRUMap<>(MAX_CACHED_PATHS); // <cleartextDirectoryId, ciphertextPath>
 	private final BidiMap<String, String> nameCache = new BidiLRUMap<>(MAX_CACHED_NAMES); // <cleartextName, ciphertextName>
 
 	private PathCachingCryptorDecorator(Cryptor cryptor) {
@@ -26,36 +26,23 @@ public class PathCachingCryptorDecorator extends AbstractCryptorDecorator {
 	/* Cryptor */
 
 	@Override
-	public String encryptDirectoryPath(String cleartextPath, String nativePathSep) {
-		if (pathCache.containsKey(cleartextPath)) {
-			return pathCache.get(cleartextPath);
-		} else {
-			final String ciphertextPath = cryptor.encryptDirectoryPath(cleartextPath, nativePathSep);
-			pathCache.put(cleartextPath, ciphertextPath);
-			return ciphertextPath;
-		}
+	public String encryptDirectoryPath(String cleartextDirectoryId, String nativePathSep) {
+		return pathCache.computeIfAbsent(cleartextDirectoryId, id -> cryptor.encryptDirectoryPath(id, nativePathSep));
 	}
 
 	@Override
 	public String encryptFilename(String cleartextName) {
-		if (nameCache.containsKey(cleartextName)) {
-			return nameCache.get(cleartextName);
-		} else {
-			final String ciphertextName = cryptor.encryptFilename(cleartextName);
-			nameCache.put(cleartextName, ciphertextName);
-			return ciphertextName;
-		}
+		return nameCache.computeIfAbsent(cleartextName, name -> cryptor.encryptFilename(name));
 	}
 
 	@Override
 	public String decryptFilename(String ciphertextName) throws DecryptFailedException {
-		if (nameCache.containsValue(ciphertextName)) {
-			return nameCache.getKey(ciphertextName);
-		} else {
-			final String cleartextName = cryptor.decryptFilename(ciphertextName);
+		String cleartextName = nameCache.getKey(ciphertextName);
+		if (cleartextName == null) {
+			cleartextName = cryptor.decryptFilename(ciphertextName);
 			nameCache.put(cleartextName, ciphertextName);
-			return ciphertextName;
 		}
+		return cleartextName;
 	}
 
 	private static class BidiLRUMap<K, V> extends AbstractDualBidiMap<K, V> {