From 7b42679baf2ad90a3218e14aa0e07b4f8264c8e0 Mon Sep 17 00:00:00 2001 From: BJ Hargrave Date: Fri, 23 Sep 2022 14:50:32 -0400 Subject: [PATCH] resource: Add a File Resource cache to ResourceBuilder The cache reduces the need to process files to Resource objects, including SHA-256 computation, for unchanged files. Fixes https://github.com/bndtools/bnd/issues/5367 Signed-off-by: BJ Hargrave --- .../resource/FileResourceCacheKeyTest.java | 127 ++++++++++++++++ .../bnd/osgi/resource/FileResourceCache.java | 136 ++++++++++++++++++ .../bnd/osgi/resource/ResourceBuilder.java | 27 +--- .../fileset/FileSetRepositoryTest.java | 11 ++ 4 files changed, 280 insertions(+), 21 deletions(-) create mode 100644 biz.aQute.bndlib.tests/test/aQute/bnd/osgi/resource/FileResourceCacheKeyTest.java create mode 100644 biz.aQute.bndlib/src/aQute/bnd/osgi/resource/FileResourceCache.java diff --git a/biz.aQute.bndlib.tests/test/aQute/bnd/osgi/resource/FileResourceCacheKeyTest.java b/biz.aQute.bndlib.tests/test/aQute/bnd/osgi/resource/FileResourceCacheKeyTest.java new file mode 100644 index 0000000000..841d139138 --- /dev/null +++ b/biz.aQute.bndlib.tests/test/aQute/bnd/osgi/resource/FileResourceCacheKeyTest.java @@ -0,0 +1,127 @@ +package aQute.bnd.osgi.resource; + +import static org.assertj.core.api.Assertions.assertThatObject; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributeView; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileTime; +import java.time.Instant; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; + +import aQute.bnd.osgi.resource.FileResourceCache.CacheKey; +import aQute.bnd.test.jupiter.InjectTemporaryDirectory; +import aQute.lib.io.IO; + +class FileResourceCacheKeyTest { + + @Test + void unchanged(@InjectTemporaryDirectory + Path tmp) throws Exception { + Path subject = tmp.resolve("test"); + IO.store("line1", subject, StandardCharsets.UTF_8); + CacheKey key1 = new CacheKey(subject); + CacheKey key2 = new CacheKey(subject); + assertThatObject(key1).isEqualTo(key2); + assertThatObject(key1).hasSameHashCodeAs(key2); + } + + @Test + void change_modified(@InjectTemporaryDirectory + Path tmp) throws Exception { + Path subject = tmp.resolve("test"); + IO.store("line1", subject, StandardCharsets.UTF_8); + CacheKey key1 = new CacheKey(subject); + BasicFileAttributes attributes = Files.getFileAttributeView(subject, BasicFileAttributeView.class) + .readAttributes(); + FileTime lastModifiedTime = attributes.lastModifiedTime(); + Instant plusSeconds = lastModifiedTime.toInstant() + .plusSeconds(10L); + Files.setLastModifiedTime(subject, FileTime.from(plusSeconds)); + CacheKey key2 = new CacheKey(subject); + assertThatObject(key1).isNotEqualTo(key2); + assertThatObject(key1).doesNotHaveSameHashCodeAs(key2); + } + + @Test + void change_size(@InjectTemporaryDirectory + Path tmp) throws Exception { + Path subject = tmp.resolve("test"); + IO.store("line1", subject, StandardCharsets.UTF_8); + CacheKey key1 = new CacheKey(subject); + BasicFileAttributes attributes = Files.getFileAttributeView(subject, BasicFileAttributeView.class) + .readAttributes(); + FileTime lastModifiedTime = attributes.lastModifiedTime(); + IO.store("line100", subject, StandardCharsets.UTF_8); + Files.setLastModifiedTime(subject, lastModifiedTime); + CacheKey key2 = new CacheKey(subject); + assertThatObject(key1).isNotEqualTo(key2); + assertThatObject(key1).doesNotHaveSameHashCodeAs(key2); + } + + @DisabledOnOs(value = OS.WINDOWS, disabledReason = "Windows FS does not support fileKey") + @Test + void change_filekey(@InjectTemporaryDirectory + Path tmp) throws Exception { + Path subject = tmp.resolve("test"); + IO.store("line1", subject, StandardCharsets.UTF_8); + CacheKey key1 = new CacheKey(subject); + BasicFileAttributes attributes = Files.getFileAttributeView(subject, BasicFileAttributeView.class) + .readAttributes(); + assertThatObject(attributes.fileKey()).isNotNull(); + FileTime lastModifiedTime = attributes.lastModifiedTime(); + Path subject2 = tmp.resolve("test.tmp"); + IO.store("line2", subject2, StandardCharsets.UTF_8); + Files.setLastModifiedTime(subject2, lastModifiedTime); + IO.rename(subject2, subject); + CacheKey key2 = new CacheKey(subject); + attributes = Files.getFileAttributeView(subject, BasicFileAttributeView.class) + .readAttributes(); + assertThatObject(attributes.fileKey()).isNotNull(); + assertThatObject(key1).as("key2 not equal") + .isNotEqualTo(key2); + assertThatObject(key1).as("key2 different hash") + .doesNotHaveSameHashCodeAs(key2); + } + + @Test + void change_file_modified(@InjectTemporaryDirectory + Path tmp) throws Exception { + Path subject = tmp.resolve("test"); + IO.store("line1", subject, StandardCharsets.UTF_8); + CacheKey key1 = new CacheKey(subject); + Path subject2 = tmp.resolve("test.tmp"); + IO.store("line2", subject2, StandardCharsets.UTF_8); + BasicFileAttributes attributes = Files.getFileAttributeView(subject2, BasicFileAttributeView.class) + .readAttributes(); + FileTime lastModifiedTime = attributes.lastModifiedTime(); + Instant plusSeconds = lastModifiedTime.toInstant() + .plusSeconds(10L); + Files.setLastModifiedTime(subject2, FileTime.from(plusSeconds)); + IO.rename(subject2, subject); + CacheKey key2 = new CacheKey(subject); + assertThatObject(key1).as("key2 not equal") + .isNotEqualTo(key2); + assertThatObject(key1).as("key2 different hash") + .doesNotHaveSameHashCodeAs(key2); + } + + @Test + void different_files(@InjectTemporaryDirectory + Path tmp) throws Exception { + Path subject1 = tmp.resolve("test1"); + IO.store("line1", subject1, StandardCharsets.UTF_8); + CacheKey key1 = new CacheKey(subject1); + Path subject2 = tmp.resolve("test2"); + IO.copy(subject1, subject2); + CacheKey key2 = new CacheKey(subject2); + assertThatObject(key1).isNotEqualTo(key2); + assertThatObject(key1).doesNotHaveSameHashCodeAs(key2); + } + +} diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/resource/FileResourceCache.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/resource/FileResourceCache.java new file mode 100644 index 0000000000..9bfe5e79a9 --- /dev/null +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/resource/FileResourceCache.java @@ -0,0 +1,136 @@ +package aQute.bnd.osgi.resource; + +import static aQute.bnd.exceptions.SupplierWithException.asSupplierOrElse; +import static aQute.bnd.osgi.Constants.MIME_TYPE_BUNDLE; +import static aQute.bnd.osgi.Constants.MIME_TYPE_JAR; + +import java.io.File; +import java.io.IOException; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributeView; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; + +import org.osgi.resource.Resource; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import aQute.bnd.exceptions.Exceptions; +import aQute.bnd.osgi.Domain; +import aQute.libg.cryptography.SHA256; + +class FileResourceCache { + private final static Logger logger = LoggerFactory.getLogger(FileResourceCache.class); + private final static long EXPIRED_DURATION_NANOS = TimeUnit.NANOSECONDS.convert(30L, + TimeUnit.MINUTES); + private static final FileResourceCache INSTANCE = new FileResourceCache(); + private final Map cache; + private long time; + + private FileResourceCache() { + cache = new ConcurrentHashMap<>(); + time = System.nanoTime(); + } + + static FileResourceCache getInstance() { + return INSTANCE; + } + + Resource getResource(File file, URI uri) { + if (!file.isFile()) { + return null; + } + // Make sure we don't grow infinitely + final long now = System.nanoTime(); + if ((now - time) > EXPIRED_DURATION_NANOS) { + time = now; + cache.keySet() + .removeIf(key -> (now - key.time) > EXPIRED_DURATION_NANOS); + } + CacheKey cacheKey = new CacheKey(file); + Resource resource = cache.computeIfAbsent(cacheKey, key -> { + logger.debug("parsing {}", file); + ResourceBuilder rb = new ResourceBuilder(); + try { + Domain manifest = Domain.domain(file); + boolean hasIdentity = false; + if (manifest != null) { + hasIdentity = rb.addManifest(manifest); + } + String mime = hasIdentity ? MIME_TYPE_BUNDLE : MIME_TYPE_JAR; + DeferredValue sha256 = new DeferredComparableValue<>(String.class, + asSupplierOrElse(() -> SHA256.digest(file) + .asHex(), null), + key.hashCode()); + rb.addContentCapability(uri, sha256, file.length(), mime); + + if (hasIdentity) { + rb.addHashes(file); + } + } catch (Exception e) { + throw Exceptions.duck(e); + } + return rb.build(); + }); + return resource; + } + + static final class CacheKey { + private final Object fileKey; + private final long lastModifiedTime; + private final long size; + final long time; + + CacheKey(File file) { + this(file.toPath()); + } + + CacheKey(Path path) { + BasicFileAttributes attributes; + try { + attributes = Files.getFileAttributeView(path, BasicFileAttributeView.class) + .readAttributes(); + } catch (IOException e) { + throw Exceptions.duck(e); + } + if (!attributes.isRegularFile()) { + throw new IllegalArgumentException("File must be a regular file: " + path); + } + Object fileKey = attributes.fileKey(); + this.fileKey = (fileKey != null) ? fileKey // + : path.toAbsolutePath(); // Windows FS does not have fileKey + this.lastModifiedTime = attributes.lastModifiedTime() + .toMillis(); + this.size = attributes.size(); + this.time = System.nanoTime(); + } + + @Override + public int hashCode() { + return (Objects.hashCode(fileKey) * 31 + Long.hashCode(lastModifiedTime)) * 31 + Long.hashCode(size); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof CacheKey)) { + return false; + } + CacheKey other = (CacheKey) obj; + return Objects.equals(fileKey, other.fileKey) && (lastModifiedTime == other.lastModifiedTime) + && (size == other.size); + } + + @Override + public String toString() { + return Objects.toString(fileKey); + } + } +} diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/resource/ResourceBuilder.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/resource/ResourceBuilder.java index 558c3753ab..17ca1ffdf8 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/resource/ResourceBuilder.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/resource/ResourceBuilder.java @@ -1,6 +1,5 @@ package aQute.bnd.osgi.resource; -import static aQute.bnd.exceptions.SupplierWithException.asSupplierOrElse; import static aQute.bnd.osgi.Constants.DUPLICATE_MARKER; import static aQute.bnd.osgi.Constants.MIME_TYPE_BUNDLE; import static aQute.bnd.osgi.Constants.MIME_TYPE_JAR; @@ -61,7 +60,6 @@ import aQute.lib.hierarchy.Hierarchy; import aQute.lib.hierarchy.NamedNode; import aQute.lib.zip.JarIndex; -import aQute.libg.cryptography.SHA256; import aQute.libg.reporter.ReporterAdapter; import aQute.service.reporter.Reporter; @@ -730,30 +728,17 @@ public boolean addFile(File file, URI uri) throws Exception { if (uri == null) uri = file.toURI(); - Domain manifest = Domain.domain(file); boolean hasIdentity = false; - if (manifest != null) { - hasIdentity = addManifest(manifest); - } - String mime = hasIdentity ? MIME_TYPE_BUNDLE : MIME_TYPE_JAR; - int deferredHashCode = hashCode(file); - DeferredValue sha256 = new DeferredComparableValue<>(String.class, - asSupplierOrElse(() -> SHA256.digest(file) - .asHex(), null), - deferredHashCode); - addContentCapability(uri, sha256, file.length(), mime); - - if (hasIdentity) { - addHashes(file); + Resource fileResource = FileResourceCache.getInstance() + .getResource(file, uri); + if (fileResource != null) { + addResource(fileResource); + hasIdentity = !fileResource.getCapabilities(IdentityNamespace.IDENTITY_NAMESPACE) + .isEmpty(); } return hasIdentity; } - private static int hashCode(File file) { - return file.getAbsoluteFile() - .hashCode(); - } - /** * Add simple class name hashes to the exported packages. This should not be * called before any package capabilities are set since we only hash class diff --git a/biz.aQute.repository/test/aQute/bnd/repository/fileset/FileSetRepositoryTest.java b/biz.aQute.repository/test/aQute/bnd/repository/fileset/FileSetRepositoryTest.java index 8fe12f74c3..54d811085b 100644 --- a/biz.aQute.repository/test/aQute/bnd/repository/fileset/FileSetRepositoryTest.java +++ b/biz.aQute.repository/test/aQute/bnd/repository/fileset/FileSetRepositoryTest.java @@ -30,6 +30,17 @@ public void includesMavenArtifacts() throws Exception { assertThat(repository.list(null)).contains("org.nanohttpd:nanohttpd", "javafx.base") .doesNotContain("javax.annotation:jsr250-api"); + // Do it again which will get file resources from the cache + repository = new FileSetRepository("test2", files); + + assertThat(repository.list(null)).contains("org.nanohttpd:nanohttpd", "javafx.base") + .doesNotContain("javax.annotation:jsr250-api"); + + assertThat(repository.refresh()).isTrue(); + + assertThat(repository.list(null)).contains("org.nanohttpd:nanohttpd", "javafx.base") + .doesNotContain("javax.annotation:jsr250-api"); + } }