Patchwork [remotenames,V2] initialization: lazy-load all remotenames

login
register
mail settings
Submitter Tony Tung
Date July 6, 2016, 12:07 a.m.
Message ID <e33e08fbe2d7f15ff777.1467763654@andromeda.local>
Download mbox | patch
Permalink /patch/15756/
State Changes Requested
Headers show

Comments

Tony Tung - July 6, 2016, 12:07 a.m.
# HG changeset patch
# User Tony Tung <tonytung@merly.org>
# Date 1467763493 25200
#      Tue Jul 05 17:04:53 2016 -0700
# Node ID e33e08fbe2d7f15ff7776d3f60497792cc1a0bf5
# Parent  183d5b47f7472b839a96e7be735bb7fbf8068969
initialization: lazy-load all remotenames

If the command doesn't actually require the remotenames to be loaded, this saves 100ms.

Things that I've based this number on: 5 runs each of hg status, hg bookmark.
Martijn Pieters - July 8, 2016, 12:10 p.m.
On 6 July 2016 at 01:07,  <ttung@fb.com> wrote:
> # HG changeset patch
> # User Tony Tung <tonytung@merly.org>
> # Date 1467763493 25200
> #      Tue Jul 05 17:04:53 2016 -0700
> # Node ID e33e08fbe2d7f15ff7776d3f60497792cc1a0bf5
> # Parent  183d5b47f7472b839a96e7be735bb7fbf8068969
> initialization: lazy-load all remotenames
>
> If the command doesn't actually require the remotenames to be loaded, this saves 100ms.
>
> Things that I've based this number on: 5 runs each of hg status, hg bookmark.
>
> diff --git a/remotenames.py b/remotenames.py
> --- a/remotenames.py
> +++ b/remotenames.py
> @@ -224,10 +224,16 @@
>          self.potentialentries = {}
>          self._kind = kind # bookmarks or branches
>          self._repo = repo
> -        self._load()
> +        self.loaded = False
>
> -    def _load(self):
> -        """Read the remotenames file, store entries matching selected kind"""
> +    def _ensureloaded(self):
> +        """Read the remotenames file, store entries matching selected kind.
> +        This only takes effect once.   Subsequent invocations do not have any
> +        effect."""
> +        if self.loaded is True:
> +            return
> +        self.loaded = True
> +

Rather than use self._ensureloaded() everywhere, you can make
`self.potentialentries` a propertycache descriptor:

    @util.propertycache
    def potentialentries(self):
        # Read the remotenames file, store entries matching selected kind.
        entries = {}
        repo = self._repo
        alias_default = repo.ui.configbool('remotenames', 'alias.default')
        for node, nametype, remote, rname in readremotenames(repo):
            if nametype != self._kind:
                continue
            # handle alias_default here
            if remote != "default" and rname == "default" and alias_default:
                name = remote
            else:
                name = joinremotename(remote, rname)
            entries[name] = (node, nametype, remote, rname)
        return entries

This removes the need to track a loaded flag, *and* the need to call
`_ensureloaded()` everywhere. First access to `self.potentialentries`
triggers the above descriptor and the result is cached for you
(replacing the descriptor for future access).

This requires that you drop the `self.potentialentries = {}` line from
`__init__`.

>          repo = self._repo
>          alias_default = repo.ui.configbool('remotenames', 'alias.default')
>          for node, nametype, remote, rname in readremotenames(repo):
> @@ -242,6 +248,8 @@
>
>      def _resolvedata(self, potentialentry):
>          """Check that the node for potentialentry exists and return it"""
> +        self._ensureloaded()
> +
>          if not potentialentry in self.potentialentries:
>              return None
>          node, nametype, remote, rname = self.potentialentries[potentialentry]
> @@ -276,6 +284,7 @@
>              return None
>
>      def keys(self):
> +        self._ensureloaded()
>          for u in self.potentialentries.keys():
>              self._fetchandcache(u)
>          return self.cache.keys()
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/remotenames.py b/remotenames.py
--- a/remotenames.py
+++ b/remotenames.py
@@ -224,10 +224,16 @@ 
         self.potentialentries = {}
         self._kind = kind # bookmarks or branches
         self._repo = repo
-        self._load()
+        self.loaded = False
 
-    def _load(self):
-        """Read the remotenames file, store entries matching selected kind"""
+    def _ensureloaded(self):
+        """Read the remotenames file, store entries matching selected kind.
+        This only takes effect once.   Subsequent invocations do not have any
+        effect."""
+        if self.loaded is True:
+            return
+        self.loaded = True
+
         repo = self._repo
         alias_default = repo.ui.configbool('remotenames', 'alias.default')
         for node, nametype, remote, rname in readremotenames(repo):
@@ -242,6 +248,8 @@ 
 
     def _resolvedata(self, potentialentry):
         """Check that the node for potentialentry exists and return it"""
+        self._ensureloaded()
+
         if not potentialentry in self.potentialentries:
             return None
         node, nametype, remote, rname = self.potentialentries[potentialentry]
@@ -276,6 +284,7 @@ 
             return None
 
     def keys(self):
+        self._ensureloaded()
         for u in self.potentialentries.keys():
             self._fetchandcache(u)
         return self.cache.keys()