D5638: branchmap: encapsulate cache updating in the map itself
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG328ca3b9e545: branchmap: encapsulate cache updating in the
map itself (authored by mjpieters, committed by ).
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D5638?vs=13543=13914
REVISION DETAIL
https://phab.mercurial-scm.org/D5638
AFFECTED FILES
contrib/perf.py
mercurial/branchmap.py
mercurial/localrepo.py
mercurial/statichttprepo.py
mercurial/streamclone.py
CHANGE DETAILS
diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
--- a/mercurial/streamclone.py
+++ b/mercurial/streamclone.py
@@ -13,7 +13,6 @@
from .i18n import _
from . import (
-branchmap,
cacheutil,
error,
narrowspec,
@@ -174,7 +173,7 @@
repo._writerequirements()
if rbranchmap:
-branchmap.replacecache(repo, rbranchmap)
+repo._branchcaches.replace(repo, rbranchmap)
repo.invalidate()
diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
--- a/mercurial/statichttprepo.py
+++ b/mercurial/statichttprepo.py
@@ -13,6 +13,7 @@
from .i18n import _
from . import (
+branchmap,
changelog,
error,
localrepo,
@@ -193,7 +194,7 @@
self.changelog = changelog.changelog(self.svfs)
self._tags = None
self.nodetagscache = None
-self._branchcaches = {}
+self._branchcaches = branchmap.BranchMapCache()
self._revbranchcache = None
self.encodepats = None
self.decodepats = None
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -992,7 +992,7 @@
self._dirstatevalidatewarned = False
-self._branchcaches = {}
+self._branchcaches = branchmap.BranchMapCache()
self._revbranchcache = None
self._filterpats = {}
self._datafilters = {}
@@ -1520,8 +1520,7 @@
def branchmap(self):
'''returns a dictionary {branch: [branchheads]} with branchheads
ordered by increasing revision number'''
-branchmap.updatecache(self)
-return self._branchcaches[self.filtername]
+return self._branchcaches[self]
@unfilteredmethod
def revbranchcache(self):
@@ -2073,9 +2072,9 @@
return
if tr is None or tr.changes['origrepolen'] < len(self):
-# updating the unfiltered branchmap should refresh all the others,
+# accessing the 'ser ved' branchmap should refresh all the others,
self.ui.debug('updating the branch cache\n')
-branchmap.updatecache(self.filtered('served'))
+self.filtered('served').branchmap()
if full:
rbc = self.revbranchcache()
@@ -2093,7 +2092,7 @@
# can't use delattr on proxy
del self.__dict__[r'_tagscache']
-self.unfiltered()._branchcaches.clear()
+self._branchcaches.clear()
self.invalidatevolatilesets()
self._sparsesignaturecache.clear()
diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -43,75 +43,89 @@
'served': 'immutable',
'immutable': 'base'}
-def updatecache(repo):
-"""Update the cache for the given filtered view on a repository"""
-# This can trigger updates for the caches for subsets of the filtered
-# view, e.g. when there is no cache for this filtered view or the cache
-# is stale.
+
+class BranchMapCache(object):
+"""Cache mapping"""
+def __init__(self):
+self._per_filter = {}
-cl = repo.changelog
-filtername = repo.filtername
-bcache = repo._branchcaches.get(filtername)
-if bcache is None or not bcache.validfor(repo):
-# cache object missing or cache object stale? Read from disk
-bcache = branchcache.fromfile(repo)
+def __getitem__(self, repo):
+self.updatecache(repo)
+return self._per_filter[repo.filtername]
+
+def updatecache(self, repo):
+"""Update the cache for the given filtered view on a repository"""
+# This can trigger updates for the caches for subsets of the filtered
+# view, e.g. when there is no cache for this filtered view or the cache
+# is stale.
-revs = []
-if bcache is None:
-# no (fresh) cache available anymore, perhaps we can re-use
-# the cache for a subset, then extend that to add info on missing
-# revisions.
-subsetname = subsettable.get(filtername)
-if subsetname is not None:
-subset = repo.filtered(subsetname)
-bcache = subset.branchmap().copy()
-extrarevs = subset.changelog.filteredrevs - cl.filteredrevs
-revs.extend(r for r in extrarevs if r <= bcache.tiprev)
-else:
-# nothing to fall back on, start empty.
-
D5638: branchmap: encapsulate cache updating in the map itself
martinvonz added a comment. In https://phab.mercurial-scm.org/D5638#86131, @pulkit wrote: > This looks good to me. @martinvonz do you have any concerns? Yes, looks good. Sorry, I thought this had already been queued. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5638 To: mjpieters, #hg-reviewers Cc: pulkit, martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5638: branchmap: encapsulate cache updating in the map itself
pulkit added a comment. This looks good to me. @martinvonz do you have any concerns? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5638 To: mjpieters, #hg-reviewers Cc: pulkit, martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5638: branchmap: encapsulate cache updating in the map itself
mjpieters updated this revision to Diff 13543.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D5638?vs=13502=13543
REVISION DETAIL
https://phab.mercurial-scm.org/D5638
AFFECTED FILES
contrib/perf.py
mercurial/branchmap.py
mercurial/localrepo.py
mercurial/statichttprepo.py
mercurial/streamclone.py
CHANGE DETAILS
diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
--- a/mercurial/streamclone.py
+++ b/mercurial/streamclone.py
@@ -13,7 +13,6 @@
from .i18n import _
from . import (
-branchmap,
cacheutil,
error,
narrowspec,
@@ -174,7 +173,7 @@
repo._writerequirements()
if rbranchmap:
-branchmap.replacecache(repo, rbranchmap)
+repo._branchcaches.replace(repo, rbranchmap)
repo.invalidate()
diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
--- a/mercurial/statichttprepo.py
+++ b/mercurial/statichttprepo.py
@@ -13,6 +13,7 @@
from .i18n import _
from . import (
+branchmap,
changelog,
error,
localrepo,
@@ -192,7 +193,7 @@
self.changelog = changelog.changelog(self.svfs)
self._tags = None
self.nodetagscache = None
-self._branchcaches = {}
+self._branchcaches = branchmap.BranchMapCache()
self._revbranchcache = None
self.encodepats = None
self.decodepats = None
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -992,7 +992,7 @@
self._dirstatevalidatewarned = False
-self._branchcaches = {}
+self._branchcaches = branchmap.BranchMapCache()
self._revbranchcache = None
self._filterpats = {}
self._datafilters = {}
@@ -1520,8 +1520,7 @@
def branchmap(self):
'''returns a dictionary {branch: [branchheads]} with branchheads
ordered by increasing revision number'''
-branchmap.updatecache(self)
-return self._branchcaches[self.filtername]
+return self._branchcaches[self]
@unfilteredmethod
def revbranchcache(self):
@@ -2073,9 +2072,9 @@
return
if tr is None or tr.changes['origrepolen'] < len(self):
-# updating the unfiltered branchmap should refresh all the others,
+# accessing the 'ser ved' branchmap should refresh all the others,
self.ui.debug('updating the branch cache\n')
-branchmap.updatecache(self.filtered('served'))
+self.filtered('served').branchmap()
if full:
rbc = self.revbranchcache()
@@ -2093,7 +2092,7 @@
# can't use delattr on proxy
del self.__dict__[r'_tagscache']
-self.unfiltered()._branchcaches.clear()
+self._branchcaches.clear()
self.invalidatevolatilesets()
self._sparsesignaturecache.clear()
diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -43,75 +43,89 @@
'served': 'immutable',
'immutable': 'base'}
-def updatecache(repo):
-"""Update the cache for the given filtered view on a repository"""
-# This can trigger updates for the caches for subsets of the filtered
-# view, e.g. when there is no cache for this filtered view or the cache
-# is stale.
+
+class BranchMapCache(object):
+"""Cache mapping"""
+def __init__(self):
+self._per_filter = {}
-cl = repo.changelog
-filtername = repo.filtername
-bcache = repo._branchcaches.get(filtername)
-if bcache is None or not bcache.validfor(repo):
-# cache object missing or cache object stale? Read from disk
-bcache = branchcache.fromfile(repo)
+def __getitem__(self, repo):
+self.updatecache(repo)
+return self._per_filter[repo.filtername]
+
+def updatecache(self, repo):
+"""Update the cache for the given filtered view on a repository"""
+# This can trigger updates for the caches for subsets of the filtered
+# view, e.g. when there is no cache for this filtered view or the cache
+# is stale.
-revs = []
-if bcache is None:
-# no (fresh) cache available anymore, perhaps we can re-use
-# the cache for a subset, then extend that to add info on missing
-# revisions.
-subsetname = subsettable.get(filtername)
-if subsetname is not None:
-subset = repo.filtered(subsetname)
-bcache = subset.branchmap().copy()
-extrarevs = subset.changelog.filteredrevs - cl.filteredrevs
-revs.extend(r for r in extrarevs if r <= bcache.tiprev)
-else:
-# nothing to fall back on, start empty.
-bcache = branchcache()
+cl = repo.changelog
+filtername = repo.filtername
+bcache = self._per_filter.get(filtername)
+if bcache
D5638: branchmap: encapsulate cache updating in the map itself
martinvonz added inline comments. INLINE COMMENTS > mjpieters wrote in localrepo.py:2077 > Yes, the updating will change in a later, as yet to be submitted patch. > > But why should it be clear //here// that there is a cache that is kept > up-to-date on access to the branchmap? That's a responsibility of the cache, > not of whatever accesses a branchmap. The cache is an implementation detail > of branchmap, and should really not bleed out into code that merely consumes > the map. Fair enough, it doesn't have to be clear that it updates the cache, but it has to be clear that it does *something*. It currently looks like a statement that can be removed, but I assume it cannot. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5638 To: mjpieters, #hg-reviewers Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5638: branchmap: encapsulate cache updating in the map itself
mjpieters added inline comments. INLINE COMMENTS > martinvonz wrote in perf.py:2299 > Same here: I think this needs to be made compatible with both versions > (before and after this patch) I'll wait for confirmation; see the other patch. > martinvonz wrote in localrepo.py:2077 > Hmm, it's much less clear now that this updates the cache. At the very least, > it deserves a comment. Is the `updatecache()` call from `__getitem__()` > necessary for your later patches? Sorry, I didn't quite follow. Yes, the updating will change in a later, as yet to be submitted patch. But why should it be clear //here// that there is a cache that is kept up-to-date on access to the branchmap? That's a responsibility of the cache, not of whatever accesses a branchmap. The cache is an implementation detail of branchmap, and should really not bleed out into code that merely consumes the map. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5638 To: mjpieters, #hg-reviewers Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5638: branchmap: encapsulate cache updating in the map itself
mjpieters updated this revision to Diff 13502.
mjpieters marked an inline comment as done.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D5638?vs=1=13502
REVISION DETAIL
https://phab.mercurial-scm.org/D5638
AFFECTED FILES
contrib/perf.py
mercurial/branchmap.py
mercurial/localrepo.py
mercurial/statichttprepo.py
mercurial/streamclone.py
CHANGE DETAILS
diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
--- a/mercurial/streamclone.py
+++ b/mercurial/streamclone.py
@@ -13,7 +13,6 @@
from .i18n import _
from . import (
-branchmap,
cacheutil,
error,
narrowspec,
@@ -174,7 +173,7 @@
repo._writerequirements()
if rbranchmap:
-branchmap.replacecache(repo, rbranchmap)
+repo._branchcaches.replace(repo, rbranchmap)
repo.invalidate()
diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
--- a/mercurial/statichttprepo.py
+++ b/mercurial/statichttprepo.py
@@ -13,6 +13,7 @@
from .i18n import _
from . import (
+branchmap,
changelog,
error,
localrepo,
@@ -192,7 +193,7 @@
self.changelog = changelog.changelog(self.svfs)
self._tags = None
self.nodetagscache = None
-self._branchcaches = {}
+self._branchcaches = branchmap.BranchMapCache()
self._revbranchcache = None
self.encodepats = None
self.decodepats = None
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -992,7 +992,7 @@
self._dirstatevalidatewarned = False
-self._branchcaches = {}
+self._branchcaches = branchmap.BranchMapCache()
self._revbranchcache = None
self._filterpats = {}
self._datafilters = {}
@@ -1520,8 +1520,7 @@
def branchmap(self):
'''returns a dictionary {branch: [branchheads]} with branchheads
ordered by increasing revision number'''
-branchmap.updatecache(self)
-return self._branchcaches[self.filtername]
+return self._branchcaches[self]
@unfilteredmethod
def revbranchcache(self):
@@ -2073,9 +2072,9 @@
return
if tr is None or tr.changes['origrepolen'] < len(self):
-# updating the unfiltered branchmap should refresh all the others,
+# accessing the 'ser ved' branchmap should refresh all the others,
self.ui.debug('updating the branch cache\n')
-branchmap.updatecache(self.filtered('served'))
+self.filtered('served').branchmap()
if full:
rbc = self.revbranchcache()
@@ -2093,7 +2092,7 @@
# can't use delattr on proxy
del self.__dict__[r'_tagscache']
-self.unfiltered()._branchcaches.clear()
+self._branchcaches.clear()
self.invalidatevolatilesets()
self._sparsesignaturecache.clear()
diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -43,75 +43,89 @@
'served': 'immutable',
'immutable': 'base'}
-def updatecache(repo):
-"""Update the cache for the given filtered view on a repository"""
-# This can trigger updates for the caches for subsets of the filtered
-# view, e.g. when there is no cache for this filtered view or the cache
-# is stale.
+
+class BranchMapCache(object):
+"""Cache mapping"""
+def __init__(self):
+self._per_filter = {}
-cl = repo.changelog
-filtername = repo.filtername
-bcache = repo._branchcaches.get(filtername)
-if bcache is None or not bcache.validfor(repo):
-# cache object missing or cache object stale? Read from disk
-bcache = branchcache.fromfile(repo)
+def __getitem__(self, repo):
+self.updatecache(repo)
+return self._per_filter[repo.filtername]
+
+def updatecache(self, repo):
+"""Update the cache for the given filtered view on a repository"""
+# This can trigger updates for the caches for subsets of the filtered
+# view, e.g. when there is no cache for this filtered view or the cache
+# is stale.
-revs = []
-if bcache is None:
-# no (fresh) cache available anymore, perhaps we can re-use
-# the cache for a subset, then extend that to add info on missing
-# revisions.
-subsetname = subsettable.get(filtername)
-if subsetname is not None:
-subset = repo.filtered(subsetname)
-bcache = subset.branchmap().copy()
-extrarevs = subset.changelog.filteredrevs - cl.filteredrevs
-revs.extend(r for r in extrarevs if r <= bcache.tiprev)
-else:
-# nothing to fall back on, start empty.
-bcache = branchcache()
+cl = repo.changelog
+filtername = repo.filtername
+bcache =
D5638: branchmap: encapsulate cache updating in the map itself
martinvonz added inline comments.
INLINE COMMENTS
> perf.py:2299
> else:
> -view._branchcaches.pop(filtername, None)
> +view._branchcaches._per_filter.pop(filtername, None)
> view.branchmap()
Same here: I think this needs to be made compatible with both versions (before
and after this patch)
> localrepo.py:2077
> self.ui.debug('updating the branch cache\n')
> -branchmap.updatecache(self.filtered('served'))
> +self.filtered('served').branchmap()
>
Hmm, it's much less clear now that this updates the cache. At the very least,
it deserves a comment. Is the `updatecache()` call from `__getitem__()`
necessary for your later patches? Sorry, I didn't quite follow.
> streamclone.py:16
> from . import (
> branchmap,
> cacheutil,
`test-check-pyflakes.t` says that this is now unused
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D5638
To: mjpieters, #hg-reviewers
Cc: martinvonz, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5638: branchmap: encapsulate cache updating in the map itself
mjpieters created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
Rather than have a repository update the cache, move handling of cache updates
into the branchmap module, in the form of a custom mapping class.
This makes later performance improvements easier to handle too.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D5638
AFFECTED FILES
contrib/perf.py
mercurial/branchmap.py
mercurial/localrepo.py
mercurial/statichttprepo.py
mercurial/streamclone.py
CHANGE DETAILS
diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
--- a/mercurial/streamclone.py
+++ b/mercurial/streamclone.py
@@ -174,7 +174,7 @@
repo._writerequirements()
if rbranchmap:
-branchmap.replacecache(repo, rbranchmap)
+repo._branchcaches.replace(repo, rbranchmap)
repo.invalidate()
diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
--- a/mercurial/statichttprepo.py
+++ b/mercurial/statichttprepo.py
@@ -13,6 +13,7 @@
from .i18n import _
from . import (
+branchmap,
changelog,
error,
localrepo,
@@ -192,7 +193,7 @@
self.changelog = changelog.changelog(self.svfs)
self._tags = None
self.nodetagscache = None
-self._branchcaches = {}
+self._branchcaches = branchmap.BranchMapCache()
self._revbranchcache = None
self.encodepats = None
self.decodepats = None
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -991,7 +991,7 @@
self._dirstatevalidatewarned = False
-self._branchcaches = {}
+self._branchcaches = branchmap.BranchMapCache()
self._revbranchcache = None
self._filterpats = {}
self._datafilters = {}
@@ -1519,8 +1519,7 @@
def branchmap(self):
'''returns a dictionary {branch: [branchheads]} with branchheads
ordered by increasing revision number'''
-branchmap.updatecache(self)
-return self._branchcaches[self.filtername]
+return self._branchcaches[self]
@unfilteredmethod
def revbranchcache(self):
@@ -2073,9 +2072,9 @@
return
if tr is None or tr.changes['origrepolen'] < len(self):
-# updating the unfiltered branchmap should refresh all the others,
+# accessing the 'ser ved' branchmap should refresh all the others,
self.ui.debug('updating the branch cache\n')
-branchmap.updatecache(self.filtered('served'))
+self.filtered('served').branchmap()
if full:
rbc = self.revbranchcache()
@@ -2093,7 +2092,7 @@
# can't use delattr on proxy
del self.__dict__[r'_tagscache']
-self.unfiltered()._branchcaches.clear()
+self._branchcaches.clear()
self.invalidatevolatilesets()
self._sparsesignaturecache.clear()
diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -43,75 +43,89 @@
'served': 'immutable',
'immutable': 'base'}
-def updatecache(repo):
-"""Update the cache for the given filtered view on a repository"""
-# This can trigger updates for the caches for subsets of the filtered
-# view, e.g. when there is no cache for this filtered view or the cache
-# is stale.
+
+class BranchMapCache(object):
+"""Cache mapping"""
+def __init__(self):
+self._per_filter = {}
-cl = repo.changelog
-filtername = repo.filtername
-bcache = repo._branchcaches.get(filtername)
-if bcache is None or not bcache.validfor(repo):
-# cache object missing or cache object stale? Read from disk
-bcache = branchcache.fromfile(repo)
+def __getitem__(self, repo):
+self.updatecache(repo)
+return self._per_filter[repo.filtername]
+
+def updatecache(self, repo):
+"""Update the cache for the given filtered view on a repository"""
+# This can trigger updates for the caches for subsets of the filtered
+# view, e.g. when there is no cache for this filtered view or the cache
+# is stale.
-revs = []
-if bcache is None:
-# no (fresh) cache available anymore, perhaps we can re-use
-# the cache for a subset, then extend that to add info on missing
-# revisions.
-subsetname = subsettable.get(filtername)
-if subsetname is not None:
-subset = repo.filtered(subsetname)
-bcache = subset.branchmap().copy()
-extrarevs = subset.changelog.filteredrevs - cl.filteredrevs
-revs.extend(r for r in extrarevs if r <= bcache.tiprev)
-else:
-# nothing to fall back on, start empty.
-bcache = branchcache()
+cl = repo.changelog
+
