From 49646aae41eee3adf3e574a8f49aae0f12ee5f4c Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 29 May 2015 10:47:50 +0200 Subject: [PATCH] improved directory name caching (>95% hitrate now) --- .../jackrabbit/CryptoResourceFactory.java | 34 +--------- .../webdav/jackrabbit/EncryptedDir.java | 39 +++--------- .../webdav/jackrabbit/EncryptedFilePart.java | 2 +- .../webdav/jackrabbit/FilenameTranslator.java | 62 ++++++++++++++++--- .../crypto/AbstractCryptorDecorator.java | 4 +- .../java/org/cryptomator/crypto/Cryptor.java | 2 +- .../crypto/PathCachingCryptorDecorator.java | 29 +++------ 7 files changed, 74 insertions(+), 98 deletions(-) diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/CryptoResourceFactory.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/CryptoResourceFactory.java index b83c1e6b6..b8037dde8 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/CryptoResourceFactory.java +++ b/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(); - } - } - } diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedDir.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedDir.java index 662e07e12..b06d1c042 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedDir.java +++ b/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); } } diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFilePart.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFilePart.java index 454bda21f..2f997d0cd 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/EncryptedFilePart.java +++ b/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); diff --git a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/FilenameTranslator.java b/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/FilenameTranslator.java index de6d74ff2..7c99fff5a 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/jackrabbit/FilenameTranslator.java +++ b/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, String> directoryIdCache = new LRUMap<>(MAX_CACHED_DIRECTORY_IDS); // 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 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 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(); } } diff --git a/main/crypto-api/src/main/java/org/cryptomator/crypto/AbstractCryptorDecorator.java b/main/crypto-api/src/main/java/org/cryptomator/crypto/AbstractCryptorDecorator.java index 9f91ddac1..71d10ed10 100644 --- a/main/crypto-api/src/main/java/org/cryptomator/crypto/AbstractCryptorDecorator.java +++ b/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 diff --git a/main/crypto-api/src/main/java/org/cryptomator/crypto/Cryptor.java b/main/crypto-api/src/main/java/org/cryptomator/crypto/Cryptor.java index c2c479f5e..2474ee907 100644 --- a/main/crypto-api/src/main/java/org/cryptomator/crypto/Cryptor.java +++ b/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. */ diff --git a/main/crypto-api/src/main/java/org/cryptomator/crypto/PathCachingCryptorDecorator.java b/main/crypto-api/src/main/java/org/cryptomator/crypto/PathCachingCryptorDecorator.java index d22c350db..39241da5f 100644 --- a/main/crypto-api/src/main/java/org/cryptomator/crypto/PathCachingCryptorDecorator.java +++ b/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 pathCache = new LRUMap<>(MAX_CACHED_PATHS); // + private final Map pathCache = new LRUMap<>(MAX_CACHED_PATHS); // private final BidiMap nameCache = new BidiLRUMap<>(MAX_CACHED_NAMES); // 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 extends AbstractDualBidiMap {