From 88e334d7263fa8c2acb70b7cb0afee95e1616536 Mon Sep 17 00:00:00 2001
From: jakubs <jakubs>
Date: Mon, 11 Mar 2013 09:29:36 +0000
Subject: [PATCH] SP-543 BIS-355 prefetch all sample identifiers to speedup
 fetching of feature vectors

SVN: 28572
---
 .../server/DssServiceRpcScreening.java        |  35 ++++++
 ...reVectorLoaderMetadataProviderFactory.java | 109 ++++++++++++------
 .../ScreeningBusinessObjectFactory.java       |  11 +-
 .../shared/imaging/FeatureVectorLoader.java   |   7 ++
 .../server/DssServiceRpcScreeningTest.java    |  24 +++-
 5 files changed, 139 insertions(+), 47 deletions(-)

diff --git a/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreening.java b/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreening.java
index 61603a40255..f251404ab55 100644
--- a/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreening.java
+++ b/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreening.java
@@ -27,6 +27,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -486,6 +487,8 @@ public class DssServiceRpcScreening extends AbstractDssServiceRpc<IDssServiceRpc
                         .createMetadataProviderFromFeatureVectors(getOpenBISService(),
                                 featureDatasets);
 
+        prefetchSampleIdentifiers(featureDatasets, metadataProvider);
+
         for (FeatureVectorDatasetReference dataset : featureDatasets)
         {
             result.add(createFeatureVectorDataset(sessionToken, dataset, codes, metadataProvider));
@@ -493,6 +496,38 @@ public class DssServiceRpcScreening extends AbstractDssServiceRpc<IDssServiceRpc
         return result;
     }
 
+    private void prefetchSampleIdentifiers(List<FeatureVectorDatasetReference> featureDatasets,
+            IMetadataProvider metadataProvider)
+    {
+        List<String> dataSetIds = new LinkedList<String>();
+        for (FeatureVectorDatasetReference featureVectorDatasetReference : featureDatasets)
+        {
+            dataSetIds.add(featureVectorDatasetReference.getDatasetCode());
+            dataSetIds.addAll(metadataProvider
+                    .tryGetContainedDatasets(featureVectorDatasetReference.getDatasetCode()));
+        }
+
+        List<ImgAnalysisDatasetDTO> dataSets =
+                getDAO().listAnalysisDatasetsByPermId(dataSetIds.toArray(new String[0]));
+        long[] containerIds = new long[dataSets.size()];
+        int i = 0;
+        for (ImgAnalysisDatasetDTO adto : dataSets)
+        {
+            containerIds[i++] = adto.getContainerId();
+        }
+
+        List<ImgContainerDTO> containers = getDAO().listContainersByIds(containerIds);
+
+        List<String> samplePermIds = new LinkedList<String>();
+
+        for (ImgContainerDTO container : containers)
+        {
+            samplePermIds.add(container.getPermId());
+        }
+
+        metadataProvider.getSampleIdentifiers(samplePermIds);
+    }
+
     private FeatureVectorDataset createFeatureVectorDataset(String sessionToken,
             FeatureVectorDatasetReference dataset, List<String> featureCodes,
             IMetadataProvider metadataProvider)
