From e6b04ec7807cfd69017b26f2cb6d19c86b3b852b Mon Sep 17 00:00:00 2001 From: BJ Hargrave Date: Mon, 19 Sep 2022 18:42:04 -0400 Subject: [PATCH] repository: Add a Resource cache to FileSetRepository The cache reduces the need to create new Resource objects, including SHA-256 computation, for unchanged files. Fixes https://github.com/bndtools/bnd/issues/5367 Signed-off-by: BJ Hargrave --- .../repository/fileset/FileSetRepository.java | 60 +-------- .../bnd/repository/fileset/ResourceCache.java | 93 +++++++++++++ .../repository/fileset/ResourceCacheKey.java | 68 ++++++++++ .../fileset/RepositoryCacheKeyTest.java | 126 ++++++++++++++++++ 4 files changed, 293 insertions(+), 54 deletions(-) create mode 100644 biz.aQute.repository/src/aQute/bnd/repository/fileset/ResourceCache.java create mode 100644 biz.aQute.repository/src/aQute/bnd/repository/fileset/ResourceCacheKey.java create mode 100644 biz.aQute.repository/test/aQute/bnd/repository/fileset/RepositoryCacheKeyTest.java diff --git a/biz.aQute.repository/src/aQute/bnd/repository/fileset/FileSetRepository.java b/biz.aQute.repository/src/aQute/bnd/repository/fileset/FileSetRepository.java index 7abfcf8a9a8..2178dca44af 100644 --- a/biz.aQute.repository/src/aQute/bnd/repository/fileset/FileSetRepository.java +++ b/biz.aQute.repository/src/aQute/bnd/repository/fileset/FileSetRepository.java @@ -1,7 +1,5 @@ package aQute.bnd.repository.fileset; -import static aQute.bnd.exceptions.FunctionWithException.asFunctionOrElse; - import java.io.File; import java.io.InputStream; import java.net.URI; @@ -10,10 +8,8 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.SortedSet; -import org.osgi.framework.namespace.IdentityNamespace; import org.osgi.resource.Capability; import org.osgi.resource.Requirement; import org.osgi.resource.Resource; @@ -24,23 +20,17 @@ import org.slf4j.LoggerFactory; import aQute.bnd.exceptions.Exceptions; -import aQute.bnd.osgi.Jar; import aQute.bnd.osgi.repository.BaseRepository; import aQute.bnd.osgi.repository.BridgeRepository; import aQute.bnd.osgi.repository.ResourcesRepository; -import aQute.bnd.osgi.resource.CapReqBuilder; -import aQute.bnd.osgi.resource.ResourceBuilder; import aQute.bnd.osgi.resource.ResourceUtils; import aQute.bnd.osgi.resource.ResourceUtils.ContentCapability; import aQute.bnd.service.Plugin; import aQute.bnd.service.Refreshable; import aQute.bnd.service.RepositoryPlugin; import aQute.bnd.util.repository.DownloadListenerPromise; -import aQute.bnd.version.MavenVersion; import aQute.bnd.version.Version; import aQute.lib.strings.Strings; -import aQute.maven.api.Revision; -import aQute.maven.provider.POM; import aQute.service.reporter.Reporter; public class FileSetRepository extends BaseRepository implements Plugin, RepositoryPlugin, Refreshable { @@ -50,6 +40,7 @@ public class FileSetRepository extends BaseRepository implements Plugin, Reposit private volatile Deferred repository; private Reporter reporter; private final PromiseFactory promiseFactory; + private static final ResourceCache cache = new ResourceCache(); public FileSetRepository(String name, Collection files) throws Exception { this.name = name; @@ -90,53 +81,14 @@ private Promise readFiles() { } private Promise parseFile(File file) { - Promise resource = promiseFactory.submit(() -> { - if (!file.isFile()) { - return null; - } - ResourceBuilder rb = new ResourceBuilder(); - try { - boolean hasIdentity = rb.addFile(file, null); - if (!hasIdentity) { - try (Jar jar = new Jar(file)) { - Optional revision = jar.getPomXmlResources() - .findFirst() - .map(asFunctionOrElse(pomResource -> new POM(null, pomResource.openInputStream(), true), - null)) - .map(POM::getRevision); - - String name = jar.getModuleName(); - if (name == null) { - name = revision.map(r -> r.program.toString()) - .orElse(null); - if (name == null) { - return null; - } - } - - Version version = revision.map(r -> r.version.getOSGiVersion()) - .orElse(null); - if (version == null) { - version = new MavenVersion(jar.getModuleVersion()).getOSGiVersion(); - } - - CapReqBuilder identity = new CapReqBuilder(IdentityNamespace.IDENTITY_NAMESPACE) - .addAttribute(IdentityNamespace.IDENTITY_NAMESPACE, name) - .addAttribute(IdentityNamespace.CAPABILITY_VERSION_ATTRIBUTE, version) - .addAttribute(IdentityNamespace.CAPABILITY_TYPE_ATTRIBUTE, IdentityNamespace.TYPE_UNKNOWN); - rb.addCapability(identity); - } - } - } catch (Exception f) { - return null; - } - logger.debug("{}: parsing {}", getName(), file); - return rb.build(); + Promise promise = promiseFactory.submit(() -> { + Resource resource = cache.getResource(this, file); + return resource; }); if (logger.isDebugEnabled()) { - resource.onFailure(failure -> logger.debug("{}: failed to parse {}", getName(), file, failure)); + promise.onFailure(failure -> logger.debug("{}: failed to parse {}", getName(), file, failure)); } - return resource; + return promise; } @Override diff --git a/biz.aQute.repository/src/aQute/bnd/repository/fileset/ResourceCache.java b/biz.aQute.repository/src/aQute/bnd/repository/fileset/ResourceCache.java new file mode 100644 index 00000000000..79a46621d47 --- /dev/null +++ b/biz.aQute.repository/src/aQute/bnd/repository/fileset/ResourceCache.java @@ -0,0 +1,93 @@ +package aQute.bnd.repository.fileset; + +import static aQute.bnd.exceptions.FunctionWithException.asFunctionOrElse; + +import java.io.File; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; + +import org.osgi.framework.namespace.IdentityNamespace; +import org.osgi.resource.Resource; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import aQute.bnd.osgi.Jar; +import aQute.bnd.osgi.resource.CapReqBuilder; +import aQute.bnd.osgi.resource.ResourceBuilder; +import aQute.bnd.service.RepositoryPlugin; +import aQute.bnd.version.MavenVersion; +import aQute.bnd.version.Version; +import aQute.maven.api.Revision; +import aQute.maven.provider.POM; + +final class ResourceCache { + private final static Logger logger = LoggerFactory + .getLogger(ResourceCache.class); + private final static long EXPIRED_DURATION_NANOS = TimeUnit.NANOSECONDS.convert(30L, + TimeUnit.MINUTES); + private final Map cache; + private long time; + + ResourceCache() { + cache = new ConcurrentHashMap<>(); + time = System.nanoTime(); + } + + Resource getResource(RepositoryPlugin repo, File file) { + if (!file.isFile()) { + return null; + } + // Make sure we don't grow infinitely + final long now = System.nanoTime(); + if ((now - time) > EXPIRED_DURATION_NANOS) { + cache.keySet() + .removeIf(key -> (now - key.time) > EXPIRED_DURATION_NANOS); + time = now; + } + ResourceCacheKey cacheKey = new ResourceCacheKey(file); + Resource resource = cache.computeIfAbsent(cacheKey, key -> { + ResourceBuilder rb = new ResourceBuilder(); + try { + boolean hasIdentity = rb.addFile(file, null); + if (!hasIdentity) { + try (Jar jar = new Jar(file)) { + Optional revision = jar.getPomXmlResources() + .findFirst() + .map(asFunctionOrElse(pomResource -> new POM(null, pomResource.openInputStream(), true), + null)) + .map(POM::getRevision); + + String name = jar.getModuleName(); + if (name == null) { + name = revision.map(r -> r.program.toString()) + .orElse(null); + if (name == null) { + return null; + } + } + + Version version = revision.map(r -> r.version.getOSGiVersion()) + .orElse(null); + if (version == null) { + version = new MavenVersion(jar.getModuleVersion()).getOSGiVersion(); + } + + CapReqBuilder identity = new CapReqBuilder(IdentityNamespace.IDENTITY_NAMESPACE) + .addAttribute(IdentityNamespace.IDENTITY_NAMESPACE, name) + .addAttribute(IdentityNamespace.CAPABILITY_VERSION_ATTRIBUTE, version) + .addAttribute(IdentityNamespace.CAPABILITY_TYPE_ATTRIBUTE, IdentityNamespace.TYPE_UNKNOWN); + rb.addCapability(identity); + } + } + } catch (Exception f) { + return null; + } + logger.debug("{}: parsing {}", repo.getName(), file); + return rb.build(); + }); + + return resource; + } +} diff --git a/biz.aQute.repository/src/aQute/bnd/repository/fileset/ResourceCacheKey.java b/biz.aQute.repository/src/aQute/bnd/repository/fileset/ResourceCacheKey.java new file mode 100644 index 00000000000..607c43f1232 --- /dev/null +++ b/biz.aQute.repository/src/aQute/bnd/repository/fileset/ResourceCacheKey.java @@ -0,0 +1,68 @@ +package aQute.bnd.repository.fileset; + +import java.io.File; +import java.io.IOException; +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.util.Objects; +import java.util.Optional; + +import aQute.bnd.exceptions.Exceptions; + +final class ResourceCacheKey { + private final Path path; + private final Object fileKey; + private final FileTime lastModifiedTime; + private final long size; + final long time; + + ResourceCacheKey(File file) { + this(file.toPath()); + } + + ResourceCacheKey(Path path) { + path = path.toAbsolutePath(); + 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); + } + this.path = path; + this.fileKey = Optional.ofNullable(attributes.fileKey()) + .orElse(path); // Windows + this.lastModifiedTime = attributes.lastModifiedTime(); + this.size = attributes.size(); + this.time = System.nanoTime(); + } + + @Override + public int hashCode() { + return (Objects.hashCode(fileKey) * 31 + Objects.hashCode(lastModifiedTime)) * 31 + Long.hashCode(size); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof ResourceCacheKey)) { + return false; + } + ResourceCacheKey other = (ResourceCacheKey) obj; + return Objects.equals(fileKey, other.fileKey) && Objects.equals(lastModifiedTime, other.lastModifiedTime) + && (size == other.size); + } + + @Override + public String toString() { + return path.toString(); + } +} diff --git a/biz.aQute.repository/test/aQute/bnd/repository/fileset/RepositoryCacheKeyTest.java b/biz.aQute.repository/test/aQute/bnd/repository/fileset/RepositoryCacheKeyTest.java new file mode 100644 index 00000000000..eb738799c86 --- /dev/null +++ b/biz.aQute.repository/test/aQute/bnd/repository/fileset/RepositoryCacheKeyTest.java @@ -0,0 +1,126 @@ +package aQute.bnd.repository.fileset; + +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.test.jupiter.InjectTemporaryDirectory; +import aQute.lib.io.IO; + +class RepositoryCacheKeyTest { + + @Test + void unchanged(@InjectTemporaryDirectory + Path tmp) throws Exception { + Path subject = tmp.resolve("test"); + IO.store("line1", subject, StandardCharsets.UTF_8); + ResourceCacheKey key1 = new ResourceCacheKey(subject); + ResourceCacheKey key2 = new ResourceCacheKey(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); + ResourceCacheKey key1 = new ResourceCacheKey(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)); + ResourceCacheKey key2 = new ResourceCacheKey(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); + ResourceCacheKey key1 = new ResourceCacheKey(subject); + BasicFileAttributes attributes = Files.getFileAttributeView(subject, BasicFileAttributeView.class) + .readAttributes(); + FileTime lastModifiedTime = attributes.lastModifiedTime(); + IO.store("line100", subject, StandardCharsets.UTF_8); + Files.setLastModifiedTime(subject, lastModifiedTime); + ResourceCacheKey key2 = new ResourceCacheKey(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); + ResourceCacheKey key1 = new ResourceCacheKey(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); + ResourceCacheKey key2 = new ResourceCacheKey(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); + ResourceCacheKey key1 = new ResourceCacheKey(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); + ResourceCacheKey key2 = new ResourceCacheKey(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); + ResourceCacheKey key1 = new ResourceCacheKey(subject1); + Path subject2 = tmp.resolve("test2"); + IO.copy(subject1, subject2); + ResourceCacheKey key2 = new ResourceCacheKey(subject2); + assertThatObject(key1).isNotEqualTo(key2); + assertThatObject(key1).doesNotHaveSameHashCodeAs(key2); + } + +}