2014-08-04 9:57 GMT+02:00 Henrik Johansen <[email protected]>:
> > On 04 Aug 2014, at 9:31 , Nicolai Hess <[email protected]> wrote: > > 2014-08-04 8:50 GMT+02:00 Henrik Johansen <[email protected]>: > >> >> On 03 Aug 2014, at 5:55 , Clément Bera <[email protected]> wrote: >> >> > >> > >> > Right, current hash is bad and long to compute. >> > >> > I think we should use instead >> > BasicIndexedEyeElement >> hash >> > ^ index hashMultiply >> > >> > host identityHash and index hash may lead to many collisions... >> > What do you think ? >> > >> >> I agree there’s not much value in including host in the hash for the use >> case of the icon lookup cache, since those were per host anyways (iirc). >> Index hash will be a sequential number, I haven’t checked the usage >> closely to see if it’ll be a problem wrt. collisions, but removal at least >> might be slow (if that’s ever done) due to lots of scanning needed to find >> the nil slot. Using hashMultiply seems a good precaution. >> >> Cheers, >> Henry >> > > > And as far as I know, the icon lookup isn't needed for the inspector, as > it does not show any icons at all! > > > Well, it still needs to figure out there is no icon to be displayed for > the entry… > In that process, it caches the result (nil), and that cache is too > expensive to query later on, due to the expensive hash calculation needed > to find an entry. > > Cheers, > Henry > Thats the reason I created the issue 13315 NewList without icons (13315 <https://pharo.fogbugz.com/default.asp?13315>) We need a way to tell NewList to not use and therefore don't try to lookup icons, if the client (the inspector) don't provide any icons at all. Maybe NewList should not hold an icon cache at all, but rely on the icon provider for proper caching. Nicolai
