Patchwork [1,of,2] run-tests: cache hghave results

login
register
mail settings
Submitter Matt Harbison
Date Feb. 26, 2018, 3:04 a.m.
Message ID <e373776c800cffaf4793.1519614283@Envy>
Download mbox | patch
Permalink /patch/28355/
State Accepted
Headers show

Comments

Matt Harbison - Feb. 26, 2018, 3:04 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1519597345 18000
#      Sun Feb 25 17:22:25 2018 -0500
# Node ID e373776c800cffaf47934cf688307f5ce8e85d17
# Parent  3bdd64ed150679886ee87341ab3fcdda4f23ffc7
run-tests: cache hghave results

Spawning a process on Windows is expensive.  I've got a version of
test-lfs-test-server.t locally which prints the http request/responses that
totals 819 lines, with 149 conditional lines, 11 #if tests, and 2 test cases.
It takes just under 1 minute with this change to run both cases, vs just over
2 minutes without this change.  Worse, when I explored adding ui.debug to the
test, it takes 13 minutes due to all of the mismatches and retests, vs less than
1 minute with this change.  Overall, the difference when running all tests is
negligible- 103 minutes with this change, vs 105 without when using -j9.

`hghave` effectively ANDs the requirements, so for a list of requirements that
test successfully, they can all be individually cached.  The 'no-' prefixed
stuff can maybe be inverted (though it wants stdout for the False case).  It
might be possible to cache this at a higher level instead of per test, but this
is where the hghave() function lives.

It also looks like an exit value of 2 from `hghave` is treated specially, but
there's nothing preventing 2 missing features from also using this value.
Yuya Nishihara - Feb. 26, 2018, 1:28 p.m.
On Sun, 25 Feb 2018 22:04:43 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1519597345 18000
> #      Sun Feb 25 17:22:25 2018 -0500
> # Node ID e373776c800cffaf47934cf688307f5ce8e85d17
> # Parent  3bdd64ed150679886ee87341ab3fcdda4f23ffc7
> run-tests: cache hghave results
> 
> Spawning a process on Windows is expensive.  I've got a version of
> test-lfs-test-server.t locally which prints the http request/responses that
> totals 819 lines, with 149 conditional lines, 11 #if tests, and 2 test cases.
> It takes just under 1 minute with this change to run both cases, vs just over
> 2 minutes without this change.  Worse, when I explored adding ui.debug to the
> test, it takes 13 minutes due to all of the mismatches and retests, vs less than
> 1 minute with this change.  Overall, the difference when running all tests is
> negligible- 103 minutes with this change, vs 105 without when using -j9.
> 
> `hghave` effectively ANDs the requirements, so for a list of requirements that
> test successfully, they can all be individually cached.  The 'no-' prefixed
> stuff can maybe be inverted (though it wants stdout for the False case).  It
> might be possible to cache this at a higher level instead of per test, but this
> is where the hghave() function lives.
> 
> It also looks like an exit value of 2 from `hghave` is treated specially, but
> there's nothing preventing 2 missing features from also using this value.
> 
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -1236,6 +1236,7 @@
>              self.name = '%s (case %s)' % (self.name, _strpath(case))
>              self.errpath = b'%s.%s.err' % (self.errpath[:-4], case)
>              self._tmpname += b'-%s' % case
> +        self._have = {}
>  
>      @property
>      def refpath(self):
> @@ -1275,6 +1276,18 @@
>          return self._processoutput(exitcode, output, salt, after, expected)
>  
>      def _hghave(self, reqs):
> +        if len(reqs) == 1:
> +            # Fastpath
> +            if reqs[0] in self._have:
> +                return self._have.get(reqs[0])
> +        elif all(r in self._have for r in reqs):
> +            for r in reqs:
> +                status, stdout = self._have.get(r)
> +                if not status:
> +                    break
> +            else:
> +                return True, None

Can't we simply cache a result per req? I think len(reqs) == 1 in most case,
so the penalty of spawning hghave per req would be negligible.

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -1236,6 +1236,7 @@ 
             self.name = '%s (case %s)' % (self.name, _strpath(case))
             self.errpath = b'%s.%s.err' % (self.errpath[:-4], case)
             self._tmpname += b'-%s' % case
+        self._have = {}
 
     @property
     def refpath(self):
@@ -1275,6 +1276,18 @@ 
         return self._processoutput(exitcode, output, salt, after, expected)
 
     def _hghave(self, reqs):
+        if len(reqs) == 1:
+            # Fastpath
+            if reqs[0] in self._have:
+                return self._have.get(reqs[0])
+        elif all(r in self._have for r in reqs):
+            for r in reqs:
+                status, stdout = self._have.get(r)
+                if not status:
+                    break
+            else:
+                return True, None
+
         # TODO do something smarter when all other uses of hghave are gone.
         runtestdir = os.path.abspath(os.path.dirname(_bytespath(__file__)))
         tdir = runtestdir.replace(b'\\', b'/')
@@ -1290,10 +1303,18 @@ 
             sys.exit(1)
 
         if ret != 0:
+            # Don't bother trying to figure out which requirement in a list was
+            # the failure.  But a single failure can be cached.
+            if len(reqs) == 1:
+                self._have[reqs[0]] = (False, stdout)
             return False, stdout
 
         if b'slow' in reqs:
             self._timeout = self._slowtimeout
+
+        for r in reqs:
+            self._have[r] = (True, None)
+
         return True, None
 
     def _iftest(self, args):