cayenne-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrus Adamchik <and...@objectstyle.org>
Subject Re: svn commit: r695898 - /cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/exp/Expression.java
Date Tue, 16 Sep 2008 14:46:48 GMT
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
> <andrus@objectstyle.org> 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, mgentry@apache.org 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);
>>>>
>>>
>>>
>>
>>
>


Mime
View raw message