commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [LANG] - suggestions for new Identity Hash code classes
Date Tue, 16 Sep 2008 13:38:24 GMT
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


Mime
View raw message