diff --git a/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/server/util/FeatureVectorLoaderMetadataProviderFactory.java b/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/server/util/FeatureVectorLoaderMetadataProviderFactory.java
index e7473530085..5581435bc4c 100644
--- a/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/server/util/FeatureVectorLoaderMetadataProviderFactory.java
+++ b/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/server/util/FeatureVectorLoaderMetadataProviderFactory.java
@@ -20,6 +20,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 import ch.systemsx.cisd.openbis.dss.generic.shared.IEncapsulatedOpenBISService;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AbstractExternalData;
@@ -103,22 +104,75 @@ public class FeatureVectorLoaderMetadataProviderFactory
     public static IMetadataProvider createMetadataProvider(
             final IEncapsulatedOpenBISService openBISService)
     {
-        return new IMetadataProvider()
+        return new BasicMetadataProvider(openBISService);
+    }
+
+    private static class BasicMetadataProvider implements IMetadataProvider
+    {
+        private final IEncapsulatedOpenBISService openBISService;
+
+        private final HashMap<String, SampleIdentifier> sampleCache;
+
+        public BasicMetadataProvider(IEncapsulatedOpenBISService openBISService)
+        {
+            this.sampleCache = new HashMap<String, SampleIdentifier>();
+            this.openBISService = openBISService;
+        }
+
+        @Override
+        public void getSampleIdentifiers(List<String> samplePermIds)
+        {
+            Map<String, SampleIdentifier> result =
+                    openBISService.listSampleIdentifiers(samplePermIds);
+            sampleCache.putAll(result);
+        }
+
+        @Override
+        public SampleIdentifier tryGetSampleIdentifier(String samplePermId)
+        {
+            SampleIdentifier result;
+            if (false == sampleCache.containsKey(samplePermId))
             {
+                result = openBISService.tryGetSampleIdentifier(samplePermId);
+                sampleCache.put(samplePermId, result);
+            } else
+            {
+                result = sampleCache.get(samplePermId);
+            }
+            return result;
+        }
+
+        @Override
+        public List<String> tryGetContainedDatasets(String datasetCode)
+        {
+            AbstractExternalData dataSet = openBISService.tryGetDataSet(datasetCode);
+            return getContainedDatasets(dataSet);
+        }
+    }
+
+    private static class CachedDatasetsMetadataProvider extends BasicMetadataProvider
+    {
+        private final HashMap<String, List<String>> containedDataSetsMap;
+
+        public CachedDatasetsMetadataProvider(IEncapsulatedOpenBISService openBISService,
+                HashMap<String, List<String>> containedDataSetsMap)
+        {
+            super(openBISService);
+            this.containedDataSetsMap = containedDataSetsMap;
+        }
 
-                @Override
-                public SampleIdentifier tryGetSampleIdentifier(String samplePermId)
-                {
-                    return openBISService.tryGetSampleIdentifier(samplePermId);
-                }
-
-                @Override
-                public List<String> tryGetContainedDatasets(String datasetCode)
-                {
-                    AbstractExternalData dataSet = openBISService.tryGetDataSet(datasetCode);
-                    return getContainedDatasets(dataSet);
-                }
-            };
+        @Override
+        public List<String> tryGetContainedDatasets(String datasetCode)
+        {
+            if (containedDataSetsMap.containsKey(datasetCode))
+            {
+                return containedDataSetsMap.get(datasetCode);
+            } else
+            {
+                throw new IllegalArgumentException("Data set code unknown to the provider. "
+                        + datasetCode);
+            }
+        }
     }
 
     /**
@@ -135,30 +189,9 @@ public class FeatureVectorLoaderMetadataProviderFactory
     }
 
     private static IMetadataProvider createMetadataProvider(
-            final IEncapsulatedOpenBISService openBISService,
-            final HashMap<String, List<String>> containedDataSetsMap)
+            IEncapsulatedOpenBISService openBISService,
+            HashMap<String, List<String>> containedDataSetsMap)
     {
-
-        return new IMetadataProvider()
-            {
-                @Override
-                public SampleIdentifier tryGetSampleIdentifier(String samplePermId)
-                {
-                    return openBISService.tryGetSampleIdentifier(samplePermId);
-                }
-
-                @Override
-                public List<String> tryGetContainedDatasets(String datasetCode)
-                {
-                    if (containedDataSetsMap.containsKey(datasetCode))
-                    {
-                        return containedDataSetsMap.get(datasetCode);
-                    } else
-                    {
-                        throw new IllegalArgumentException(
-                                "Data set code unknown to the provider. " + datasetCode);
-                    }
-                }
-            };
+        return new CachedDatasetsMetadataProvider(openBISService, containedDataSetsMap);
     }
 }
diff --git a/screening/source/java/ch/systemsx/cisd/openbis/plugin/screening/server/ScreeningBusinessObjectFactory.java b/screening/source/java/ch/systemsx/cisd/openbis/plugin/screening/server/ScreeningBusinessObjectFactory.java
index 7ab010e2704..cf80c599004 100644
--- a/screening/source/java/ch/systemsx/cisd/openbis/plugin/screening/server/ScreeningBusinessObjectFactory.java
+++ b/screening/source/java/ch/systemsx/cisd/openbis/plugin/screening/server/ScreeningBusinessObjectFactory.java
@@ -116,6 +116,12 @@ public final class ScreeningBusinessObjectFactory extends AbstractPluginBusiness
             {
                 private IDatasetLister lister;
 
+                @Override
+                public void getSampleIdentifiers(List<String> samplePermIds)
+                {
+
+                }
+
                 @Override
                 public SampleIdentifier tryGetSampleIdentifier(String samplePermId)
                 {
@@ -125,11 +131,10 @@ public final class ScreeningBusinessObjectFactory extends AbstractPluginBusiness
                 @Override
                 public List<String> tryGetContainedDatasets(String datasetCode)
                 {
-                    List<String> result =
-                            getLister().listContainedCodes(datasetCode);
+                    List<String> result = getLister().listContainedCodes(datasetCode);
                     return result;
                 }
-                
+
                 private IDatasetLister getLister()
                 {
                     if (lister == null)
diff --git a/screening/source/java/ch/systemsx/cisd/openbis/plugin/screening/shared/imaging/FeatureVectorLoader.java b/screening/source/java/ch/systemsx/cisd/openbis/plugin/screening/shared/imaging/FeatureVectorLoader.java
index 0070194a42d..2e726e92e5d 100644
--- a/screening/source/java/ch/systemsx/cisd/openbis/plugin/screening/shared/imaging/FeatureVectorLoader.java
+++ b/screening/source/java/ch/systemsx/cisd/openbis/plugin/screening/shared/imaging/FeatureVectorLoader.java
@@ -84,6 +84,13 @@ public class FeatureVectorLoader
 
     public static interface IMetadataProvider
     {
+        /**
+         * Prefetches in a single call all sample identifiers for given perm ids. Calling this
+         * method before doing calls to <code>getSampleIdentifiers</code> will greatly improve
+         * performance.
+         */
+        void getSampleIdentifiers(List<String> samplePermIds);
+
         /** fetches sample identifier from openBIS */
         SampleIdentifier tryGetSampleIdentifier(String samplePermId);
 
