[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[GitHub] activemq-artemis pull request #2228: ARTEMIS-2017 SelectorParser cache not t...

Github user jbertram commented on a diff in the pull request:

    --- Diff: artemis-selector/src/main/java/org/apache/activemq/artemis/selector/impl/SelectorParser.java ---
    @@ -80,11 +78,15 @@ public static BooleanExpression parse(String sql) throws FilterException {
                    StrictParser parser = new StrictParser(new StringReader(actual));
                    e = parser.JmsSelector();
    -            cache.put(sql, e);
    +            synchronized (cache) {
    --- End diff --
    My guess is that the LRUCache is there mainly because it was used in Apollo and basically the whole selector implementation from Apollo was ported over to the Artemis code-base during the Apache donation process.  It is a simple mitigation against excessive parsing, but perhaps it is premature optimization as well.  I don't see a big problem with pulling it out.  It would simplify the code-base a bit and eliminate the threading problem as well.  I certainly don't see any reason to invest resources into a more robust/elegant/higher-performance cache implementation.