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:30:29 EDT

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

    -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:31:07 EDT