Patchwork [remotenames-ext] lazyremotenamedict: make key resolution optional

login
register
mail settings
Submitter Kostia Balytskyi
Date Jan. 23, 2017, 5:14 p.m.
Message ID <ca861f4172c008902927.1485191687@devvm1416.lla2.facebook.com>
Download mbox | patch
Permalink /patch/18270/
State Accepted
Headers show

Comments

Kostia Balytskyi - Jan. 23, 2017, 5:14 p.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1485190647 28800
#      Mon Jan 23 08:57:27 2017 -0800
# Node ID ca861f4172c00890292749d698fe681296059067
# Parent  18f8e0f8ba54270bf158734c781327581cf43634
lazyremotenamedict: make key resolution optional

This fix does not change the default behavior of lazyremotenamedict.
Rather, it adds a keyword argument + a config option to control
whether nodes to which remotenames point are resolved in a revlog or not
when lazyremotenamedict.keys() is called.
An example of a case when we care about the resolving is when we want
to make sure that every key points to an existing node. An opposite
example is when we need all the keys before we intersect that set with
some other set and *later* we might want to check the new, much smaller
set of key-node mappings for existence. In such cases, not resolving
the entire initial set of nodes may be a significant performance gain.
Sean Farley - Jan. 23, 2017, 6:10 p.m.
Kostia Balytskyi <ikostia@fb.com> writes:

> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1485190647 28800
> #      Mon Jan 23 08:57:27 2017 -0800
> # Node ID ca861f4172c00890292749d698fe681296059067
> # Parent  18f8e0f8ba54270bf158734c781327581cf43634
> lazyremotenamedict: make key resolution optional
>
> This fix does not change the default behavior of lazyremotenamedict.
> Rather, it adds a keyword argument + a config option to control
> whether nodes to which remotenames point are resolved in a revlog or not
> when lazyremotenamedict.keys() is called.
> An example of a case when we care about the resolving is when we want
> to make sure that every key points to an existing node. An opposite
> example is when we need all the keys before we intersect that set with
> some other set and *later* we might want to check the new, much smaller
> set of key-node mappings for existence. In such cases, not resolving
> the entire initial set of nodes may be a significant performance gain.

Makes sense to me. Running tests, then will push. Thanks for the patch!

Patch

diff --git a/remotenames.py b/remotenames.py
--- a/remotenames.py
+++ b/remotenames.py
@@ -347,12 +347,25 @@  class lazyremotenamedict(UserDict.DictMi
         else:
             return None
 
-    def keys(self):
+    def keys(self, resolvekeys=None):
+        """Get a list of bookmark names
+
+        `resolvekeys` allows callee to ask whether nodes to which these keys
+        point should be resolved in a revlog (to safeguard against a key
+        pointing to a non-existent node). If this kwarg:
+            - is None: remotenames.resolvekeys config value is read,
+              defaulting to True, as the behavior before this fix
+            - is not None: the bool value of resolvekeys is used"""
+        if resolvekeys is None:
+            resolvekeys = self._repo.ui.configbool("remotenames",
+                                                   "resolvekeys", True)
         if not self.loaded:
             self._load()
-        for u in self.potentialentries.keys():
-            self._fetchandcache(u)
-        return self.cache.keys()
+        if resolvekeys:
+            for u in self.potentialentries.keys():
+                self._fetchandcache(u)
+            return self.cache.keys()
+        return self.potentialentries.keys()
 
 class remotenames(dict):
     """This class encapsulates all the remotenames state. It also contains