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

From: Andrus Adamchik (andru..bjectstyle.org)
Date: Tue Sep 16 2008 - 10:36:25 EDT

  • Next message: Michael Gentry: "Re: svn commit: r695898 - /cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/exp/Expression.java"

    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, mgentr..pache.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.
    >> *
    >> *..ince 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);
    >>
    >
    >



    This archive was generated by hypermail 2.0.0 : Tue Sep 16 2008 - 10:36:58 EDT