[ https://issues.apache.org/jira/browse/CURATOR-447?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16361409#comment-16361409 ]
ASF GitHub Bot commented on CURATOR-447: ---------------------------------------- Github user dragonsinth commented on a diff in the pull request: https://github.com/apache/curator/pull/250#discussion_r167672355 --- Diff: curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java --- @@ -397,14 +396,11 @@ public void processResult(CuratorFramework client, CuratorEvent event) throws Ex break; } - ConcurrentMap<String, TreeNode> childMap = children; - if ( childMap == null ) + ConcurrentMap<String, TreeNode> childMap; + while ( (childMap = children) == null ) { childMap = Maps.newConcurrentMap(); - if ( !childrenUpdater.compareAndSet(this, null, childMap) ) - { - childMap = children; - } + if ( childrenUpdater.compareAndSet(this, null, childMap) ) break; --- End diff -- I see why you turned this into a loop (nice catch), but I think it reads a little clearly and more deliberateif you revert back to the original code and just change the `if` -> `while`. Ideally if Curator moves wholesale to Java 1.8, funcs, like: ``` ConcurrentMap<String, TreeNode> childMap = childrenUpdater.updateAndGet(this, (oldChildren) -> { return oldChildren != null ? oldChildren : Maps.newConcurrentMap(); }); ``` > TreeCache: Improve memory usage and concurrent update logic > ----------------------------------------------------------- > > Key: CURATOR-447 > URL: https://issues.apache.org/jira/browse/CURATOR-447 > Project: Apache Curator > Issue Type: Improvement > Components: Recipes > Affects Versions: 3.3.0, 2.12.0 > Reporter: Nick Hill > Priority: Minor > Labels: performance > > Jira https://issues.apache.org/jira/browse/CURATOR-374 reduced per-node > memory usage in {{TreeCache}}. It can be improved further via removal of the > {{nodeState}} field - its {{LIVE}} state corresponds exactly to the adjacent > {{childData}} field being non-null, and a sentinel {{ChildData}} value can be > used for the {{DEAD}} state. This simplification also reduces the room for > bugs and state inconsistencies. > Other improvements included: > * A further simplification to have {{TreeNode}} extend {{AtomicReference}}, > which obviates the need for an explicit {{childData}} field > * More robust cache update logic (in get-children and get-data event > callbacks) > * Avoid overhead of incrementing/decrementing the {{outstandingOps}} atomic > integer post-initialization -- This message was sent by Atlassian JIRA (v7.6.3#76005)