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]