diff --git a/screening/sourceTest/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreeningTest.java b/screening/sourceTest/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreeningTest.java
index f6e57d93178..f0b7bfd9806 100644
--- a/screening/sourceTest/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreeningTest.java
+++ b/screening/sourceTest/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreeningTest.java
@@ -23,8 +23,10 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
@@ -348,6 +350,9 @@ public class DssServiceRpcScreeningTest extends AssertJUnit
         FeatureVectorDatasetReference r1 = createFeatureVectorDatasetReference(DATASET_CODE);
         FeatureVectorDatasetReference r2 = createFeatureVectorDatasetReference(DATASET_CODE2);
 
+        prepareListAnalysisDatasets(1, 2);
+        prepareListContainers(true, 1, 2);
+
         String[][] featureCodesPerDataset = new String[][]
             {
                 { "F1", "F2" } };
@@ -384,7 +389,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit
     private void prepareLoadFeatures(long[] dataSetIDs, String[][] featureCodesPerDataset)
     {
         prepareListAnalysisDatasets(dataSetIDs);
-        prepareListContainers(dataSetIDs);
+        prepareListContainers(false, dataSetIDs);
         prepareGetFeatureDefinitions(dataSetIDs, featureCodesPerDataset);
         prepareGetFeatureVocabularyTerms(dataSetIDs);
         prepareCreateFeatureVectorDataSet(dataSetIDs, featureCodesPerDataset);
@@ -899,12 +904,14 @@ public class DssServiceRpcScreeningTest extends AssertJUnit
             });
     }
 
-    private void prepareListContainers(final long[] dataSetIDs)
+    private void prepareListContainers(final boolean callService, final long... dataSetIDs)
     {
         context.checking(new Expectations()
             {
                 {
                     long[] containerIds = new long[dataSetIDs.length];
+                    String[] sampleIdentifiers = new String[dataSetIDs.length];
+
                     List<ImgContainerDTO> containers = new ArrayList<ImgContainerDTO>();
 
                     for (int i = 0; i < dataSetIDs.length; i++)
@@ -914,11 +921,16 @@ public class DssServiceRpcScreeningTest extends AssertJUnit
                         ImgContainerDTO container = new ImgContainerDTO("12-34", 1, 2, 0);
                         container.setId(containerIds[i]);
                         containers.add(container);
-
-                        one(service).tryGetSampleIdentifier("12-34");
-                        will(returnValue(new SampleIdentifier(new SpaceIdentifier("1", "S"), "P1")));
+                        sampleIdentifiers[i] = "12-34";
                     }
 
+                    if (callService)
+                    {
+                        one(service).listSampleIdentifiers(Arrays.asList(sampleIdentifiers));
+                        Map<String, SampleIdentifier> map = new HashMap<String, SampleIdentifier>();
+                        map.put("12-34", new SampleIdentifier(new SpaceIdentifier("1", "S"), "P1"));
+                        will(returnValue(map));
+                    }
                     one(dao).listContainersByIds(containerIds);
                     will(returnValue(containers));
                 }
@@ -950,7 +962,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit
             });
     }
 
-    private void prepareListAnalysisDatasets(final long[] dataSetIDs)
+    private void prepareListAnalysisDatasets(final long... dataSetIDs)
     {
         context.checking(new Expectations()
             {
-- 
GitLab