spectral planned changes to this revision. spectral added a comment.
In https://phab.mercurial-scm.org/D4366#67144, @indygreg wrote: > I'm not sure how I feel about so many methods having the `if dir in self._lazydirs: self._loadlazy(dir)` pattern. > > On one hand, action at a distance when it involves caching can be dangerous. And doing the lookup inline will avoid a Python function call. > > On the other, it seems very redundant. > > Overall I'm OK with the patch. I just feel like using a `collections.defaultdict` or implementing `__missing__` to automagically resolve missing keys might work out better. Do you think this is a reasonable request? I think it's a reasonable request, but I'm not sure it's feasible with how collections.defaultdict/__missing__ work. They are *only* called by __getitem__. We'd need a separate dict subclass for _dirs that handles __contains__ as well, at the very least, for all the cases of `if dir not in self._dirs: return <something>` I'm playing around with such a thing, it's not difficult but I think might be more awkward than helpful. Even if that doesn't bear fruit, while implementing it I noticed something: I'm not actually populating self._lazydirs in this patch, I appear to have lost it during split/merge/whatever. Oops. I have to hope that the numbers I have in the commit description were before I lost this, since otherwise it's completely incomprehensible that this patch would cause a performance benefit (well, maybe not, if dead code can cause a performance loss, why not the other way around?) REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D4366 To: spectral, #hg-reviewers Cc: indygreg, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
