Skip to content

Commit b4cc32f

Browse files
committed
Polishing ReadOnlyZipStore for PR integration
- Clean up debugging and temporary logging statements - Rename ensureCacheNew to buildZipIndex for clarity - Replace synchronized HashSet wrappers with ConcurrentHashMap.newKeySet() to improve performance and reduce locking overhead during indexing
1 parent 670a24b commit b4cc32f

1 file changed

Lines changed: 13 additions & 54 deletions

File tree

src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java

Lines changed: 13 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import java.util.Collection;
2323
import java.util.HashSet;
2424
import java.util.Collections;
25-
// Java logging for debugging
2625
import java.util.logging.Logger;
2726
import java.util.logging.Level;
2827

@@ -61,39 +60,7 @@ public ReadOnlyZipStore(@Nonnull String zipPath) {
6160
this(Paths.get(zipPath));
6261
}
6362

64-
/*
65-
private synchronized void ensureCacheOriginal() {
66-
fileIndex = new LinkedHashMap<>();
67-
directoryIndex = new LinkedHashSet<>();
68-
69-
InputStream inputStream = underlyingStore.getInputStream();
70-
if (inputStream == null) {
71-
isCached = true;
72-
return;
73-
}
74-
75-
try (ZipArchiveInputStream zis = new ZipArchiveInputStream(inputStream)) {
76-
ZipArchiveEntry entry;
77-
while ((entry = zis.getNextEntry()) != null) {
78-
String name = normalizeEntryName(entry.getName());
79-
if (entry.isDirectory()) {
80-
directoryIndex.add(name);
81-
} else {
82-
fileIndex.put(name, entry.getSize());
83-
}
84-
}
85-
} catch (IOException e) {
86-
throw StoreException.readFailed(
87-
underlyingStore.toString(), new String[]{},
88-
new IOException("Failed to read ZIP directory from underlying store", e));
89-
}
90-
isCached = true;
91-
}
92-
*/
93-
94-
// Helper method to add parent directories of a file entry to the directory index
95-
96-
63+
// Helper for buildZipIndex to add all parent directories of a given entry to the directory index, ensuring they are present for lookups
9764
private void addParentDirs(String entryName, Set<String> dirIndex) {
9865
int lastSlash = entryName.lastIndexOf('/'); // Find the last '/' in the file name
9966
while (lastSlash > 0) { // Keep going until no more slashes are found
@@ -105,40 +72,41 @@ private void addParentDirs(String entryName, Set<String> dirIndex) {
10572
}
10673
}
10774

75+
// Helper for buildZipIndex to add a file entry to the file index and ensure all its parent directories are indexed as well
10876
private void insertLeafEntry(String entryStrippedPath, long size) {
10977
fileSizeIndex.put(entryStrippedPath, size); // Cache the file size for quick access
11078
int lastSlash = entryStrippedPath.lastIndexOf('/'); // Find the last '/' in the file name returns -1 if not found
11179
if (lastSlash >= 0) {
11280
String name = entryStrippedPath.substring(lastSlash + 1); // Extract the file name after the last slash
11381
String parentDir = entryStrippedPath.substring(0, lastSlash); // Extract the parent directory path without trailing slash
11482
directoryToChildrenFilesIndex.computeIfAbsent(parentDir,
115-
k -> Collections.synchronizedSet(new HashSet<>())).add(name); // Add the file name to the parent directory's set of children files
83+
k -> ConcurrentHashMap.newKeySet()).add(name); // Add the file name to the parent directory's set of children files
11684
insertDirectoryEntry(parentDir); // Recursively ensure all parent directories are added
11785

11886
} else {// no slashes, file is in root directory
11987
directoryToChildrenFilesIndex.computeIfAbsent("",
120-
k -> Collections.synchronizedSet(new HashSet<>())).add(entryStrippedPath);
88+
k -> ConcurrentHashMap.newKeySet()).add(entryStrippedPath);
12189
}
12290
}
12391

