On 16/09/2008, Simon Kitching <skitching@apache.org> wrote:
> sebb schrieb:
>
> > On 16/09/2008, Simon Kitching <skitching@apache.org> 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: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
|