From 51a2d0ed79a32b40ce42609520fec11ff86666d2 Mon Sep 17 00:00:00 2001
From: brinn <brinn>
Date: Mon, 10 Dec 2012 09:31:45 +0000
Subject: [PATCH] [CCS-22/SP-398] Genedata Performance Tuning (improve loading
speed of images through the API) Close the HDF5ContainerReader on cache
expiration. Add a mode for unit tests where caching is disabled and use it
for all use cases with containers as they do not treat the files as
immutable.
SVN: 27889
---
.../openbis/common/hdf5/HDF5Container.java | 9 +++
.../common/hdf5/HDF5ContainerReader.java | 72 +++++++++++++++----
.../common/hdf5/HDF5ContainerTest.java | 7 ++
...icalStructureDuplicatorFileToHDF5Test.java | 7 ++
.../ConcatenatedContentInputStreamTest.java | 8 +++
...faultFileBasedHierarchicalContentTest.java | 7 ++
6 files changed, 96 insertions(+), 14 deletions(-)
diff --git a/openbis-common/source/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5Container.java b/openbis-common/source/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5Container.java
index efc7aea1c73..cf449d9ab0c 100644
--- a/openbis-common/source/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5Container.java
+++ b/openbis-common/source/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5Container.java
@@ -29,6 +29,15 @@ import ch.systemsx.cisd.hdf5.IHDF5SimpleWriter;
*/
public class HDF5Container
{
+ /**
+ * Disable caching for unit testing where the same file name is reused but the file content
+ * changes between tests.
+ */
+ public static void disableCaching()
+ {
+ HDF5ContainerReader.disableCaching();
+ }
+
private final File hdf5Container;
/**
diff --git a/openbis-common/source/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5ContainerReader.java b/openbis-common/source/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5ContainerReader.java
index 01c3e0e0fb5..e1b2c650f79 100644
--- a/openbis-common/source/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5ContainerReader.java
+++ b/openbis-common/source/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5ContainerReader.java
@@ -48,12 +48,15 @@ final class HDF5ContainerReader implements IHDF5ContainerReader
private final IHDF5ArchiveReader archiveReader;
+ private final File archiveFile;
+
private long lastAccessed;
private int referenceCount;
- Reader(IHDF5ArchiveReader archiveReader)
+ Reader(File archiveFile, IHDF5ArchiveReader archiveReader)
{
+ this.archiveFile = archiveFile;
this.archiveReader = archiveReader;
this.lastAccessed = System.currentTimeMillis();
this.referenceCount = 1;
@@ -65,6 +68,11 @@ final class HDF5ContainerReader implements IHDF5ContainerReader
return archiveReader;
}
+ File getArchiveFile()
+ {
+ return archiveFile;
+ }
+
void incCount()
{
++referenceCount;
@@ -75,37 +83,64 @@ final class HDF5ContainerReader implements IHDF5ContainerReader
--referenceCount;
}
+ boolean isUnreferenced()
+ {
+ return referenceCount == 0;
+ }
+
boolean isExpired()
{
return (referenceCount == 0 && (System.currentTimeMillis() - lastAccessed) > RETENTION_TIME_MILLIS);
}
}
-
+
static
{
final Timer t = new Timer("HDF5ContainerReader - Cache Cleaner", true);
- t.schedule(new TimerTask() {
- @Override
- public void run()
+ t.schedule(new TimerTask()
{
- synchronized (fileToReaderMap)
+ @Override
+ public void run()
{
- final Iterator<Reader> it = fileToReaderMap.values().iterator();
- while (it.hasNext())
+ synchronized (fileToReaderMap)
{
- final Reader container = it.next();
- if (container.isExpired())
+ final Iterator<Reader> it = fileToReaderMap.values().iterator();
+ while (it.hasNext())
{
- it.remove();
+ final Reader container = it.next();
+ if (container.isExpired())
+ {
+ container.getArchiveReader().close();
+ it.remove();
+ }
}
}
}
- } } , CACHE_CLEANER_INTERVAL_MILLIS, CACHE_CLEANER_INTERVAL_MILLIS);
+ }, CACHE_CLEANER_INTERVAL_MILLIS, CACHE_CLEANER_INTERVAL_MILLIS);
}
private static final Map<File, Reader> fileToReaderMap =
new HashMap<File, HDF5ContainerReader.Reader>();
-
+
+ private static boolean noCaching = false;
+
+ /**
+ * Disable caching for unit testing where the same file name is reused but the file content
+ * changes between tests.
+ */
+ static void disableCaching()
+ {
+ noCaching = true;
+ synchronized (fileToReaderMap)
+ {
+ for (Reader r : fileToReaderMap.values())
+ {
+ r.getArchiveReader().close();
+ }
+ fileToReaderMap.clear();
+ }
+ }
+
private static Reader openReader(File hdf5Container)
{
Reader entryOrNull;
@@ -114,7 +149,8 @@ final class HDF5ContainerReader implements IHDF5ContainerReader
entryOrNull = fileToReaderMap.get(hdf5Container);
if (entryOrNull == null)
{
- entryOrNull = new Reader(HDF5ArchiverFactory.openForReading(hdf5Container));
+ entryOrNull =
+ new Reader(hdf5Container, HDF5ArchiverFactory.openForReading(hdf5Container));
fileToReaderMap.put(hdf5Container, entryOrNull);
} else
{
@@ -135,6 +171,14 @@ final class HDF5ContainerReader implements IHDF5ContainerReader
public void close()
{
reader.decCount();
+ if (noCaching && reader.isUnreferenced())
+ {
+ synchronized (fileToReaderMap)
+ {
+ reader.getArchiveReader().close();
+ fileToReaderMap.remove(reader.getArchiveFile());
+ }
+ }
}
@Override
diff --git a/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5ContainerTest.java b/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5ContainerTest.java
index 3a4f58f3f04..b893a49caf1 100644
--- a/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5ContainerTest.java
+++ b/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/hdf5/HDF5ContainerTest.java
@@ -22,6 +22,7 @@ import java.io.File;
import java.io.IOException;
import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
import ch.systemsx.cisd.base.tests.AbstractFileSystemTestCase;
@@ -35,6 +36,12 @@ public class HDF5ContainerTest extends AbstractFileSystemTestCase
{
private final static int KB = 1024;
+ @BeforeTest
+ public void disableHDF5ContainerCaching()
+ {
+ HDF5Container.disableCaching();
+ }
+
@Override
@BeforeMethod
public void setUp() throws IOException
diff --git a/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/hdf5/HierarchicalStructureDuplicatorFileToHDF5Test.java b/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/hdf5/HierarchicalStructureDuplicatorFileToHDF5Test.java
index 817986f8698..e5e6c91579d 100644
--- a/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/hdf5/HierarchicalStructureDuplicatorFileToHDF5Test.java
+++ b/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/hdf5/HierarchicalStructureDuplicatorFileToHDF5Test.java
@@ -20,6 +20,7 @@ import java.io.File;
import java.io.IOException;
import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
import ch.systemsx.cisd.base.tests.AbstractFileSystemTestCase;
@@ -34,6 +35,12 @@ public class HierarchicalStructureDuplicatorFileToHDF5Test extends AbstractFileS
private HDF5Container container;
+ @BeforeTest
+ public void disableHDF5ContainerCaching()
+ {
+ HDF5Container.disableCaching();
+ }
+
@BeforeMethod
@Override
public void setUp() throws IOException
diff --git a/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/io/ConcatenatedContentInputStreamTest.java b/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/io/ConcatenatedContentInputStreamTest.java
index f5cdd68f87e..bd908f938ba 100644
--- a/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/io/ConcatenatedContentInputStreamTest.java
+++ b/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/io/ConcatenatedContentInputStreamTest.java
@@ -23,11 +23,13 @@ import java.util.ArrayList;
import java.util.List;
import org.testng.AssertJUnit;
+import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
import ch.systemsx.cisd.base.exceptions.IOExceptionUnchecked;
import ch.systemsx.cisd.base.tests.AbstractFileSystemTestCase;
import ch.systemsx.cisd.common.io.ConcatenatedFileOutputStreamWriter;
+import ch.systemsx.cisd.openbis.common.hdf5.HDF5Container;
import ch.systemsx.cisd.openbis.common.io.hierarchical_content.api.IHierarchicalContentNode;
/**
@@ -37,6 +39,12 @@ import ch.systemsx.cisd.openbis.common.io.hierarchical_content.api.IHierarchical
*/
public class ConcatenatedContentInputStreamTest extends AbstractFileSystemTestCase
{
+ @BeforeTest
+ public void disableHDF5ContainerCaching()
+ {
+ HDF5Container.disableCaching();
+ }
+
@Test
public void testNoFiles() throws IOException
{
diff --git a/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/io/hierarchical_content/DefaultFileBasedHierarchicalContentTest.java b/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/io/hierarchical_content/DefaultFileBasedHierarchicalContentTest.java
index 90e6889e439..144b2b974da 100644
--- a/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/io/hierarchical_content/DefaultFileBasedHierarchicalContentTest.java
+++ b/openbis-common/sourceTest/java/ch/systemsx/cisd/openbis/common/io/hierarchical_content/DefaultFileBasedHierarchicalContentTest.java
@@ -29,6 +29,7 @@ import org.jmock.Expectations;
import org.jmock.Mockery;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
import ch.systemsx.cisd.base.exceptions.IOExceptionUnchecked;
@@ -76,6 +77,12 @@ public class DefaultFileBasedHierarchicalContentTest extends AbstractFileSystemT
private IDelegatedAction onCloseAction;
+ @BeforeTest
+ public void disableHDF5ContainerCaching()
+ {
+ HDF5Container.disableCaching();
+ }
+
@BeforeMethod
public void beforeMethod() throws Exception
{
--
GitLab