D5291: branchmap: build the revbranchcache._namesreverse() only when required
This revision was automatically updated to reflect the committed changes. Closed by commit rHG50a64c321c1e: branchmap: build the revbranchcache._namesreverse() only when required (authored by pulkit, committed by ). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5291?vs=12586=12592 REVISION DETAIL https://phab.mercurial-scm.org/D5291 AFFECTED FILES mercurial/branchmap.py CHANGE DETAILS diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py --- a/mercurial/branchmap.py +++ b/mercurial/branchmap.py @@ -397,15 +397,18 @@ self._names = [] self._rbcnamescount = len(self._names) # number of names read at # _rbcsnameslen -self._namesreverse = dict((b, r) for r, b in enumerate(self._names)) def _clear(self): self._rbcsnameslen = 0 del self._names[:] self._rbcnamescount = 0 -self._namesreverse.clear() self._rbcrevslen = len(self._repo.changelog) self._rbcrevs = bytearray(self._rbcrevslen * _rbcrecsize) +util.clearcachedproperty(self, '_namesreverse') + +@util.propertycache +def _namesreverse(self): +return dict((b, r) for r, b in enumerate(self._names)) def branchinfo(self, rev): """Return branch name and close flag for rev, using and updating To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5291: branchmap: build the revbranchcache._namesreverse() only when required
pulkit updated this revision to Diff 12586. pulkit edited the summary of this revision. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5291?vs=12576=12586 REVISION DETAIL https://phab.mercurial-scm.org/D5291 AFFECTED FILES mercurial/branchmap.py CHANGE DETAILS diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py --- a/mercurial/branchmap.py +++ b/mercurial/branchmap.py @@ -397,15 +397,18 @@ self._names = [] self._rbcnamescount = len(self._names) # number of names read at # _rbcsnameslen -self._namesreverse = dict((b, r) for r, b in enumerate(self._names)) def _clear(self): self._rbcsnameslen = 0 del self._names[:] self._rbcnamescount = 0 -self._namesreverse.clear() self._rbcrevslen = len(self._repo.changelog) self._rbcrevs = bytearray(self._rbcrevslen * _rbcrecsize) +util.clearcachedproperty(self, '_namesreverse') + +@util.propertycache +def _namesreverse(self): +return dict((b, r) for r, b in enumerate(self._names)) def branchinfo(self, rev): """Return branch name and close flag for rev, using and updating To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5291: branchmap: build the revbranchcache._namesreverse() only when required
yuja added a comment. > > Maybe _namesreverse can be a propertycache, instead of duplicating it. > > `del self.__dict__[r'...']` can be used to discard the cache. > > Hm, I am not sure if I feel good about doing `del self.__dict__[r'...']` thing. It's pretty common in Mercurial codebase. That said, I found a better way. `util.clearcachedproperty(self, '...')`. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5291 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5291: branchmap: build the revbranchcache._namesreverse() only when required
> > Maybe _namesreverse can be a propertycache, instead of duplicating it. > > `del self.__dict__[r'...']` can be used to discard the cache. > > Hm, I am not sure if I feel good about doing `del self.__dict__[r'...']` > thing. It's pretty common in Mercurial codebase. That said, I found a better way. `util.clearcachedproperty(self, '...')`. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5291: branchmap: build the revbranchcache._namesreverse() only when required
pulkit added a comment. In https://phab.mercurial-scm.org/D5291#78794, @yuja wrote: > > +if self._namesreverse is None: > > +self._namesreverse = dict((b, r) for r, b in enumerate(self._names)) > > > > if b in self._namesreverse: > > branchidx = self._namesreverse[b] > > else: > > > > @@ -467,6 +469,8 @@ > > > > def setdata(self, branch, rev, node, close): > > """add new data information to the cache""" > > > > +if self._namesreverse is None: > > +self._namesreverse = dict((b, r) for r, b in enumerate(self._names)) > > Maybe _namesreverse can be a propertycache, instead of duplicating it. > `del self.__dict__[r'...']` can be used to discard the cache. Hm, I am not sure if I feel good about doing `del self.__dict__[r'...']` thing. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5291 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5291: branchmap: build the revbranchcache._namesreverse() only when required
yuja added a comment. > +if self._namesreverse is None: > +self._namesreverse = dict((b, r) for r, b in enumerate(self._names)) > > if b in self._namesreverse: > branchidx = self._namesreverse[b] > else: > > @@ -467,6 +469,8 @@ > > def setdata(self, branch, rev, node, close): > """add new data information to the cache""" > > +if self._namesreverse is None: > +self._namesreverse = dict((b, r) for r, b in enumerate(self._names)) Maybe _namesreverse can be a propertycache, instead of duplicating it. `del self.__dict__[r'...']` can be used to discard the cache. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5291 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5291: branchmap: build the revbranchcache._namesreverse() only when required
> +if self._namesreverse is None: > +self._namesreverse = dict((b, r) for r, b in > enumerate(self._names)) > if b in self._namesreverse: > branchidx = self._namesreverse[b] > else: > @@ -467,6 +469,8 @@ > > def setdata(self, branch, rev, node, close): > """add new data information to the cache""" > +if self._namesreverse is None: > +self._namesreverse = dict((b, r) for r, b in > enumerate(self._names)) Maybe _namesreverse can be a propertycache, instead of duplicating it. `del self.__dict__[r'...']` can be used to discard the cache. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5291: branchmap: build the revbranchcache._namesreverse() only when required
pulkit created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY On big repositories with a lot of named branches and that also increasing over time, building of this dict can be expensive and shows up in profile. For our internal repository, this saves ~0.05 seconds. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5291 AFFECTED FILES mercurial/branchmap.py CHANGE DETAILS diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py --- a/mercurial/branchmap.py +++ b/mercurial/branchmap.py @@ -397,13 +397,13 @@ self._names = [] self._rbcnamescount = len(self._names) # number of names read at # _rbcsnameslen -self._namesreverse = dict((b, r) for r, b in enumerate(self._names)) +self._namesreverse = None def _clear(self): self._rbcsnameslen = 0 del self._names[:] self._rbcnamescount = 0 -self._namesreverse.clear() +self._namesreverse = None self._rbcrevslen = len(self._repo.changelog) self._rbcrevs = bytearray(self._rbcrevslen * _rbcrecsize) @@ -453,6 +453,8 @@ """Retrieve branch info from changelog and update _rbcrevs""" changelog = self._repo.changelog b, close = changelog.branchinfo(rev) +if self._namesreverse is None: +self._namesreverse = dict((b, r) for r, b in enumerate(self._names)) if b in self._namesreverse: branchidx = self._namesreverse[b] else: @@ -467,6 +469,8 @@ def setdata(self, branch, rev, node, close): """add new data information to the cache""" +if self._namesreverse is None: +self._namesreverse = dict((b, r) for r, b in enumerate(self._names)) if branch in self._namesreverse: branchidx = self._namesreverse[branch] else: To: pulkit, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
