[PATCH 1 of 6 RFC] manifest: introduce an accessor class for manifests

Durham Goode durham at fb.com
Thu Nov 3 18:33:05 EDT 2016


This series is RFC because it conflicts with my V5 manifest series that 
is out.

These patches remove the circular dependency between manifestlog and 
localrepo by adding a manifestaccessor instance which provides 
invalidating access to the manifest revlog, so multiple objects can 
reference the revlog data without holding on to the repo itself.  I sent 
this out now to get feedback since I think people may have opinions on 
this approach.


On 11/3/16 3:27 PM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1478208817 25200
> #      Thu Nov 03 14:33:37 2016 -0700
> # Branch stable
> # Node ID 1788ee9e1df92ac94b9be84eac6d16e3bad903a9
> # Parent  b9f7b0c10027764cee77f9c6d61877fcffea837f
> manifest: introduce an accessor class for manifests
>
> This introduces a revlogaccessor class which can be used to allow multiple
> objects hold an auto-invalidating reference to a revlog, without having to hold
> a reference to the actual repo object. Future patches will switch repo.manifest
> and repo.manifestlog to access the manifest through this accessor. This will fix
> the circular reference caused by manifestlog and manifestctx holding a reference
> to the repo
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -514,6 +514,11 @@ class localrepository(object):
>           # manifest creation.
>           return manifest.manifest(self.svfs)
>   
> +    @unfilteredpropertycache
> +    def manifestaccessor(self):
> +        return revlogaccessor('00manifest.i', self.svfs,
> +                              self._constructmanifest)
> +
>       @storecache('00manifest.i')
>       def manifestlog(self):
>           return manifest.manifestlog(self.svfs, self)
> @@ -1275,6 +1280,8 @@ class localrepository(object):
>                   delattr(unfiltered, k)
>               except AttributeError:
>                   pass
> +        self.manifestaccessor.clear(clearfilecache)
> +
>           self.invalidatecaches()
>           if not self.currenttransaction():
>               # TODO: Changing contents of store outside transaction
> @@ -1296,6 +1303,7 @@ class localrepository(object):
>               if k == 'dirstate' or k not in self.__dict__:
>                   continue
>               ce.refresh()
> +        self.manifestaccessor.refresh()
>   
>       def _lock(self, vfs, lockname, wait, releasefn, acquirefn, desc,
>                 inheritchecker=None, parentenvvar=None):
> @@ -2004,3 +2012,40 @@ def newreporequirements(repo):
>           requirements.add('manifestv2')
>   
>       return requirements
> +
> +def revlogaccessor(filename, opener, constructor):
> +    """Creates an accessor that provides cached and invalidated access to a
> +    revlog, via instance.revlog. This is useful for letting multiple objects
> +    hold a reference to the revlog, without having to hold a possibly-circular
> +    reference to the actual repository.  """
> +
> +    # We have to use a runtime type here, because the only way to create a
> +    # property is to put it on a class itself, and the property is dynamically
> +    # defined by the filename parameter.
> +    class accessor(object):
> +        def __init__(self):
> +            self._filecache = {}
> +
> +        @scmutil.filecache(filename)
> +        def revlog(self):
> +            return constructor()
> +
> +        def join(self, name):
> +            return opener.join(name)
> +
> +        def clear(self, clearfilecache):
> +            for k in self._filecache.keys():
> +                if clearfilecache:
> +                    del self._filecache[k]
> +                try:
> +                    delattr(self, k)
> +                except AttributeError:
> +                    pass
> +
> +        def refresh(self):
> +            for k, ce in self._filecache.items():
> +                if k not in self.__dict__:
> +                    continue
> +                ce.refresh()
> +
> +    return accessor()
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -1254,7 +1254,7 @@ class manifestlog(object):
>               usetreemanifest = opts.get('treemanifest', usetreemanifest)
>           self._treeinmem = usetreemanifest
>   
> -        self._oldmanifest = repo._constructmanifest()
> +        self._oldmanifest = repo.manifestaccessor.revlog
>           self._revlog = self._oldmanifest
>   
>           # We'll separate this into it's own cache once oldmanifest is no longer
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DQIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=FaX7fFZjM89DjuYNmCnhnV7ZNbcHdxiC0tnJmRFQSnY&s=IFdLTTntCXcFrGeZCVRg9ucxucsdcssbsl8Om89v5rY&e=



More information about the Mercurial-devel mailing list