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

From: Michael Gentry (blacknex..mail.com)
Date: Tue Sep 16 2008 - 10:58:52 EDT

  • Next message: Tore Halset (JIRA): "[jira] Commented: (CAY-1108) [PATCH] MySQL/DbMerger compatibility"

    Compared to actual DB latency, it is probably not much of a
    bottleneck. The fact that the Expression can be manipulated after
    being returned is a big issue, though. I know in my use-cases I don't
    manipulate them, but that doesn't mean others don't. I'll revert the
    change and think about it some more, but it might not be worth the
    complexity of trying to cache it.

    Thanks!

    On Tue, Sep 16, 2008 at 10:46 AM, Andrus Adamchik
    <andru..bjectstyle.org> wrote:
    > 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
    >> <andru..bjectstyle.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, 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:59:41 EDT