[ https://issues.apache.org/jira/browse/CURATOR-447?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16361667#comment-16361667 ]
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_r167733216 --- Diff: curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java --- @@ -702,19 +687,18 @@ private TreeNode find(String findPath) ImmutableMap.Builder<String, ChildData> builder = ImmutableMap.builder(); for ( Map.Entry<String, TreeNode> entry : map.entrySet() ) { - TreeNode childNode = entry.getValue(); - ChildData childData = childNode.childData; - // Double-check liveness after retreiving data. - if ( childData != null && childNode.nodeState == NodeState.LIVE ) + ChildData childData = entry.getValue().childData; + // Double-check liveness after retrieving data. + if ( isLive(childData) ) --- End diff -- It's subtle, but the point I'm trying to make is that you have to read `.childData` exactly once, then check the liveness, then use only that value going forward. It would be an error to remove the local variable here and reference `entry.getValue().childData` twice. It's a "double" check because _generally_ there shouldn't be any non-LIVE children in the parent `children` map that we're iterating. But due to volatility we have to "double check" that child nodes read from the map are still alive after pulling `.childData` > 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)