Re: Modify ZooKeeper Java client to hold weak references to Watcher objects
On 22 March 2010 16:59, Patrick Hunt wrote: > Would "WeakWatcher", a proxy watcher that holds a weak reference to the > proxied watcher, not also solve this with minimal overhead? You could do > this today. > > Hi, Yup a WeakWatcher would certainly provide an interim solution. > > 1/ Add a useWeakReferences parameter to new constructor (sets default >> behaviour) >> 2/ Add alternative methods, which take useWeakWatcherRef boolean. >> > > I would prefer option 2 over 1, since I could see some users wishing to use > both weak/strong in the same session. Couldn't we just mark the watcher with > a "implements WeakWatcher" obviating the need for an explicit parameter? > I agree would be clearer just to add parameter to methods. If you set this through a constructor option people might lose track of what the default behaviour was. > > An additoinal option - we could do the same thing by instead having a > "ZooKeeperWeakWatches" subclass of zookeeper that provides this additional > functionality (if we could overcome other objections) which would not impact > existing users. You might be able to do this today, or with minimal changes > to the ZooKeeper class... > > > I would prefer deregisterWatcher, as a result. >> > > Seems like this would be a useful feature. Today you can get away with not > doing this by using something like a StrongWatcher proxy similar to above > (or some other way of ignoring the eventual trigger). Each deregister > requires a roundtrip to the server however (also handle the case when > disconnected of course). We might want to allow multiple paths to be dereg > as once... (the server will kill the connection if the request packet size > exceeds 1mb though so keep that in mind too...). Not sure if there are other > technical issues to consider. > I guess an argument for deregisterWatcher is this: even if you request for your Watcher to be held with a weak reference, just because you've lost your own reference doesn't mean than the GC has had time to determine that it is now unreferenced and that the weak reference held by the Zk client will return null. Consequently, if you have code which reacts to events, there is a race condition where your zombie could come back to life unless it has idempotent behaviour. I was thinking about this in relation to a simple set primitive called ZkDistributedSet I have developed (pasted below). Just say for example I have an File Explorer type application. As the user selects a new node, I create a new ZkDistributedSet and assign it to explorer.currentNodeContents. I also register the FileExplorer to receive onStateChange events with the ZkDistributedSet base class ZkSyncPrimitive. Now, as I browse the node hierarchy, I simply do explorer.currentNodeContent = new ZkDistributedSet(currentNodePath). A problem is going to be that occasionally I am going to receive events generated when nodes I visited earlier have changed. In this case it doesn't matter so much... the user might simply see an extra flicker as the File Explorer unnecessarily redraws the contents of the current node. The question is whether there are other situations where this could cause a real problem. It seems probably not. So long as the behaviour of the primitives is idempotent, and the client code does not make assumptions about the state of primitives it holds references to when it hears one has changed. > Patrick > > > Dominic Williams wrote: > >> Quite a few platforms make the specification of weak listeners explicit >> e.g. >> in ActionScript you can specify a boolean in addEventListener that >> specifies >> whether the event source should hold weak references to listeners. >> Therefore >> I think whether you want an event source to hold a weak or strong >> reference >> to your listener is really an application-level choice i.e. it is the >> application logic's choice whether to supply an anonymous inner class >> instance that will disappear unless the event source holds a strong >> reference, or a named object that you want to be freed from memory when >> *you* remove your references to it. >> >> I think for many application programming problems, there is no nice way to >> manage memory unless an event source offers to hold weak references to >> listeners. >> >> deregisterWatcher certainly works for things like locks where you do try { >> lock.lock(); } finally { lock.unlock(); } but how about where you are >> continually creating objects that maintain a copy of some items beneath >> nodes (where, say, you are constantly changing your focus from one node to >> another). >> >> In this case, most probably you will want to create a primitive that >> calls deregisterWatcher in finalize(). But the problem of course is that >> finalize() will never get called. >> >> For that reason, you end up with references to primitives on which you >> *must* call close() before you lose a reference to them. >> >> In practice, this is just not possible to do reliably which is why I
Re: Modify ZooKeeper Java client to hold weak references to Watcher objects
Would "WeakWatcher", a proxy watcher that holds a weak reference to the proxied watcher, not also solve this with minimal overhead? You could do this today. 1/ Add a useWeakReferences parameter to new constructor (sets default behaviour) 2/ Add alternative methods, which take useWeakWatcherRef boolean. I would prefer option 2 over 1, since I could see some users wishing to use both weak/strong in the same session. Couldn't we just mark the watcher with a "implements WeakWatcher" obviating the need for an explicit parameter? An additoinal option - we could do the same thing by instead having a "ZooKeeperWeakWatches" subclass of zookeeper that provides this additional functionality (if we could overcome other objections) which would not impact existing users. You might be able to do this today, or with minimal changes to the ZooKeeper class... I would prefer deregisterWatcher, as a result. Seems like this would be a useful feature. Today you can get away with not doing this by using something like a StrongWatcher proxy similar to above (or some other way of ignoring the eventual trigger). Each deregister requires a roundtrip to the server however (also handle the case when disconnected of course). We might want to allow multiple paths to be dereg as once... (the server will kill the connection if the request packet size exceeds 1mb though so keep that in mind too...). Not sure if there are other technical issues to consider. Patrick Dominic Williams wrote: Quite a few platforms make the specification of weak listeners explicit e.g. in ActionScript you can specify a boolean in addEventListener that specifies whether the event source should hold weak references to listeners. Therefore I think whether you want an event source to hold a weak or strong reference to your listener is really an application-level choice i.e. it is the application logic's choice whether to supply an anonymous inner class instance that will disappear unless the event source holds a strong reference, or a named object that you want to be freed from memory when *you* remove your references to it. I think for many application programming problems, there is no nice way to manage memory unless an event source offers to hold weak references to listeners. deregisterWatcher certainly works for things like locks where you do try { lock.lock(); } finally { lock.unlock(); } but how about where you are continually creating objects that maintain a copy of some items beneath nodes (where, say, you are constantly changing your focus from one node to another). In this case, most probably you will want to create a primitive that calls deregisterWatcher in finalize(). But the problem of course is that finalize() will never get called. For that reason, you end up with references to primitives on which you *must* call close() before you lose a reference to them. In practice, this is just not possible to do reliably which is why I don't think there can be a substitute to weak references. Best, Dominic On 19 March 2010 18:47, Henry Robinson wrote: (moved to zookeeper-dev) This API exposes internal implementation details which ties future versions of the client to supporting a particular set of semantics. I would prefer deregisterWatcher, as a result. Henry On 19 March 2010 03:11, Dominic Williams wrote: Hi I can see some people might be assigning for example anonymous class instances as watchers/handlers, and not keeping any references to them. To avoid breaking existing use cases, two options: 1/ Add a useWeakReferences parameter to new constructor (sets default behaviour) 2/ Add alternative methods, which take useWeakWatcherRef boolean. Internally will be trivial to have both private final Map> dataWatches = new HashMap>(); private final Map> dataWatchesWeak = new HashMap>(); On 18 March 2010 22:47, Ted Dunning wrote: This kind of sounds strange to me. My typical idiom is to create a watcher but not retain any references to it outside the client. It sounds to me like your change will cause my watchers to be collected and deactivated when GC happens. On Thu, Mar 18, 2010 at 3:32 AM, Dominic Williams < thedwilli...@googlemail.com> wrote: The current ZooKeeper client holds strong references to Watcher objects. I want to change the client so it only holds weak references. Feedback please. -- Henry Robinson Software Engineer Cloudera 415-994-6679
Re: Modify ZooKeeper Java client to hold weak references to Watcher objects
Quite a few platforms make the specification of weak listeners explicit e.g. in ActionScript you can specify a boolean in addEventListener that specifies whether the event source should hold weak references to listeners. Therefore I think whether you want an event source to hold a weak or strong reference to your listener is really an application-level choice i.e. it is the application logic's choice whether to supply an anonymous inner class instance that will disappear unless the event source holds a strong reference, or a named object that you want to be freed from memory when *you* remove your references to it. I think for many application programming problems, there is no nice way to manage memory unless an event source offers to hold weak references to listeners. deregisterWatcher certainly works for things like locks where you do try { lock.lock(); } finally { lock.unlock(); } but how about where you are continually creating objects that maintain a copy of some items beneath nodes (where, say, you are constantly changing your focus from one node to another). In this case, most probably you will want to create a primitive that calls deregisterWatcher in finalize(). But the problem of course is that finalize() will never get called. For that reason, you end up with references to primitives on which you *must* call close() before you lose a reference to them. In practice, this is just not possible to do reliably which is why I don't think there can be a substitute to weak references. Best, Dominic On 19 March 2010 18:47, Henry Robinson wrote: > (moved to zookeeper-dev) > > This API exposes internal implementation details which ties future versions > of the client to supporting a particular set of semantics. > > I would prefer deregisterWatcher, as a result. > > Henry > > On 19 March 2010 03:11, Dominic Williams >wrote: > > > Hi I can see some people might be assigning for example anonymous class > > instances as watchers/handlers, and not keeping any references to them. > > > > To avoid breaking existing use cases, two options: > > > > 1/ Add a useWeakReferences parameter to new constructor (sets default > > behaviour) > > > > 2/ Add alternative methods, which take useWeakWatcherRef boolean. > > > > Internally will be trivial to have both > > > > private final Map> dataWatches = > >new HashMap>(); > > > > private final Map> dataWatchesWeak = > > new HashMap>(); > > > > On 18 March 2010 22:47, Ted Dunning wrote: > > > > > This kind of sounds strange to me. > > > > > > My typical idiom is to create a watcher but not retain any references > to > > it > > > outside the client. It sounds to me like your change will cause my > > > watchers > > > to be collected and deactivated when GC happens. > > > > > > On Thu, Mar 18, 2010 at 3:32 AM, Dominic Williams < > > > thedwilli...@googlemail.com> wrote: > > > > > > > > > > > The current ZooKeeper client holds strong references to Watcher > > objects. > > > I > > > > want to change the client so it only holds weak references. Feedback > > > > please. > > > > > > > > > -- > Henry Robinson > Software Engineer > Cloudera > 415-994-6679 >
Re: Modify ZooKeeper Java client to hold weak references to Watcher objects
(moved to zookeeper-dev) This API exposes internal implementation details which ties future versions of the client to supporting a particular set of semantics. I would prefer deregisterWatcher, as a result. Henry On 19 March 2010 03:11, Dominic Williams wrote: > Hi I can see some people might be assigning for example anonymous class > instances as watchers/handlers, and not keeping any references to them. > > To avoid breaking existing use cases, two options: > > 1/ Add a useWeakReferences parameter to new constructor (sets default > behaviour) > > 2/ Add alternative methods, which take useWeakWatcherRef boolean. > > Internally will be trivial to have both > > private final Map> dataWatches = >new HashMap>(); > > private final Map> dataWatchesWeak = > new HashMap>(); > > On 18 March 2010 22:47, Ted Dunning wrote: > > > This kind of sounds strange to me. > > > > My typical idiom is to create a watcher but not retain any references to > it > > outside the client. It sounds to me like your change will cause my > > watchers > > to be collected and deactivated when GC happens. > > > > On Thu, Mar 18, 2010 at 3:32 AM, Dominic Williams < > > thedwilli...@googlemail.com> wrote: > > > > > > > > The current ZooKeeper client holds strong references to Watcher > objects. > > I > > > want to change the client so it only holds weak references. Feedback > > > please. > > > -- Henry Robinson Software Engineer Cloudera 415-994-6679