On 16/09/2008, Simon Kitching <[EMAIL PROTECTED]> wrote: > sebb schrieb: > > > On 16/09/2008, Simon Kitching <[EMAIL PROTECTED]> wrote: > > > > > > > One other concern is regarding garbage collection. From a brief look at > the > > > code, this thread-local registry object contains a set of Integer > hashcodes. > > > With this change, the set will now contain real hard references to > objects, > > > preventing them from being garbage-collected while they are in the set. > Does > > > this matter? > > > > > > > > > > The entries are only retained in the registry during the hashcode > > calculation - if any were left behind they could mess up subsequent > > calls in the same thread. > > > > As far as I can see, the code always removes any items that are added, > > but it would be worth double-checking that. > > > > > > I did suspect that the objects are removed from the registry before normal > return. So then the IDKey approach should be fine. > > But it would seem to me to be safer to have a call to registry.remove() to > ensure that the threadlocal object is completely cleared, rather than just > relying on the code to correctly balance adds/removes on the HashSet.
The add and remove are in the same try block; remove is in the finally clause. > Or a call to getRegistry().clear(). Could do, but seems like overkill. > Or at least a comment on the registry stating > that it is always empty on return from the reflectionHashCode method etc. OK. > It's not quite clear to me why a threadlocal is being used here. I'm > guessing it is for performance to avoid recreating the HashSet object. I don't know for sure, but that is probably the case. > Or is > it to "tunnel" the registry set between cooperating classes? Not at present, although the registy access methods are package protected. That may be a mistake... > If the latter, > then registry.remove() seems a good idea when it is no longer used, to avoid > leaking an object per thread. > > Sorry for the vague comments; I don't have time to really get to grips with > the code ATM.. NP, it's always useful to thrash such things out at the design stage rather than after deployment ... > Regards, > > Simon > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