92+
// Helper for buildZipIndex to add a directory entry to the directory index and ensure all its parent directories are indexed as well
12493
private void insertDirectoryEntry(String entryStrippedPath) {
12594
int lastSlash = entryStrippedPath.lastIndexOf('/'); // Find the last '/' in the file name returns -1 if not found
12695
if (lastSlash >= 0) {
12796
String name = entryStrippedPath.substring(lastSlash + 1); // Extract the file name after the last slash
12897
String parentDir = entryStrippedPath.substring(0, lastSlash); // Extract the parent directory path without trailing slash
12998
directoryToChildrenDirectoriesIndex.computeIfAbsent(parentDir,
130-
k -> Collections.synchronizedSet(new HashSet<>())).add(name); // Add the directory name to the parent directory's set of children directories
99+
k -> ConcurrentHashMap.newKeySet()).add(name); // Add the file name to the parent directory's set of children files
131100
insertDirectoryEntry(parentDir); // Recursively ensure all parent directories are added
132101

133102
} else {// no slashes, file is in root directory
134103
// No parent directory, just add the file name to the root directory index
135104
directoryToChildrenDirectoriesIndex.computeIfAbsent("",
136-
k -> Collections.synchronizedSet(new HashSet<>())).add(entryStrippedPath);
105+
k -> ConcurrentHashMap.newKeySet()).add(entryStrippedPath);
137106
}
138107
}
139108

140-
141-
private synchronized void ensureCacheNew() {
109+
private synchronized void buildZipIndex() {
142110
// Fast path using ZipFile
143111
try (ZipFile zf = new ZipFile(zipStorePath.toFile())) {
144112
// Optionally pre-size: count entries once (cheap and avoids rehashing on huge zips)
@@ -167,17 +135,16 @@ private synchronized void ensureCacheNew() {
167135
e.getName());
168136
}
169137
directoryToChildrenDirectoriesIndex.computeIfAbsent(entryStrippedPath,
170-
k -> Collections.synchronizedSet(new HashSet<>()));
138+
k -> ConcurrentHashMap.newKeySet());
171139
directoryToChildrenFilesIndex.computeIfAbsent(entryStrippedPath,
172-
k -> Collections.synchronizedSet(new HashSet<>()));
140+
k -> ConcurrentHashMap.newKeySet());
173141
insertDirectoryEntry(entryStrippedPath);
174142
} else {
175143
// Put file size (may be -1 for STORED anomalies, but ZipFile usually knows it)
176144
long size = e.getSize();
177145
insertLeafEntry(entryStrippedPath, size);
178146
}
179147
}
180-
isCached = true;
181148
} catch (IOException e) {
182149
throw StoreException.readFailed(
183150
underlyingStore.toString(), new String[]{},
@@ -189,19 +156,11 @@ private synchronized void ensureCacheNew() {
189156
private synchronized void ensureCache() {
190157
if (isCached) return;
191158
long startTime, endTime;
192-
//System.out.println("START ensureCacheOriginal()");
193-
//startTime = System.currentTimeMillis();
194-
// swap to test the new implementation:
195-
//ensureCacheOriginal(); // or ensureCacheNew()
196-
//endTime = System.currentTimeMillis();
197-
//System.out.println("END ensureCacheOriginal() in " + (endTime - startTime) + " ms");
198-
199-
logger.log(Level.INFO, "Starting ensureCacheNew() for underlying store: {0}", underlyingStore.toString());
200159
startTime = System.currentTimeMillis();
201-
// swap to test the new implementation:
202-
ensureCacheNew(); // or ensureCacheOriginal()
160+
buildZipIndex();
161+
isCached = true;
203162
endTime = System.currentTimeMillis();
204-
logger.log(Level.INFO, "ensureCacheNew() completed in {0} ms", (endTime - startTime));
163+
logger.log(Level.FINE, "Indexed ZIP store {0} with {1} file entries in {2} ms", new Object[]{zipStorePath.toString(), fileSizeIndex.size(), (endTime - startTime)});
205164
}
206165

207166
String resolveKeys(String[] keys) {

0 commit comments

Comments
 (0)