Github user njhill commented on the issue:

    https://github.com/apache/curator/pull/250
  
    Thanks @dragonsinth, there's of course no rush at all!
    
    I agree that `TreeNode extends AtomicReference` isn't a "win" as such, it 
just looked cleaner to me than using another `AtomicReferenceFieldUpdater`. But 
it's likely subjective and certainly a minor thing.
    
    The `outstandingOps` change seems clearer to me - the field has no use for 
most of the life of the cache and yet is incremented/decremented on every 
single change. It's not the CPU cycle overhead but more the (CPU) cache 
coherency overhead since it's an `AtomicLong`. I didn't think the change 
introduces any raciness - the only times when `outstandingOps` needs to be 
modified (during initialization or re-initialization) it will be guaranteed to 
be non-null.
    
    > I would expect the memory footprint of `ConcurrentMap<String, TreeNode>` 
children to mostly dominate the memory overhead of our own classes
    
    Good point. The description of 
[CURATOR-374](https://issues.apache.org/jira/browse/CURATOR-374) implied this 
might have _some_ mem overhead benefit for very large caches, but removal of 
the `nodeState` field was motivated more by state simplification and 
consolidation of atomic updates. I'll do some quick measurements but I 
acknowledge percentage-wise the saving might not be significant.


---

Reply via email to