keith-turner commented on code in PR #113:
URL: https://github.com/apache/accumulo-access/pull/113#discussion_r2942047559
##########
modules/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java:
##########
@@ -43,7 +45,27 @@ public final class AccessEvaluatorImpl implements
AccessEvaluator {
*/
AccessEvaluatorImpl(Predicate<String> authorizationChecker,
AuthorizationValidator authorizationValidator) {
- this.authorizedPredicate = auth ->
authorizationChecker.test(auth.toString());
+
+ // This map helps avoid allocating string objects on each call to this
predicate
+ Map<CharsWrapper,Boolean> checkCache = new ConcurrentHashMap<>();
+ this.authorizedPredicate = auth -> {
+ if (auth instanceof CharsWrapper wrapped) {
+ // Try to avoid allocating a string object and copying the char array.
+ Boolean cachedResult = checkCache.get(wrapped);
+ if (cachedResult == null) {
+ // Not in cache, so have to allocate and copy
+ String authStr = wrapped.toString();
+ cachedResult = authorizationChecker.test(authStr);
Review Comment:
> I tried to track down to see if it could be modified while the predicate
still existed, and it wasn't obvious at a glance.
It should never be modified. The usage pattern is to obtain the chars
wrapper from the thread local, create the predicates that use it, and then call
some recursive functions that parse/evaluate. Those recursive functions never
let the predicate escape. So its only scoped to that thread and the functions
it calls after creating the predicate. An efficient way to handle this in
another language would be to allocate a struct on the stack that is used for
recursive parse/evaluate, but that is not possible in java so need to use the
thread local.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
