Submitter | David Soria Parra |
---|---|
Date | Aug. 7, 2014, 6:08 p.m. |
Message ID | <5dd7cb3ba93256f916a9.1407434927@dev544.prn1.facebook.com> |
Download | mbox | patch |
Permalink | /patch/5313/ |
State | Accepted |
Headers | show |
Comments
On 08/07/2014 11:08 AM, David Soria Parra wrote: > # HG changeset patch > # User David Soria Parra <davidsp@fb.com> > # Date 1407430439 25200 > # Thu Aug 07 09:53:59 2014 -0700 > # Node ID 5dd7cb3ba93256f916a970a4dc1f1b7e178e4176 > # Parent 3da6820d38112cddfc21e2b99ab02957f93d80cf > repoview: add optional hiddencache verification > > To ensure our cache approach is correct and users aren't affected, we are > verifying the cache by default. If we don't encounter any user reports of an > invalid cache we will remove the verification and the internal variable. > > diff --git a/mercurial/repoview.py b/mercurial/repoview.py > --- a/mercurial/repoview.py > +++ b/mercurial/repoview.py > @@ -86,11 +86,19 @@ > if iscachevalid(repo, hideable): > data = repo.vfs.read(cachefile) > hidden = frozenset(struct.unpack('%sI' % (len(data) / 4), data)) > + # internal: don't document as it will be removed in the future. > + if repo.ui.configbool('repoview', 'verifycache', True): > + blocked = cl.ancestors(_gethiddenblockers(repo), inclusive=True) > + computed = frozenset(r for r in hideable if r not in blocked) > + if computed != hidden: > + invalidatecache(repo) > + repo.ui.warn(_('Cache inconsistency detected. Please ' + > + 'open an issue on http://bz.selenic.com.\n')) > + hidden = computed > else: > # recompute cache > blocked = cl.ancestors(_gethiddenblockers(repo), inclusive=True) > hidden = frozenset(r for r in hideable if r not in blocked) > - > # write cache to file > try: > data = struct.pack('%sI' % len(hidden), *hidden) 1. We have a "experimental" config section for such purpose. something like `experimental.cachehidden` would do. 2. You are not writing the new result in case of invalid cache 3. Given that the current default should be "not use the cache" for now I would see the current code structure (some function just exists for the speudo code):: hidden = hiddencomputation(…) if repo.ui.config('experimental', 'cachehidden', False): cache = readcache(…) # or return None if missing or invalid if cache is not None and cache != hidden: repo.ui.warn(…) cache = None if cache is None: writecache(…) return hidden such structure will be easy to translate when we drop the config option: hidden = readcache(…) # or return None if missing or invalid if hidden is None: hidden = hiddencomputation(…) writecache(…) return hidden This implies merging patches 2 and 3. (the cache logic -may- be partially introduced as uncalled function in prior patches if you want to focus on series readability)
Patch
diff --git a/mercurial/repoview.py b/mercurial/repoview.py --- a/mercurial/repoview.py +++ b/mercurial/repoview.py @@ -86,11 +86,19 @@ if iscachevalid(repo, hideable): data = repo.vfs.read(cachefile) hidden = frozenset(struct.unpack('%sI' % (len(data) / 4), data)) + # internal: don't document as it will be removed in the future. + if repo.ui.configbool('repoview', 'verifycache', True): + blocked = cl.ancestors(_gethiddenblockers(repo), inclusive=True) + computed = frozenset(r for r in hideable if r not in blocked) + if computed != hidden: + invalidatecache(repo) + repo.ui.warn(_('Cache inconsistency detected. Please ' + + 'open an issue on http://bz.selenic.com.\n')) + hidden = computed else: # recompute cache blocked = cl.ancestors(_gethiddenblockers(repo), inclusive=True) hidden = frozenset(r for r in hideable if r not in blocked) - # write cache to file try: data = struct.pack('%sI' % len(hidden), *hidden)