Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2228#discussion_r208672703
--- 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 --
@jbertram @michaelandrepearce If it is not an hot path I thing that what
Justin is suggesting makes totally sense: anyway if we can afford to put guava
in place I believe that
https://google.github.io/guava/releases/16.0/api/docs/com/google/common/cache/CacheBuilder.html#maximumSize(long)
would be a good option too.
It will scale with a factor specified by the concurrent level specified,
but it could (I do not remember TBH) introduce some behavioural changes from
the current impl.
I understand both @michaelandrepearce and @jbertram opinions so I think
that the general rule to balance changes vs effectiveness on common case apply
here: I leave you decide how much this tradeoff apply here...
I'm honored to be "summoned" :+1:
---
