[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 michaelandrepearce 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 --
    I would have two points here then:
    If perf here isn't critical, why even bother then having an LRUCache? Removing it would remove any threading issues right.
    If it is critical:
    Then a more elegant option than sync blocks would look to update LRUCache to extend ConcurrentLinkedHashMap (https://github.com/ben-manes/concurrentlinkedhashmap) or even better full hog replace it entirely and migrate to using something like Caffeine (https://github.com/ben-manes/caffeine)