Skip to content

Commit a4d466d

Browse files
committed
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 #5367 Signed-off-by: BJ Hargrave <[email protected]>
1 parent 6cd8742 commit a4d466d

File tree

4 files changed

+286
-21
lines changed

4 files changed

+286
-21
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
package aQute.bnd.osgi.resource;
2+
3+
import static org.assertj.core.api.Assertions.assertThatObject;
4+
5+
import java.nio.charset.StandardCharsets;
6+
import java.nio.file.Files;
7+
import java.nio.file.Path;
8+
import java.nio.file.attribute.BasicFileAttributeView;
9+
import java.nio.file.attribute.BasicFileAttributes;
10+
import java.nio.file.attribute.FileTime;
11+
import java.time.Instant;
12+
13+
import org.junit.jupiter.api.Test;
14+
import org.junit.jupiter.api.condition.DisabledOnOs;
15+
import org.junit.jupiter.api.condition.OS;
16+
17+
import aQute.bnd.osgi.resource.FileResourceCache.CacheKey;
18+
import aQute.bnd.test.jupiter.InjectTemporaryDirectory;
19+
import aQute.lib.io.IO;
20+
21+
class FileResourceCacheKeyTest {
22+
23+
@Test
24+
void unchanged(@InjectTemporaryDirectory
25+
Path tmp) throws Exception {
26+
Path subject = tmp.resolve("test");
27+
IO.store("line1", subject, StandardCharsets.UTF_8);
28+
CacheKey key1 = new CacheKey(subject);
29+
CacheKey key2 = new CacheKey(subject);
30+
assertThatObject(key1).isEqualTo(key2);
31+
assertThatObject(key1).hasSameHashCodeAs(key2);
32+
}
33+
34+
@Test
35+
void change_modified(@InjectTemporaryDirectory
36+
Path tmp) throws Exception {
37+
Path subject = tmp.resolve("test");
38+
IO.store("line1", subject, StandardCharsets.UTF_8);
39+
CacheKey key1 = new CacheKey(subject);
40+
BasicFileAttributes attributes = Files.getFileAttributeView(subject, BasicFileAttributeView.class)
41+
.readAttributes();
42+
FileTime lastModifiedTime = attributes.lastModifiedTime();
43+
Instant plusSeconds = lastModifiedTime.toInstant()
44+
.plusSeconds(10L);
45+
Files.setLastModifiedTime(subject, FileTime.from(plusSeconds));
46+
CacheKey key2 = new CacheKey(subject);
47+
assertThatObject(key1).isNotEqualTo(key2);
48+
assertThatObject(key1).doesNotHaveSameHashCodeAs(key2);
49+
}
50+
51+
@Test
52+
void change_size(@InjectTemporaryDirectory
53+
Path tmp) throws Exception {
54+
Path subject = tmp.resolve("test");
55+
IO.store("line1", subject, StandardCharsets.UTF_8);
56+
CacheKey key1 = new CacheKey(subject);
57+
BasicFileAttributes attributes = Files.getFileAttributeView(subject, BasicFileAttributeView.class)
58+
.readAttributes();
59+
FileTime lastModifiedTime = attributes.lastModifiedTime();
60+
IO.store("line100", subject, StandardCharsets.UTF_8);
61+
Files.setLastModifiedTime(subject, lastModifiedTime);
62+
CacheKey key2 = new CacheKey(subject);
63+
assertThatObject(key1).isNotEqualTo(key2);
64+
assertThatObject(key1).doesNotHaveSameHashCodeAs(key2);
65+
}
66+
67+
@DisabledOnOs(value = OS.WINDOWS, disabledReason = "Windows FS does not support fileKey")
68+
@Test
69+
void change_filekey(@InjectTemporaryDirectory
70+
Path tmp) throws Exception {
71+
Path subject = tmp.resolve("test");
72+
IO.store("line1", subject, StandardCharsets.UTF_8);
73+
CacheKey key1 = new CacheKey(subject);
74+
BasicFileAttributes attributes = Files.getFileAttributeView(subject, BasicFileAttributeView.class)
75+
.readAttributes();
76+
assertThatObject(attributes.fileKey()).isNotNull();
77+
FileTime lastModifiedTime = attributes.lastModifiedTime();
78+
Path subject2 = tmp.resolve("test.tmp");
79+
IO.store("line2", subject2, StandardCharsets.UTF_8);
80+
Files.setLastModifiedTime(subject2, lastModifiedTime);
81+
IO.rename(subject2, subject);
82+
CacheKey key2 = new CacheKey(subject);
83+
attributes = Files.getFileAttributeView(subject, BasicFileAttributeView.class)
84+
.readAttributes();
85+
assertThatObject(attributes.fileKey()).isNotNull();
86+
assertThatObject(key1).as("key2 not equal")
87+
.isNotEqualTo(key2);
88+
assertThatObject(key1).as("key2 different hash")
89+
.doesNotHaveSameHashCodeAs(key2);
90+
}
91+
92+
@Test
93+
void change_file_modified(@InjectTemporaryDirectory
94+
Path tmp) throws Exception {
95+
Path subject = tmp.resolve("test");
96+
IO.store("line1", subject, StandardCharsets.UTF_8);
97+
CacheKey key1 = new CacheKey(subject);
98+
Path subject2 = tmp.resolve("test.tmp");
99+
IO.store("line2", subject2, StandardCharsets.UTF_8);
100+
BasicFileAttributes attributes = Files.getFileAttributeView(subject2, BasicFileAttributeView.class)
101+
.readAttributes();
102+
FileTime lastModifiedTime = attributes.lastModifiedTime();
103+
Instant plusSeconds = lastModifiedTime.toInstant()
104+
.plusSeconds(10L);
105+
Files.setLastModifiedTime(subject2, FileTime.from(plusSeconds));
106+
IO.rename(subject2, subject);
107+
CacheKey key2 = new CacheKey(subject);
108+
assertThatObject(key1).as("key2 not equal")
109+
.isNotEqualTo(key2);
110+
assertThatObject(key1).as("key2 different hash")
111+
.doesNotHaveSameHashCodeAs(key2);
112+
}
113+
114+
@Test
115+
void different_files(@InjectTemporaryDirectory
116+
Path tmp) throws Exception {
117+
Path subject1 = tmp.resolve("test1");
118+
IO.store("line1", subject1, StandardCharsets.UTF_8);
119+
CacheKey key1 = new CacheKey(subject1);
120+
Path subject2 = tmp.resolve("test2");
121+
IO.copy(subject1, subject2);
122+
CacheKey key2 = new CacheKey(subject2);
123+
assertThatObject(key1).isNotEqualTo(key2);
124+
assertThatObject(key1).doesNotHaveSameHashCodeAs(key2);
125+
}
126+
127+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
package aQute.bnd.osgi.resource;
2+
3+
import static aQute.bnd.exceptions.SupplierWithException.asSupplierOrElse;
4+
import static aQute.bnd.osgi.Constants.MIME_TYPE_BUNDLE;
5+
import static aQute.bnd.osgi.Constants.MIME_TYPE_JAR;
6+
7+
import java.io.File;
8+
import java.io.IOException;
9+
import java.net.URI;
10+
import java.nio.file.Files;
11+
import java.nio.file.Path;
12+
import java.nio.file.attribute.BasicFileAttributeView;
13+
import java.nio.file.attribute.BasicFileAttributes;
14+
import java.util.Map;
15+
import java.util.Objects;
16+
import java.util.concurrent.ConcurrentHashMap;
17+
import java.util.concurrent.TimeUnit;
18+
19+
import org.osgi.resource.Resource;
20+
import org.slf4j.Logger;
21+
import org.slf4j.LoggerFactory;
22+
23+
import aQute.bnd.exceptions.Exceptions;
24+
import aQute.bnd.osgi.Domain;
25+
import aQute.libg.cryptography.SHA256;
26+
27+
class FileResourceCache {
28+
private final static Logger logger = LoggerFactory.getLogger(FileResourceCache.class);
29+
private final static long EXPIRED_DURATION_NANOS = TimeUnit.NANOSECONDS.convert(30L,
30+
TimeUnit.MINUTES);
31+
private static final FileResourceCache INSTANCE = new FileResourceCache();
32+
private final Map<CacheKey, Resource> cache;
33+
private long time;
34+
35+
private FileResourceCache() {
36+
cache = new ConcurrentHashMap<>();
37+
time = System.nanoTime();
38+
}
39+
40+
static FileResourceCache getInstance() {
41+
return INSTANCE;
42+
}
43+
44+
Resource getResource(File file, URI uri) {
45+
if (!file.isFile()) {
46+
return null;
47+
}
48+
// Make sure we don't grow infinitely
49+
final long now = System.nanoTime();
50+
if ((now - time) > EXPIRED_DURATION_NANOS) {
51+
cache.keySet()
52+
.removeIf(key -> (now - key.time) > EXPIRED_DURATION_NANOS);
53+
time = now;
54+
}
55+
CacheKey cacheKey = new CacheKey(file);
56+
Resource resource = cache.computeIfAbsent(cacheKey, key -> {
57+
logger.debug("parsing {}", file);
58+
ResourceBuilder rb = new ResourceBuilder();
59+
try {
60+
Domain manifest = Domain.domain(file);
61+
boolean hasIdentity = false;
62+
if (manifest != null) {
63+
hasIdentity = rb.addManifest(manifest);
64+
}
65+
String mime = hasIdentity ? MIME_TYPE_BUNDLE : MIME_TYPE_JAR;
66+
int deferredHashCode = hashCode(file);
67+
DeferredValue<String> sha256 = new DeferredComparableValue<>(String.class,
68+
asSupplierOrElse(() -> SHA256.digest(file)
69+
.asHex(), null),
70+
deferredHashCode);
71+
rb.addContentCapability(uri, sha256, file.length(), mime);
72+
73+
if (hasIdentity) {
74+
rb.addHashes(file);
75+
}
76+
} catch (Exception e) {
77+
throw Exceptions.duck(e);
78+
}
79+
return rb.build();
80+
});
81+
return resource;
82+
}
83+
84+
private static int hashCode(File file) {
85+
return file.getAbsoluteFile()
86+
.hashCode();
87+
}
88+
89+
static final class CacheKey {
90+
private final Object fileKey;
91+
private final long lastModifiedTime;
92+
private final long size;
93+
final long time;
94+
95+
CacheKey(File file) {
96+
this(file.toPath());
97+
}
98+
99+
CacheKey(Path path) {
100+
BasicFileAttributes attributes;
101+
try {
102+
attributes = Files.getFileAttributeView(path, BasicFileAttributeView.class)
103+
.readAttributes();
104+
} catch (IOException e) {
105+
throw Exceptions.duck(e);
106+
}
107+
if (!attributes.isRegularFile()) {
108+
throw new IllegalArgumentException("File must be a regular file: " + path);
109+
}
110+
Object fileKey = attributes.fileKey();
111+
this.fileKey = (fileKey != null) ? fileKey //
112+
: path.toAbsolutePath(); // Windows FS does not have fileKey
113+
this.lastModifiedTime = attributes.lastModifiedTime()
114+
.toMillis();
115+
this.size = attributes.size();
116+
this.time = System.nanoTime();
117+
}
118+
119+
@Override
120+
public int hashCode() {
121+
return (Objects.hashCode(fileKey) * 31 + Long.hashCode(lastModifiedTime)) * 31 + Long.hashCode(size);
122+
}
123+
124+
@Override
125+
public boolean equals(Object obj) {
126+
if (this == obj) {
127+
return true;
128+
}
129+
if (!(obj instanceof CacheKey)) {
130+
return false;
131+
}
132+
CacheKey other = (CacheKey) obj;
133+
return Objects.equals(fileKey, other.fileKey) && (lastModifiedTime == other.lastModifiedTime)
134+
&& (size == other.size);
135+
}
136+
137+
@Override
138+
public String toString() {
139+
return Objects.toString(fileKey);
140+
}
141+
}
142+
}

Diff for: biz.aQute.bndlib/src/aQute/bnd/osgi/resource/ResourceBuilder.java

+6-21
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package aQute.bnd.osgi.resource;
22

3-
import static aQute.bnd.exceptions.SupplierWithException.asSupplierOrElse;
43
import static aQute.bnd.osgi.Constants.DUPLICATE_MARKER;
54
import static aQute.bnd.osgi.Constants.MIME_TYPE_BUNDLE;
65
import static aQute.bnd.osgi.Constants.MIME_TYPE_JAR;
@@ -61,7 +60,6 @@
6160
import aQute.lib.hierarchy.Hierarchy;
6261
import aQute.lib.hierarchy.NamedNode;
6362
import aQute.lib.zip.JarIndex;
64-
import aQute.libg.cryptography.SHA256;
6563
import aQute.libg.reporter.ReporterAdapter;
6664
import aQute.service.reporter.Reporter;
6765

@@ -730,30 +728,17 @@ public boolean addFile(File file, URI uri) throws Exception {
730728
if (uri == null)
731729
uri = file.toURI();
732730

733-
Domain manifest = Domain.domain(file);
734731
boolean hasIdentity = false;
735-
if (manifest != null) {
736-
hasIdentity = addManifest(manifest);
737-
}
738-
String mime = hasIdentity ? MIME_TYPE_BUNDLE : MIME_TYPE_JAR;
739-
int deferredHashCode = hashCode(file);
740-
DeferredValue<String> sha256 = new DeferredComparableValue<>(String.class,
741-
asSupplierOrElse(() -> SHA256.digest(file)
742-
.asHex(), null),
743-
deferredHashCode);
744-
addContentCapability(uri, sha256, file.length(), mime);
745-
746-
if (hasIdentity) {
747-
addHashes(file);
732+
Resource fileResource = FileResourceCache.getInstance()
733+
.getResource(file, uri);
734+
if (fileResource != null) {
735+
addResource(fileResource);
736+
hasIdentity = !fileResource.getCapabilities(IdentityNamespace.IDENTITY_NAMESPACE)
737+
.isEmpty();
748738
}
749739
return hasIdentity;
750740
}
751741

752-
private static int hashCode(File file) {
753-
return file.getAbsoluteFile()
754-
.hashCode();
755-
}
756-
757742
/**
758743
* Add simple class name hashes to the exported packages. This should not be
759744
* called before any package capabilities are set since we only hash class

Diff for: biz.aQute.repository/test/aQute/bnd/repository/fileset/FileSetRepositoryTest.java

+11
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,17 @@ public void includesMavenArtifacts() throws Exception {
3030
assertThat(repository.list(null)).contains("org.nanohttpd:nanohttpd", "javafx.base")
3131
.doesNotContain("javax.annotation:jsr250-api");
3232

33+
// Do it again which will get file resources from the cache
34+
repository = new FileSetRepository("test2", files);
35+
36+
assertThat(repository.list(null)).contains("org.nanohttpd:nanohttpd", "javafx.base")
37+
.doesNotContain("javax.annotation:jsr250-api");
38+
39+
assertThat(repository.refresh()).isTrue();
40+
41+
assertThat(repository.list(null)).contains("org.nanohttpd:nanohttpd", "javafx.base")
42+
.doesNotContain("javax.annotation:jsr250-api");
43+
3344
}
3445

3546
}

0 commit comments

Comments
 (0)