Re: svn commit: r695898 - /cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/exp/Expression.java

[Andrus Adamchik]

One way would be to make one of the stack objects a factory for  
expressions ... then it can be cached in a non-static way. e.g.  
ObjectContext.getExpressionFactory().fromString(..). This will solve  
(a) and (c), but not (b), which is a fundamental property of the  
Expression class.

But I wonder this - are there common cases when expression parsing is  
a bottleneck? It sure is not superfast, but I've never seen it being  
#1 (or even #5) reason of slow performance.

Andrus



On Sep 16, 2008, at 5:39 PM, Michael Gentry wrote:

So there is no good way of caching parsed expressions, then?

Thanks!


On Tue, Sep 16, 2008 at 10:36 AM, Andrus Adamchik
<[EMAIL PROTECTED]> wrote:
Also (c) (related to my "emeddable" note) - in an environment like  
J2EE,
where there are layers upon layers of explicit and implicit  
containers
between the VM and the application code, having a static object  
storage
buried in a framework is a bad practice - it prevents the container  
layers
from performing their functions of isolating "contexts" for various  
users
(whatever those contexts might be).

Andrus

On Sep 16, 2008, at 5:30 PM, Andrus Adamchik wrote:

-1. Unmanaged static caches are bad, especially in an embeddable  
framework
like Cayenne. This is (a) a potential memory leak and (b) a  
potential
synchronization nightmare (Expression is not immutable and can be  
modified
by the caller).

Andrus


On Sep 16, 2008, at 5:24 PM, [EMAIL PROTECTED] wrote:

Author: mgentry
Date: Tue Sep 16 07:24:11 2008
New Revision: 695898

URL: http://svn.apache.org/viewvc?rev=695898&view=rev
Log:
Updated Expression.fromString to cache the parsed expressions  
instead of
re-parsing them every time.

Modified:

cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/ 
java/org/apache/cayenne/exp/Expression.java

Modified:
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/ 
java/org/apache/cayenne/exp/Expression.java
URL:
http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/exp/Expression.java?rev=695898&r1=695897&r2=695898&view=diff

= 
= 
= 
= 
= 
= 
= 
= 
= 
= 
= 
===================================================================
---
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/ 
java/org/apache/cayenne/exp/Expression.java
(original)
+++
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/ 
java/org/apache/cayenne/exp/Expression.java
Tue Sep 16 07:24:11 2008
@@ -26,6 +26,7 @@
import java.io.StringWriter;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -119,21 +120,36 @@

 protected int type;

+    private static Map<String, Expression> expressionCache =
+        Collections.synchronizedMap(new HashMap<String,
Expression>(32));
+
 /**
  * Parses string, converting it to Expression. If string does not
represent a
  * semantically correct expression, an ExpressionException is  
thrown.
  *
  * @since 1.1
  */
-    // TODO: cache expression strings, since this operation is  
pretty
slow
 public static Expression fromString(String expressionString) {
     if (expressionString == null) {
         throw new NullPointerException("Null expression string.");
     }

-        Reader reader = new StringReader(expressionString);
+        // Retrieve, if possible, the expression from the cache.
+        Expression expression =  
expressionCache.get(expressionString);
+
+        // If the expression was cached, return it immediately  
without
parsing.
+        if (expression != null)
+            return expression;
+
+        // Couldn't find expression, parse, cache, and return it.
     try {
-            return new ExpressionParser(reader).expression();
+            Reader reader = new StringReader(expressionString);
+
+            expression = new  
ExpressionParser(reader).expression();
+
+            expressionCache.put(expressionString, expression);
+
+            return expression;
     }
     catch (ParseException ex) {
         throw new ExpressionException(ex.getMessage(), ex);