You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2025/02/13 14:44:26 UTC
[PR] Removed double check locking in AccessExpressionImpl.parse [accumulo-access]
dlmarion opened a new pull request, #86:
URL: https://github.com/apache/accumulo-access/pull/86
(no comment)
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Removed double check locking in AccessExpressionImpl.parse [accumulo-access]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #86:
URL: https://github.com/apache/accumulo-access/pull/86#discussion_r1954734336
##########
src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java:
##########
@@ -46,13 +48,12 @@ public String getExpression() {
@Override
public ParsedAccessExpression parse() {
- if (parsed == null) {
- synchronized (this) {
- if (parsed == null) {
- parsed = ParsedAccessExpressionImpl.parseExpression(expression.getBytes(UTF_8));
- }
+ return parseTreeRef.updateAndGet((parseTree) -> {
Review Comment:
I think these changes are functionally equivalent. One nice feature for this function to have is that it always returns the same reference and never spuriously returns a different object. Both algorithms seem to have this behavior.
This change has two differences. First it will always write to memory because updateAndGet always writes to memory even when nothing changed. Second threads may compute a parse tree that is thrown away. Computing a parse tree and throwing it aways is probably not a big deal as long as the same reference is always returned.
It would be nice to avoid always writing to the volatile when we read as that could have cache side effects.
The following is an attempt to avoid the write to memory when its not needed. Multiple threads may still compute the parse tree, but the method should always return just one of those.
```java
var parseTree = parsed.get();
if (parseTree == null) {
parsed.compareAndSet(null,
ParsedAccessExpressionImpl.parseExpression(expression.getBytes(UTF_8)));
// must get() again in case another thread won w/ the compare and set, this ensures this
// method always returns the exact same object
parseTree = parsed.get();
}
return parseTree;
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Removed double check locking in AccessExpressionImpl.parse [accumulo-access]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #86:
URL: https://github.com/apache/accumulo-access/pull/86
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org