On 16/09/2008, Simon Kitching <[EMAIL PROTECTED]> wrote:
> sebb schrieb:
>
> > On 16/09/2008, Jörg Schaible <[EMAIL PROTECTED]> wrote:
> >
> >
> > > sebb wrote:
> > >  > On 16/09/2008, Simon Kitching <[EMAIL PROTECTED]> wrote:
> > >
> > > [snip]
> > >
> > >
> > >
> > > >
> > > > >  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...
> > >
> > >
> > > My guess: It's used in case of a circular object graph ...
> > >
> > >
> > >
> >
> > Yes, they are used in the enclosing class for just that purpose.
> >
> > But as far as I can see there is no need for the methods to be
> > anything other than private - and the contents of the HashMap only
> > contain anything during the calculation of the hashcode, so it's not
> > obvious how the methods could be called anyway.
> >
> > I meant it was probably a mistake for the methods to be package protected.
> >
> >
>  In that case, I wonder why it bothers to use a thread-local at all, rather
> than just passing the registry object around as a parameter...
>
>  Is the performance benefit of avoiding creation of a HashSet object really
> measurable given that this class is doing significant amounts of reflection?
>
>  Because this class never calls ThreadLocal.remove(), the created HashSet
> object will remain attached to every thread that ever was active when a
> HashCodeBuilder call was made. That's not *too* bad, ie it seems to me that
> it would be acceptable as long as there was a corresponding benefit
> somewhere. But I can't see the benefit ATM..

Ditto.

I think it would make also sense to privatise the add/remove etc methods.

However, this would change the API.
But as that part of the API seems to be useless, I would not object to
it being removed.

>
> ---------------------------------------------------------------------
>  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]

Reply via email to