From e8f5207b251fb770e68307306104e213e65e20cc Mon Sep 17 00:00:00 2001
From: anttil <anttil>
Date: Tue, 18 Dec 2012 10:41:23 +0000
Subject: [PATCH] BIS-291 / SP-432: Change Evaluator cache from thread-local to
global. Prepopulate cache on creation. Limit cache size by value defined in
service.properties. Limit caching only to managed properties (caching of
dynamic properties and validation scripts removed).
SVN: 27951
---
.../common/jython/evaluator/Evaluator.java | 3 +-
.../jython/evaluator/EvaluatorCache.java | 148 ---------
.../jython/evaluator/EvaluatorCacheTest.java | 135 --------
openbis/resource/dependency-structure.ddf | 2 +-
.../generic/server/JythonEvaluatorPool.java | 210 ++++++++++++
.../calculator/DynamicPropertyCalculator.java | 4 +-
.../EntityValidationCalculator.java | 4 +-
.../managed_property/IAtomicEvaluation.java | 29 ++
.../managed_property/IEvaluationRunner.java | 27 ++
.../ManagedPropertyEvaluator.java | 154 +++++++--
.../ManagedPropertyEvaluatorFactory.java | 11 +-
.../source/java/genericApplicationContext.xml | 5 +
.../server/JythonEvaluatorPoolTest.java | 314 ++++++++++++++++++
.../cisd/openbis/generic/server}/script.py | 0
14 files changed, 724 insertions(+), 322 deletions(-)
delete mode 100644 common/source/java/ch/systemsx/cisd/common/jython/evaluator/EvaluatorCache.java
delete mode 100644 common/sourceTest/java/ch/systemsx/cisd/common/jython/evaluator/EvaluatorCacheTest.java
create mode 100644 openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java
create mode 100644 openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/IAtomicEvaluation.java
create mode 100644 openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/IEvaluationRunner.java
create mode 100644 openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java
rename {common/sourceTest/java/ch/systemsx/cisd/common/jython/evaluator => openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server}/script.py (100%)
diff --git a/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java b/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java
index 6bc76ad1812..bf7a8a6c8cc 100644
--- a/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java
+++ b/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java
@@ -19,6 +19,7 @@ package ch.systemsx.cisd.common.jython.evaluator;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -516,7 +517,7 @@ public final class Evaluator
{
results.add(key.toString());
}
- return results;
+ return Collections.unmodifiableCollection(results);
}
}
diff --git a/common/source/java/ch/systemsx/cisd/common/jython/evaluator/EvaluatorCache.java b/common/source/java/ch/systemsx/cisd/common/jython/evaluator/EvaluatorCache.java
deleted file mode 100644
index 36085987809..00000000000
--- a/common/source/java/ch/systemsx/cisd/common/jython/evaluator/EvaluatorCache.java
+++ /dev/null
@@ -1,148 +0,0 @@
-/*
- * Copyright 2012 ETH Zuerich, CISD
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package ch.systemsx.cisd.common.jython.evaluator;
-
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
-
-/**
- * Cache for Evaluator instances. Enables considerable speedup of python expressions, as the python
- * code needs to be loaded into an interpreter only once (per thread; every thread has a cache of
- * its own).
- *
- * @author anttil
- */
-public class EvaluatorCache
-{
-
- /**
- * ThreadLocal cache of Evaluator instances and their initial states. The Key consists of the
- * script and the expression of the Evaluator.
- */
- private static ThreadLocal<Map<Key, EvaluatorState>> caches =
- new ThreadLocal<Map<Key, EvaluatorState>>()
- {
- @Override
- protected Map<Key, EvaluatorState> initialValue()
- {
- return new HashMap<Key, EvaluatorState>();
- }
- };
-
- /**
- * Returns an Evaluator instance for given script and expression from cache. If the cache
- * contains no such Evaluator, then it is created and added to the cache. All global variables
- * that were set after the Evaluator instance was cached are cleared.
- */
- public static Evaluator getEvaluator(String expression, Class<?> supportFunctionsOrNull,
- String scriptOrNull)
- {
- Key key = new Key(scriptOrNull, expression);
- EvaluatorState state = caches.get().get(key);
- if (state == null)
- {
- Evaluator evaluator = new Evaluator(expression, supportFunctionsOrNull, scriptOrNull);
- Collection<String> globals = evaluator.getGlobalVariables();
- state = new EvaluatorState(evaluator, globals);
- caches.get().put(key, state);
- }
- return state.getCleanInstance();
- }
-
- /**
- * A class containing an Evaluator instance and its initial state.
- *
- * @author anttil
- */
- private static class EvaluatorState
- {
- private final Evaluator evaluator;
-
- private final Collection<String> globals;
-
- public EvaluatorState(Evaluator evaluator, Collection<String> globals)
- {
- this.evaluator = evaluator;
- this.globals = globals;
- }
-
- /**
- * Returns an evaluator instance in its initial state.
- */
- public Evaluator getCleanInstance()
- {
- for (String value : evaluator.getGlobalVariables())
- {
- if (globals.contains(value) == false)
- {
- evaluator.delete(value);
- }
- }
- return this.evaluator;
- }
- }
-
- /**
- * Caching key.
- *
- * @author anttil
- */
- private static class Key
- {
- private final String script;
-
- private final String expression;
-
- public Key(String script, String expression)
- {
- this.script = (script != null ? script : "");
- this.expression = (expression != null ? expression : "");
- }
-
- public String getScript()
- {
- return script;
- }
-
- public String getExpression()
- {
- return expression;
- }
-
- @Override
- public boolean equals(Object o)
- {
- if (o instanceof Key)
- {
- Key key = (Key) o;
- return key.getScript().equals(getScript())
- && key.getExpression().equals(getExpression());
- } else
- {
- return false;
- }
- }
-
- @Override
- public int hashCode()
- {
- return script.hashCode() + 17 * expression.hashCode();
- }
- }
-
-}
diff --git a/common/sourceTest/java/ch/systemsx/cisd/common/jython/evaluator/EvaluatorCacheTest.java b/common/sourceTest/java/ch/systemsx/cisd/common/jython/evaluator/EvaluatorCacheTest.java
deleted file mode 100644
index 6950914d809..00000000000
--- a/common/sourceTest/java/ch/systemsx/cisd/common/jython/evaluator/EvaluatorCacheTest.java
+++ /dev/null
@@ -1,135 +0,0 @@
-/*
- * Copyright 2012 ETH Zuerich, CISD
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package ch.systemsx.cisd.common.jython.evaluator;
-
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.not;
-import static org.hamcrest.CoreMatchers.sameInstance;
-import static org.hamcrest.MatcherAssert.assertThat;
-
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.IOException;
-import java.nio.MappedByteBuffer;
-import java.nio.channels.FileChannel;
-import java.nio.charset.Charset;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
-import java.util.concurrent.LinkedBlockingDeque;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
-
-import org.testng.annotations.Test;
-
-/**
- * @author anttil
- */
-public class EvaluatorCacheTest
-{
-
- private static String DUMMY_SCRIPT = "x=1\ny=2";
-
- private static String DUMMY_SCRIPT_2 = "x=2\ny=1";
-
- private ExecutorService executor = new ThreadPoolExecutor(1, 1, 1000, TimeUnit.HOURS,
- new LinkedBlockingDeque<Runnable>());
-
- @Test
- public void sameThreadGetsAlwaysSameEvaluatorInstanceForSameScriptAndExpression()
- throws Exception
- {
- Evaluator evaluator1 = EvaluatorCache.getEvaluator("", null, DUMMY_SCRIPT);
- Evaluator evaluator2 = EvaluatorCache.getEvaluator("", null, DUMMY_SCRIPT);
-
- assertThat(evaluator1, is(sameInstance(evaluator2)));
- }
-
- @Test
- public void differenceInScriptsCausesDifferentEvaluatorInstancesToBeReturned() throws Exception
- {
- Evaluator evaluator1 = EvaluatorCache.getEvaluator("", null, DUMMY_SCRIPT);
- Evaluator evaluator2 = EvaluatorCache.getEvaluator("", null, DUMMY_SCRIPT_2);
-
- assertThat(evaluator1, is(not(sameInstance(evaluator2))));
- }
-
- @Test
- public void differenceInExpressionsCausesDifferentEvaluatorInstancesToBeReturned()
- throws Exception
- {
- Evaluator evaluator1 = EvaluatorCache.getEvaluator("get_something()", null, DUMMY_SCRIPT);
- Evaluator evaluator2 =
- EvaluatorCache.getEvaluator("get_something_else()", null, DUMMY_SCRIPT);
-
- assertThat(evaluator1, is(not(sameInstance(evaluator2))));
- }
-
- @Test
- public void differentThreadsGetDifferentEvaluatorsForSameScriptAndSameExpressions()
- throws Exception
- {
- Evaluator evaluator1 = EvaluatorCache.getEvaluator("", null, DUMMY_SCRIPT);
- Future<Evaluator> evaluator2 = executor.submit(new Callable<Evaluator>()
- {
- @Override
- public Evaluator call() throws Exception
- {
- return EvaluatorCache.getEvaluator("", null, DUMMY_SCRIPT);
- }
- });
-
- assertThat(evaluator1, is(not(sameInstance(evaluator2.get()))));
- }
-
- @Test
- public void globalVariablesSetInPreviousRunAreClearedBeforeHandingOutAnEvaluatorInstance()
- throws Exception
- {
- Evaluator evaluator =
- EvaluatorCache
- .getEvaluator("return_x_if_defined_else_0()", null, readFile("script.py"));
- evaluator.evalFunction("set_x", "3");
-
- assertThat(evaluator.evalAsString(), is("3"));
- evaluator =
- EvaluatorCache
- .getEvaluator("return_x_if_defined_else_0()", null, readFile("script.py"));
- assertThat(evaluator.evalAsString(), is("0"));
- }
-
- private static String readFile(String name)
- {
- String filePath = "sourceTest/java/ch/systemsx/cisd/common/jython/evaluator/" + name;
- try
- {
- FileInputStream stream = new FileInputStream(new File(filePath));
- try
- {
- FileChannel fc = stream.getChannel();
- MappedByteBuffer bb = fc.map(FileChannel.MapMode.READ_ONLY, 0, fc.size());
- return Charset.defaultCharset().decode(bb).toString();
- } finally
- {
- stream.close();
- }
- } catch (IOException e)
- {
- throw new IllegalArgumentException("Could not read file " + filePath);
- }
- }
-}
diff --git a/openbis/resource/dependency-structure.ddf b/openbis/resource/dependency-structure.ddf
index f9de9f7157c..55a502a32d3 100644
--- a/openbis/resource/dependency-structure.ddf
+++ b/openbis/resource/dependency-structure.ddf
@@ -10,7 +10,7 @@
[generic.client] = ${generic}.client.*
[generic.server] = ${generic}.server.*
-[generic.shared] = ${generic}.shared.*
+[generic.shared] = ${generic}.shared.* excluding ch.systemsx.cisd.openbis.generic.shared.managed_property.ManagedPropertyEvaluatorFactory
check sets [generic.client] [generic.server] [generic.shared]
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java
new file mode 100644
index 00000000000..f3da537e7c7
--- /dev/null
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java
@@ -0,0 +1,210 @@
+/*
+ * Copyright 2012 ETH Zuerich, CISD
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ch.systemsx.cisd.openbis.generic.server;
+
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.apache.log4j.Logger;
+
+import ch.systemsx.cisd.common.jython.evaluator.Evaluator;
+import ch.systemsx.cisd.common.jython.evaluator.EvaluatorException;
+import ch.systemsx.cisd.common.logging.LogCategory;
+import ch.systemsx.cisd.common.logging.LogFactory;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ScriptType;
+import ch.systemsx.cisd.openbis.generic.shared.dto.ScriptPE;
+import ch.systemsx.cisd.openbis.generic.shared.managed_property.IAtomicEvaluation;
+import ch.systemsx.cisd.openbis.generic.shared.managed_property.IEvaluationRunner;
+import ch.systemsx.cisd.openbis.generic.shared.managed_property.ManagedPropertyFunctions;
+
+/**
+ * Pool of Jython Evaluators for managed properties. When the pool is created, it is filled with new
+ * Evaluator instances for every managed property script. Enables thread-safe execution of calls to
+ * python functions defined in these scripts.
+ *
+ * @author anttil
+ */
+public class JythonEvaluatorPool
+{
+ private static Logger log = LogFactory.getLogger(LogCategory.OPERATION,
+ JythonEvaluatorPool.class);
+
+ public static int DEFAULT_POOL_SIZE = 100;
+
+ public static JythonEvaluatorPool INSTANCE;
+
+ private Map<String, EvaluatorState> cache;
+
+ private Lock cacheLock;
+
+ public JythonEvaluatorPool(IDAOFactory daoFactory, String poolSize)
+ {
+ this(daoFactory, createCache(poolSize));
+ }
+
+ public JythonEvaluatorPool(IDAOFactory daoFactory, Map<String, EvaluatorState> cache)
+ {
+ this.cache = cache;
+ this.cacheLock = new ReentrantLock();
+
+ for (ScriptPE script : daoFactory.getScriptDAO().listEntities(ScriptType.MANAGED_PROPERTY,
+ null))
+ {
+ try
+ {
+ cache.put(script.getScript(),
+ new EvaluatorState(createEvaluatorFor(script.getScript())));
+ } catch (EvaluatorException e)
+ {
+ log.warn("Could not create evaluator for script " + script.getName(), e);
+ }
+ }
+ log.info("Initialization successful with " + cache.size() + " evaluators");
+ INSTANCE = this;
+ }
+
+ /**
+ * Return runner that can be used to evaluate python functions using evaluator in the pool.
+ */
+ public IEvaluationRunner getRunner(final String script)
+ {
+ return new IEvaluationRunner()
+ {
+ @Override
+ public <T> T evaluate(IAtomicEvaluation<T> evaluation)
+ {
+ return JythonEvaluatorPool.this.evaluate(script, evaluation);
+ }
+ };
+ }
+
+ /**
+ * Evaluate python functions from a script using an Evaluator instance in the pool. If the
+ * Evaluator instance does not exist, create it. Give access to the instance for only one thread
+ * at a time.
+ */
+ private <T> T evaluate(String script, IAtomicEvaluation<T> evaluation)
+ {
+ EvaluatorState state = cache.get(script);
+ if (state == null)
+ {
+ cacheLock.lock();
+ try
+ {
+ state = cache.get(script);
+ if (state == null)
+ {
+ state = new EvaluatorState(createEvaluatorFor(script));
+ cache.put(script, state);
+ }
+ } finally
+ {
+ cacheLock.unlock();
+ }
+ }
+
+ Lock lock = state.getLock();
+ lock.lock();
+ try
+ {
+ return evaluation.evaluate(state.getCleanInstance());
+ } finally
+ {
+ lock.unlock();
+ }
+ }
+
+ private Evaluator createEvaluatorFor(String script)
+ {
+ return new Evaluator("", ManagedPropertyFunctions.class, script);
+ }
+
+ /**
+ * The pooled object. Contains the Evaluator and its initial state to which it is always
+ * returned to when a new evaluation is started.
+ */
+ public static class EvaluatorState
+ {
+ private final Evaluator evaluator;
+
+ private final Collection<String> globals;
+
+ private final Lock lock;
+
+ public EvaluatorState(Evaluator evaluator)
+ {
+ this.evaluator = evaluator;
+ this.globals = evaluator.getGlobalVariables();
+ this.lock = new ReentrantLock();
+ }
+
+ /**
+ * Returns an evaluator instance in its initial state.
+ */
+ public synchronized Evaluator getCleanInstance()
+ {
+ for (String value : evaluator.getGlobalVariables())
+ {
+ if (globals.contains(value) == false)
+ {
+ evaluator.delete(value);
+ }
+ }
+ return this.evaluator;
+ }
+
+ public Lock getLock()
+ {
+ return lock;
+ }
+ }
+
+ public static Map<String, EvaluatorState> createCache(String poolSize)
+ {
+ int size = DEFAULT_POOL_SIZE;
+ try
+ {
+ int number = Integer.parseInt(poolSize);
+ if (number > 0)
+ {
+ size = number;
+ }
+ } catch (NumberFormatException e)
+ {
+ }
+
+ return createCache(size);
+ }
+
+ public static Map<String, EvaluatorState> createCache(final int poolSize)
+ {
+ return new LinkedHashMap<String, EvaluatorState>(16, 0.75f, true)
+ {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected boolean removeEldestEntry(Map.Entry<String, EvaluatorState> eldest)
+ {
+ return size() > poolSize;
+ }
+ };
+ }
+}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/DynamicPropertyCalculator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/DynamicPropertyCalculator.java
index 7aec427b39c..70d095c2f05 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/DynamicPropertyCalculator.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/DynamicPropertyCalculator.java
@@ -17,7 +17,6 @@
package ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator;
import ch.systemsx.cisd.common.jython.evaluator.Evaluator;
-import ch.systemsx.cisd.common.jython.evaluator.EvaluatorCache;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator.api.IEntityAdaptor;
import ch.systemsx.cisd.openbis.generic.shared.calculator.AbstractCalculator;
@@ -55,8 +54,7 @@ public class DynamicPropertyCalculator extends AbstractCalculator
initialScript += expression;
calculatedExpression = INVOKE_CALCULATE_EXPR;
}
- return new DynamicPropertyCalculator(EvaluatorCache.getEvaluator(calculatedExpression,
- Math.class,
+ return new DynamicPropertyCalculator(new Evaluator(calculatedExpression, Math.class,
initialScript));
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/EntityValidationCalculator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/EntityValidationCalculator.java
index 9867d1902d9..d94f4957278 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/EntityValidationCalculator.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/EntityValidationCalculator.java
@@ -17,7 +17,6 @@
package ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator;
import ch.systemsx.cisd.common.jython.evaluator.Evaluator;
-import ch.systemsx.cisd.common.jython.evaluator.EvaluatorCache;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator.api.IEntityAdaptor;
import ch.systemsx.cisd.openbis.generic.shared.calculator.AbstractCalculator;
@@ -58,8 +57,7 @@ public class EntityValidationCalculator extends AbstractCalculator
initialScript += expression;
String calculatedExpression = INVOKE_CALCULATE_EXPR;
- return new EntityValidationCalculator(EvaluatorCache.getEvaluator(calculatedExpression,
- Math.class,
+ return new EntityValidationCalculator(new Evaluator(calculatedExpression, Math.class,
initialScript), validationRequestedDelegate);
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/IAtomicEvaluation.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/IAtomicEvaluation.java
new file mode 100644
index 00000000000..31599d06c19
--- /dev/null
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/IAtomicEvaluation.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2012 ETH Zuerich, CISD
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ch.systemsx.cisd.openbis.generic.shared.managed_property;
+
+import ch.systemsx.cisd.common.jython.evaluator.Evaluator;
+
+/**
+ * Evaluation of python code loaded in the interpreter of an Evaluator.
+ *
+ * @author anttil
+ */
+public interface IAtomicEvaluation<T>
+{
+ public T evaluate(Evaluator evaluator);
+}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/IEvaluationRunner.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/IEvaluationRunner.java
new file mode 100644
index 00000000000..0ff971f17b5
--- /dev/null
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/IEvaluationRunner.java
@@ -0,0 +1,27 @@
+/*
+ * Copyright 2012 ETH Zuerich, CISD
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ch.systemsx.cisd.openbis.generic.shared.managed_property;
+
+/**
+ * Any object that is able to execute evaluations defined with IAtomicEvaluation.
+ *
+ * @author anttil
+ */
+public interface IEvaluationRunner
+{
+ public <T> T evaluate(IAtomicEvaluation<T> evaluation);
+}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/ManagedPropertyEvaluator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/ManagedPropertyEvaluator.java
index 070338b973b..c11ae92693b 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/ManagedPropertyEvaluator.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/ManagedPropertyEvaluator.java
@@ -25,7 +25,6 @@ import java.util.Set;
import org.apache.log4j.Logger;
import ch.systemsx.cisd.common.jython.evaluator.Evaluator;
-import ch.systemsx.cisd.common.jython.evaluator.EvaluatorCache;
import ch.systemsx.cisd.common.jython.evaluator.EvaluatorException;
import ch.systemsx.cisd.common.logging.LogCategory;
import ch.systemsx.cisd.common.logging.LogFactory;
@@ -107,7 +106,7 @@ public class ManagedPropertyEvaluator
private static final String PERSON_VARIABLE_NAME = "person";
- private final Evaluator evaluator;
+ private final IEvaluationRunner runner;
private final List<String> columnNames;
@@ -117,16 +116,68 @@ public class ManagedPropertyEvaluator
private List<IManagedInputWidgetDescription> inputWidgetDescriptions;
- public ManagedPropertyEvaluator(String scriptExpression)
+ public ManagedPropertyEvaluator(final String scriptExpression)
{
- evaluator =
- EvaluatorCache.getEvaluator("", ManagedPropertyFunctions.class, scriptExpression);
- updateFromBatchFunctionDefined = evaluator.hasFunction(UPDATE_FROM_BATCH_INPUT_FUNCTION);
+ this(new IEvaluationRunner()
+ {
+ private Evaluator evaluator;
+ {
+ this.evaluator =
+ new Evaluator("", ManagedPropertyFunctions.class, scriptExpression);
+ }
+
+ @Override
+ public <T> T evaluate(IAtomicEvaluation<T> evaluation)
+ {
+ return evaluation.evaluate(evaluator);
+ }
+
+ });
+ }
+
+ public ManagedPropertyEvaluator(IEvaluationRunner runner)
+ {
+ this.runner = runner;
+ updateFromBatchFunctionDefined =
+ runner.evaluate(new IAtomicEvaluation<Boolean>()
+ {
+ @Override
+ public Boolean evaluate(Evaluator evaluator)
+ {
+ return evaluator.hasFunction(UPDATE_FROM_BATCH_INPUT_FUNCTION);
+ }
+ });
+
updateFromRegistrationFormFunctionDefined =
- evaluator.hasFunction(UPDATE_FROM_REGISTRATION_FORM_FUNCTION);
+ runner.evaluate(new IAtomicEvaluation<Boolean>()
+ {
+ @Override
+ public Boolean evaluate(Evaluator evaluator)
+ {
+ return evaluator.hasFunction(UPDATE_FROM_REGISTRATION_FORM_FUNCTION);
+ }
+ });
+
boolean batchColumnNamesFunctionDefined =
- evaluator.hasFunction(BATCH_COLUMN_NAMES_FUNCTION);
- boolean inputWidgetsFunctionDefined = evaluator.hasFunction(INPUT_WIDGETS_FUNCTION);
+ runner.evaluate(new IAtomicEvaluation<Boolean>()
+ {
+ @Override
+ public Boolean evaluate(Evaluator evaluator)
+ {
+ return evaluator.hasFunction(BATCH_COLUMN_NAMES_FUNCTION);
+ }
+ });
+
+ boolean inputWidgetsFunctionDefined =
+ runner.evaluate(new IAtomicEvaluation<Boolean>()
+ {
+ @Override
+ public Boolean evaluate(Evaluator evaluator)
+ {
+ return evaluator.hasFunction(INPUT_WIDGETS_FUNCTION);
+ }
+ });
+
checkCombinationsOfDefinedFunctions(batchColumnNamesFunctionDefined,
inputWidgetsFunctionDefined);
columnNames = new ArrayList<String>();
@@ -209,9 +260,17 @@ public class ManagedPropertyEvaluator
}
}
- private List<?> evalFunction(String functionName)
+ private List<?> evalFunction(final String functionName)
{
- Object result = evaluator.evalFunction(functionName);
+ Object result = runner.evaluate(new IAtomicEvaluation<Object>()
+ {
+ @Override
+ public Object evaluate(Evaluator evaluator)
+ {
+ return evaluator.evalFunction(functionName);
+ }
+ });
+
if (result instanceof List == false)
{
throw new EvaluatorException("Function '" + functionName
@@ -221,7 +280,8 @@ public class ManagedPropertyEvaluator
return (List<?>) result;
}
- public void configureUI(IManagedProperty managedProperty, EntityPropertyPE entityPropertyPE)
+ public void configureUI(final IManagedProperty managedProperty,
+ final EntityPropertyPE entityPropertyPE)
{
if (operationLog.isDebugEnabled())
{
@@ -229,13 +289,21 @@ public class ManagedPropertyEvaluator
managedProperty));
}
- evaluator.set(PROPERTY_VARIABLE_NAME, managedProperty);
- evaluator.set(PROPERTY_PE_VARIABLE_NAME, entityPropertyPE);
- evaluator.evalFunction(CONFIGURE_UI_FUNCTION);
+ runner.evaluate(new IAtomicEvaluation<Void>()
+ {
+ @Override
+ public Void evaluate(Evaluator evaluator)
+ {
+ evaluator.set(PROPERTY_VARIABLE_NAME, managedProperty);
+ evaluator.set(PROPERTY_PE_VARIABLE_NAME, entityPropertyPE);
+ evaluator.evalFunction(CONFIGURE_UI_FUNCTION);
+ return null;
+ }
+ });
}
- public void updateFromUI(IManagedProperty managedProperty, IPerson person,
- IManagedUiAction action)
+ public void updateFromUI(final IManagedProperty managedProperty, final IPerson person,
+ final IManagedUiAction action)
{
if (operationLog.isDebugEnabled())
{
@@ -243,9 +311,17 @@ public class ManagedPropertyEvaluator
managedProperty));
}
- evaluator.set(PROPERTY_VARIABLE_NAME, managedProperty);
- evaluator.set(PERSON_VARIABLE_NAME, person);
- evaluator.evalFunction(UPDATE_FROM_UI_FUNCTION, action);
+ runner.evaluate(new IAtomicEvaluation<Void>()
+ {
+ @Override
+ public Void evaluate(Evaluator evaluator)
+ {
+ evaluator.set(PROPERTY_VARIABLE_NAME, managedProperty);
+ evaluator.set(PERSON_VARIABLE_NAME, person);
+ evaluator.evalFunction(UPDATE_FROM_UI_FUNCTION, action);
+ return null;
+ }
+ });
}
public List<String> getBatchColumnNames()
@@ -258,8 +334,8 @@ public class ManagedPropertyEvaluator
return inputWidgetDescriptions;
}
- public void updateFromBatchInput(IManagedProperty managedProperty, IPerson person,
- Map<String, String> bindings)
+ public void updateFromBatchInput(final IManagedProperty managedProperty, final IPerson person,
+ final Map<String, String> bindings)
{
if (updateFromBatchFunctionDefined == false)
{
@@ -269,20 +345,38 @@ public class ManagedPropertyEvaluator
}
} else
{
- evaluator.set(PROPERTY_VARIABLE_NAME, managedProperty);
- evaluator.set(PERSON_VARIABLE_NAME, person);
- evaluator.evalFunction(UPDATE_FROM_BATCH_INPUT_FUNCTION, bindings);
+ runner.evaluate(new IAtomicEvaluation<Void>()
+ {
+ @Override
+ public Void evaluate(Evaluator evaluator)
+ {
+ evaluator.set(PROPERTY_VARIABLE_NAME, managedProperty);
+ evaluator.set(PERSON_VARIABLE_NAME, person);
+ evaluator.evalFunction(UPDATE_FROM_BATCH_INPUT_FUNCTION, bindings);
+ return null;
+ }
+ });
}
}
- public void updateFromRegistrationForm(IManagedProperty managedProperty, IPerson person,
- List<Map<String, String>> bindings)
+ public void updateFromRegistrationForm(final IManagedProperty managedProperty,
+ final IPerson person,
+ final List<Map<String, String>> bindings)
{
if (updateFromRegistrationFormFunctionDefined)
{
- evaluator.set(PROPERTY_VARIABLE_NAME, managedProperty);
- evaluator.set(PERSON_VARIABLE_NAME, person);
- evaluator.evalFunction(UPDATE_FROM_REGISTRATION_FORM_FUNCTION, bindings);
+ runner.evaluate(new IAtomicEvaluation<Void>()
+ {
+ @Override
+ public Void evaluate(Evaluator evaluator)
+ {
+ evaluator.set(PROPERTY_VARIABLE_NAME, managedProperty);
+ evaluator.set(PERSON_VARIABLE_NAME, person);
+ evaluator.evalFunction(UPDATE_FROM_REGISTRATION_FORM_FUNCTION,
+ bindings);
+ return null;
+ }
+ });
}
}
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/ManagedPropertyEvaluatorFactory.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/ManagedPropertyEvaluatorFactory.java
index 666a73ba0dd..566ffab4e8a 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/ManagedPropertyEvaluatorFactory.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/managed_property/ManagedPropertyEvaluatorFactory.java
@@ -16,6 +16,7 @@
package ch.systemsx.cisd.openbis.generic.shared.managed_property;
+import ch.systemsx.cisd.openbis.generic.server.JythonEvaluatorPool;
/**
* Factory for creating managed property evaluators. (Could do some caching or other cleverness.)
@@ -24,8 +25,16 @@ package ch.systemsx.cisd.openbis.generic.shared.managed_property;
*/
public class ManagedPropertyEvaluatorFactory
{
+
public static ManagedPropertyEvaluator createManagedPropertyEvaluator(String scriptExpression)
{
- return new ManagedPropertyEvaluator(scriptExpression);
+ if (JythonEvaluatorPool.INSTANCE != null)
+ {
+ return new ManagedPropertyEvaluator(JythonEvaluatorPool.INSTANCE
+ .getRunner(scriptExpression));
+ } else
+ {
+ return new ManagedPropertyEvaluator(scriptExpression);
+ }
}
}
diff --git a/openbis/source/java/genericApplicationContext.xml b/openbis/source/java/genericApplicationContext.xml
index bf4e1fb663c..1ca5fc4f561 100644
--- a/openbis/source/java/genericApplicationContext.xml
+++ b/openbis/source/java/genericApplicationContext.xml
@@ -213,6 +213,11 @@
<bean id="entity-operation-checker" class="ch.systemsx.cisd.openbis.generic.server.business.EntityOperationChecker"/>
+ <bean id="jython-evaluator-pool"
+ class="ch.systemsx.cisd.openbis.generic.server.JythonEvaluatorPool">
+ <constructor-arg ref="dao-factory" />
+ <constructor-arg value="${jython-evaluator-pool-size}" />
+ </bean>
<!--
// Transaction
-->
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java
new file mode 100644
index 00000000000..d3fa266895a
--- /dev/null
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java
@@ -0,0 +1,314 @@
+/*
+ * Copyright 2012 ETH Zuerich, CISD
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ch.systemsx.cisd.openbis.generic.server;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.MappedByteBuffer;
+import java.nio.channels.FileChannel;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.LinkedBlockingDeque;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.hamcrest.TypeSafeMatcher;
+import org.jmock.Expectations;
+import org.jmock.Mockery;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import ch.systemsx.cisd.common.jython.evaluator.Evaluator;
+import ch.systemsx.cisd.openbis.generic.server.JythonEvaluatorPool.EvaluatorState;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.IScriptDAO;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ScriptType;
+import ch.systemsx.cisd.openbis.generic.shared.dto.ScriptPE;
+import ch.systemsx.cisd.openbis.generic.shared.managed_property.IAtomicEvaluation;
+
+/**
+ * @author anttil
+ */
+public class JythonEvaluatorPoolTest
+{
+ private JythonEvaluatorPool pool;
+
+ private Map<String, EvaluatorState> cache;
+
+ private static int POOL_SIZE = 10;
+
+ @Test
+ public void sameEvaluatorIsAlwaysUsedForSameScript() throws Exception
+ {
+ long start = System.currentTimeMillis();
+ while (secondsSpentSince(start) < 2)
+ {
+ List<Evaluator> resultList = executeParallel(getEvaluatorInUse());
+ assertThat(resultList, isFilledWithSameInstanceOf(Evaluator.class));
+ }
+ }
+
+ @Test
+ public void globalVariablesSetInPreviousRunsAreClearedBeforeNewEvaluation() throws Exception
+ {
+ long start = System.currentTimeMillis();
+ while (secondsSpentSince(start) < 2)
+ {
+ List<Integer> results =
+ executeParallel(returnZeroIfAVariableIsNotSetThenSetItToSomeValue());
+
+ Set<Integer> set = new HashSet<Integer>(results);
+ assertThat(set.size(), is(1));
+ assertThat(set.contains(new Integer(0)), is(true));
+ }
+ }
+
+ @Test
+ public void poolSizeLimitIsNotExceeded() throws Exception
+ {
+ List<String> scripts = new ArrayList<String>();
+ for (int i = 0; i < POOL_SIZE * 2; i++)
+ {
+ scripts.add(createRandomScript());
+ }
+
+ for (String script : scripts)
+ {
+ pool.getRunner(script).evaluate(dummyEvaluation());
+ }
+
+ assertThat(cache.size(), is(POOL_SIZE));
+ }
+
+ @Test
+ public void whenCacheSizeLimitIsExceededLeastAccessedElementIsEvicted() throws Exception
+ {
+ List<String> scripts = new ArrayList<String>();
+ for (int i = 0; i < POOL_SIZE + 1; i++)
+ {
+ scripts.add(createRandomScript());
+ }
+
+ for (String script : scripts)
+ {
+ pool.getRunner(script).evaluate(dummyEvaluation());
+ pool.getRunner(scripts.get(0)).evaluate(dummyEvaluation());
+ }
+
+ assertThat(cache.containsKey(scripts.get(0)), is(true));
+ assertThat(cache.containsKey(scripts.get(1)), is(false));
+ }
+
+ private String createRandomScript()
+ {
+ return "x = '" + UUID.randomUUID().toString() + "'";
+ }
+
+ private long secondsSpentSince(long time)
+ {
+ return (System.currentTimeMillis() - time) / 1000;
+ }
+
+ private ExecutorService executor = new ThreadPoolExecutor(10, 50, 1000, TimeUnit.HOURS,
+ new LinkedBlockingDeque<Runnable>());
+
+ private <T> List<T> executeParallel(Callable<T> task) throws Exception
+ {
+ List<Callable<T>> tasks = new ArrayList<Callable<T>>();
+ for (int i = 0; i < 100; i++)
+ {
+ tasks.add(task);
+ }
+ List<Future<T>> futures = executor.invokeAll(tasks);
+ return getFutureValues(futures);
+ }
+
+ private <T> List<T> getFutureValues(List<Future<T>> futures) throws InterruptedException,
+ ExecutionException
+ {
+ List<T> results = new ArrayList<T>();
+ for (Future<T> future : futures)
+ {
+ results.add(future.get());
+ }
+ return results;
+ }
+
+ private Callable<Evaluator> getEvaluatorInUse()
+ {
+ final String script =
+ "def something():\n\treturn '" + UUID.randomUUID().toString() + "'";
+
+ return new Callable<Evaluator>()
+ {
+ @Override
+ public Evaluator call() throws Exception
+ {
+ return pool.getRunner(script).evaluate(
+ new IAtomicEvaluation<Evaluator>()
+ {
+ @Override
+ public Evaluator evaluate(Evaluator evaluator)
+ {
+ return evaluator;
+ }
+ }
+ );
+ }
+ };
+ }
+
+ private Callable<Integer> returnZeroIfAVariableIsNotSetThenSetItToSomeValue()
+ {
+ final String script = readFile("script.py");
+
+ return new Callable<Integer>()
+ {
+ @Override
+ public Integer call() throws Exception
+ {
+ return pool.getRunner(script).evaluate(
+ new IAtomicEvaluation<Integer>()
+ {
+ @Override
+ public Integer evaluate(Evaluator evaluator)
+ {
+ Integer x =
+ (Integer) evaluator
+ .evalFunction("return_x_if_defined_else_0");
+ evaluator.evalFunction("set_x", 3);
+ return x;
+ }
+ }
+ );
+ }
+ };
+ }
+
+ private IAtomicEvaluation<Void> dummyEvaluation()
+ {
+ return new IAtomicEvaluation<Void>()
+ {
+ @Override
+ public Void evaluate(Evaluator evaluator)
+ {
+ return null;
+ }
+ };
+ }
+
+ private String readFile(String name)
+ {
+ String filePath = "sourceTest/java/ch/systemsx/cisd/openbis/generic/server/" + name;
+ try
+ {
+ FileInputStream stream = new FileInputStream(new File(filePath));
+ try
+ {
+ FileChannel fc = stream.getChannel();
+ MappedByteBuffer bb = fc.map(FileChannel.MapMode.READ_ONLY, 0, fc.size());
+ return Charset.defaultCharset().decode(bb).toString();
+ } finally
+ {
+ stream.close();
+ }
+ } catch (IOException e)
+ {
+ throw new IllegalArgumentException("Could not read file " + filePath);
+ }
+ }
+
+ private <T> Matcher<Collection<T>> isFilledWithSameInstanceOf(Class<T> clazz)
+ {
+ return new TypeSafeMatcher<Collection<T>>()
+ {
+ @Override
+ public void describeTo(Description description)
+ {
+ description.appendText("A collection with only one instance in it");
+ }
+
+ @Override
+ public boolean matchesSafely(Collection<T> collection)
+ {
+ if (collection.isEmpty())
+ {
+ return false;
+ }
+ if (collection.size() == 1)
+ {
+ return true;
+ }
+
+ T t = null;
+ for (T element : collection)
+ {
+ if (t == null)
+ {
+ t = element;
+ } else if (t == element)
+ {
+ continue;
+ } else
+ {
+ return false;
+ }
+ }
+ return true;
+ }
+ };
+ }
+
+ @BeforeMethod
+ public void fixture()
+ {
+ Mockery context = new Mockery();
+ final IDAOFactory daoFactory = context.mock(IDAOFactory.class);
+ final IScriptDAO scriptDao = context.mock(IScriptDAO.class);
+ context.checking(new Expectations()
+ {
+ {
+ List<ScriptPE> scripts = new ArrayList<ScriptPE>();
+
+ allowing(daoFactory).getScriptDAO();
+ will(returnValue(scriptDao));
+
+ allowing(scriptDao).listEntities(ScriptType.MANAGED_PROPERTY, null);
+ will(returnValue(scripts));
+ }
+ });
+
+ cache = JythonEvaluatorPool.createCache(Integer.toString(POOL_SIZE));
+ pool = new JythonEvaluatorPool(daoFactory, cache);
+ }
+}
diff --git a/common/sourceTest/java/ch/systemsx/cisd/common/jython/evaluator/script.py b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/script.py
similarity index 100%
rename from common/sourceTest/java/ch/systemsx/cisd/common/jython/evaluator/script.py
rename to openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/script.py
--
GitLab