D6710: branchmap: properly refresh/warm all branchmap caches

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  Overall principle seems good. I made couple of inline comment.

INLINE COMMENTS

> localrepo.py:2200
>  self.ui.debug('updating the branch cache\n')
> -self.filtered('served').branchmap()
> -self.filtered('served.hidden').branchmap()
> +for filt in ['visible', 'visible-hidden', 'served.hidden']:
> +self.filtered(filt).branchmap()

Should we have this list explicitly stored in a list next to the filtermap ? 
That would seems more robust to future changes.

> localrepo.py:2224
> +filtered = self.filtered(filt)
> +filtered.branchmap().write(filtered)
> +

Why the explicite write here ? We don't seems to need it for the previous 
section. Is this because if the cache of the previous subset is valid, the 
write would be skipped ?
If so, consider clarifying it in your comment.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6710/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6710

To: spectral, #hg-reviewers
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6710: branchmap: properly refresh/warm all branchmap caches

[spectral (Kyle Lippincott)]

spectral added a comment.
spectral planned changes to this revision.


  Needs some test updates.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6710/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6710

To: spectral, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6710: branchmap: properly refresh/warm all branchmap caches

[spectral (Kyle Lippincott)]

spectral created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The comment about 'served' refreshing all the others only applies when served 
is
  the head of the branchmap cache subsets. This was already no longer the case
  (and is presumably why served.hidden was added) but the comment was not
  adjusted. Currently, the "heads" of the branchmap cache subsets are the three
  specified here: visible, visible-hidden, served.hidden.
  
  Additionally, 'full' claims it will warm all of the caches that are known 
about,
  but this is not the case - it does not actually warm the branchmap caches for
  subsets that we haven't requested or for subsets that are still considered
  "valid".

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6710

AFFECTED FILES
  mercurial/localrepo.py

CHANGE DETAILS

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2192,10 +2192,13 @@
 return
 
 if tr is None or tr.changes['origrepolen'] < len(self):
-# accessing the 'ser ved' branchmap should refresh all the others,
+# There are three "heads" to the cache hierarchy: visible,
+# visible-hidden, and served.hidden. Updating any of these three
+# should cause all of the others (currently: served, immutable,
+# base) that are stale to be updated.
 self.ui.debug('updating the branch cache\n')
-self.filtered('served').branchmap()
-self.filtered('served.hidden').branchmap()
+for filt in ['visible', 'visible-hidden', 'served.hidden']:
+self.filtered(filt).branchmap()
 
 if full:
 unfi = self.unfiltered()
@@ -2214,6 +2217,12 @@
 self.tags()
 self.filtered('served').tags()
 
+# Warm the branchmap caches even for caches we haven't needed yet,
+# including forcing a write to disk.
+for filt in repoview.filtertable.keys():
+filtered = self.filtered(filt)
+filtered.branchmap().write(filtered)
+
 def invalidatecaches(self):
 
 if r'_tagscache' in vars(self):



To: spectral, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel