diff mbox series

[3/5] rteval: kcompile: Skip mrproper, and re-extract if clean fails

Message ID 20240304211655.20174-4-crwood@redhat.com
State New
Headers show
Series rteval: Fixes and speedups | expand

Commit Message

Crystal Wood March 4, 2024, 9:16 p.m. UTC
We only ever do out-of-tree builds, so the kernel directory should not be
getting damaged except by incomplete extraction (which mrproper would
probably not fix) or external meddling.  So, skip the mrproper and
instead re-extract if a "make clean" fails.

Also, add -j to cleancmd to further speed things up.  Startup speed may
not seem all that important given how long rteval is typically run for,
but this helps make quick tests (e.g. while debugging things, or when
hunting a latency that shows up very quickly) less painful.

On my 12-cpu laptop, this patch saves about 15 seconds of startup time.

Signed-off-by: Crystal Wood <crwood@redhat.com>
---
 rteval/modules/loads/kcompile.py | 40 ++++++++++++++------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

Comments

John Kacur April 2, 2024, 7:07 p.m. UTC | #1
On Mon, 4 Mar 2024, Crystal Wood wrote:

> We only ever do out-of-tree builds, so the kernel directory should not be
> getting damaged except by incomplete extraction (which mrproper would
> probably not fix) or external meddling.  So, skip the mrproper and
> instead re-extract if a "make clean" fails.
> 
> Also, add -j to cleancmd to further speed things up.  Startup speed may
> not seem all that important given how long rteval is typically run for,
> but this helps make quick tests (e.g. while debugging things, or when
> hunting a latency that shows up very quickly) less painful.
> 
> On my 12-cpu laptop, this patch saves about 15 seconds of startup time.
> 
> Signed-off-by: Crystal Wood <crwood@redhat.com>
> ---
>  rteval/modules/loads/kcompile.py | 40 ++++++++++++++------------------
>  1 file changed, 18 insertions(+), 22 deletions(-)
> 
> diff --git a/rteval/modules/loads/kcompile.py b/rteval/modules/loads/kcompile.py
> index de1539986723..db1941074157 100644
> --- a/rteval/modules/loads/kcompile.py
> +++ b/rteval/modules/loads/kcompile.py
> @@ -54,7 +54,7 @@ class KBuildJob:
>              self.jobs = self.calc_jobs_per_cpu() * cpus_available
>  
>          self.runcmd = f"make O={self.objdir} -C {self.kdir} -j{self.jobs}"
> -        self.cleancmd = f"make O={self.objdir} -C {self.kdir} clean allmodconfig"
> +        self.cleancmd = f"make O={self.objdir} -C {self.kdir} clean allmodconfig -j${self.jobs}"
>          self.cleancmd += f"&& pushd {self.objdir} && {self.kdir}/scripts/config -d CONFIG_MODULE_SIG_SHA1 -e CONFIG_MODULE_SIG_SHA512 && popd && make O={self.objdir} -C {self.kdir} olddefconfig"
>          if self.binder:
>              self.runcmd = self.binder + " " + self.runcmd
> @@ -90,8 +90,8 @@ class KBuildJob:
>      def clean(self, sin=None, sout=None, serr=None):
>          """ Runs command to clean any previous builds and configure kernel """
>          self.log(Log.DEBUG, f"cleaning objdir {self.objdir}")
> -        subprocess.call(self.cleancmd, shell=True,
> -                        stdin=sin, stdout=sout, stderr=serr)
> +        return subprocess.call(self.cleancmd, shell=True,
> +                               stdin=sin, stdout=sout, stderr=serr)
>  
>      def run(self, sin=None, sout=None, serr=None):
>          """ Use Popen to launch a kcompile job """
> @@ -234,6 +234,10 @@ class Kcompile(CommandLineLoad):
>                  self.logger, self.cpus[n] if self.cpulist else None)
>              self.args.append(str(self.buildjobs[n])+";")
>  
> +    def _repair_tarball(self):
> +        self._log(Log.DEBUG, "Invalid state in kernel build tree, reloading")
> +        self._remove_build_dirs()
> +        self._extract_tarball()
>  
>      def _WorkloadBuild(self):
>          if self._donotrun:
> @@ -246,30 +250,22 @@ class Kcompile(CommandLineLoad):
>          else:
>              out = err = null
>  
> -        # clean up any damage from previous runs
> -        try:
> -            cmd = ["make", "-C", self.mydir, "mrproper"]
> -            ret = subprocess.call(cmd, stdin=null, stdout=out, stderr=err)
> +        # clean up object dirs and make sure each has a config file
> +        for n in self.nodes:
> +            ret = self.buildjobs[n].clean(sin=null, sout=out, serr=err)
>              if ret:
> -                # if the above make failed, remove and reinstall the source tree
> -                self._log(Log.DEBUG, "Invalid state in kernel build tree, reloading")
> -                self._remove_build_dirs()
> -                self._extract_tarball()
> -                ret = subprocess.call(cmd, stdin=null, stdout=out, stderr=err)
> -                if ret:
> -                    # give up
> -                    raise rtevalRuntimeError(self, f"kcompile setup failed: {ret}")
> -        except KeyboardInterrupt as m:
> -            self._log(Log.DEBUG, "keyboard interrupt, aborting")
> -            return
> -        self._log(Log.DEBUG, "ready to run")
> +                self._repair_tarball()
> +                ret = self.buildjobs[n].clean(sin=null, sout=out, serr=err)
> +            if ret:
> +                raise rtevalRuntimeError(self, f"kcompile setup failed: {ret}")
> +
>          if self._logging:
>              os.close(out)
>              os.close(err)
> -        # clean up object dirs and make sure each has a config file
> -        for n in self.nodes:
> -            self.buildjobs[n].clean(sin=null, sout=null, serr=null)
> +
>          os.close(null)
> +
> +        self._log(Log.DEBUG, "ready to run")
>          self._setReady()
>  
>      def _WorkloadPrepare(self):
> -- 
> 2.43.0

You would probably achieve close to the same or better by just using '-j' 
with make mrproper

[jkacur@fionn tmp]$ time tar xJf ../linux-6.6.1.tar.xz 

real	0m7.224s
user	0m7.233s
sys	0m2.204s
[jkacur@fionn tmp]$ cd linux-6.6.1/
[jkacur@fionn linux-6.6.1]$ time make mrproper

real	0m14.451s
user	0m7.998s
sys	0m6.307s
[jkacur@fionn linux-6.6.1]$ time make mrproper -j

real	0m1.336s
user	0m3.473s
sys	0m2.147s

John
Crystal Wood April 3, 2024, 2:57 p.m. UTC | #2
On Tue, 2024-04-02 at 15:07 -0400, John Kacur wrote:
> 
> 
> On Mon, 4 Mar 2024, Crystal Wood wrote:
> 
> > We only ever do out-of-tree builds, so the kernel directory should not
> > be
> > getting damaged except by incomplete extraction (which mrproper would
> > probably not fix) or external meddling.  So, skip the mrproper and
> > instead re-extract if a "make clean" fails.
> > 
> > Also, add -j to cleancmd to further speed things up.  Startup speed may
> > not seem all that important given how long rteval is typically run for,
> > but this helps make quick tests (e.g. while debugging things, or when
> > hunting a latency that shows up very quickly) less painful.
> > 
> > On my 12-cpu laptop, this patch saves about 15 seconds of startup time.
> > 
> > Signed-off-by: Crystal Wood <crwood@redhat.com>

> You would probably achieve close to the same or better by just using '-j' 
> with make mrproper

Perhaps it would be almost as fast, depending on the system you're running
it on (and how many non-isolated cores it has).  But it really doesn't seem
necessary.

As for better, how?  We're going from mrproper *and* clean to just clean. 
The re-extraction only happens if clean fails.


-Crystal
John Kacur April 5, 2024, 10:11 p.m. UTC | #3
On Wed, 3 Apr 2024, Crystal Wood wrote:

> On Tue, 2024-04-02 at 15:07 -0400, John Kacur wrote:
> > 
> > 
> > On Mon, 4 Mar 2024, Crystal Wood wrote:
> > 
> > > We only ever do out-of-tree builds, so the kernel directory should not
> > > be
> > > getting damaged except by incomplete extraction (which mrproper would
> > > probably not fix) or external meddling.  So, skip the mrproper and
> > > instead re-extract if a "make clean" fails.
> > > 
> > > Also, add -j to cleancmd to further speed things up.  Startup speed may
> > > not seem all that important given how long rteval is typically run for,
> > > but this helps make quick tests (e.g. while debugging things, or when
> > > hunting a latency that shows up very quickly) less painful.
> > > 
> > > On my 12-cpu laptop, this patch saves about 15 seconds of startup time.
> > > 
> > > Signed-off-by: Crystal Wood <crwood@redhat.com>
> 
> > You would probably achieve close to the same or better by just using '-j' 
> > with make mrproper
> 
> Perhaps it would be almost as fast, depending on the system you're running
> it on (and how many non-isolated cores it has).  But it really doesn't seem
> necessary.
> 
> As for better, how?  We're going from mrproper *and* clean to just clean. 
> The re-extraction only happens if clean fails.
> 
We already reextract if we're in a state that is broken. Perhaps you're 
right and the mrproper isn't necessary, but perhaps I have a long memory 
of when we just did this by default and a lot of problems went away. 
Maybe I'm just stubborn. It seems like an "if it ain't broke don't fix it" 
problem to me, especially since your other patches already shortened the 
time for very short development runs. If you'd like to send a patch to add 
'-j' to this I would except that, otherwise, let's move on to some more 
interesting problems.

John
diff mbox series

Patch

diff --git a/rteval/modules/loads/kcompile.py b/rteval/modules/loads/kcompile.py
index de1539986723..db1941074157 100644
--- a/rteval/modules/loads/kcompile.py
+++ b/rteval/modules/loads/kcompile.py
@@ -54,7 +54,7 @@  class KBuildJob:
             self.jobs = self.calc_jobs_per_cpu() * cpus_available
 
         self.runcmd = f"make O={self.objdir} -C {self.kdir} -j{self.jobs}"
-        self.cleancmd = f"make O={self.objdir} -C {self.kdir} clean allmodconfig"
+        self.cleancmd = f"make O={self.objdir} -C {self.kdir} clean allmodconfig -j${self.jobs}"
         self.cleancmd += f"&& pushd {self.objdir} && {self.kdir}/scripts/config -d CONFIG_MODULE_SIG_SHA1 -e CONFIG_MODULE_SIG_SHA512 && popd && make O={self.objdir} -C {self.kdir} olddefconfig"
         if self.binder:
             self.runcmd = self.binder + " " + self.runcmd
@@ -90,8 +90,8 @@  class KBuildJob:
     def clean(self, sin=None, sout=None, serr=None):
         """ Runs command to clean any previous builds and configure kernel """
         self.log(Log.DEBUG, f"cleaning objdir {self.objdir}")
-        subprocess.call(self.cleancmd, shell=True,
-                        stdin=sin, stdout=sout, stderr=serr)
+        return subprocess.call(self.cleancmd, shell=True,
+                               stdin=sin, stdout=sout, stderr=serr)
 
     def run(self, sin=None, sout=None, serr=None):
         """ Use Popen to launch a kcompile job """
@@ -234,6 +234,10 @@  class Kcompile(CommandLineLoad):
                 self.logger, self.cpus[n] if self.cpulist else None)
             self.args.append(str(self.buildjobs[n])+";")
 
+    def _repair_tarball(self):
+        self._log(Log.DEBUG, "Invalid state in kernel build tree, reloading")
+        self._remove_build_dirs()
+        self._extract_tarball()
 
     def _WorkloadBuild(self):
         if self._donotrun:
@@ -246,30 +250,22 @@  class Kcompile(CommandLineLoad):
         else:
             out = err = null
 
-        # clean up any damage from previous runs
-        try:
-            cmd = ["make", "-C", self.mydir, "mrproper"]
-            ret = subprocess.call(cmd, stdin=null, stdout=out, stderr=err)
+        # clean up object dirs and make sure each has a config file
+        for n in self.nodes:
+            ret = self.buildjobs[n].clean(sin=null, sout=out, serr=err)
             if ret:
-                # if the above make failed, remove and reinstall the source tree
-                self._log(Log.DEBUG, "Invalid state in kernel build tree, reloading")
-                self._remove_build_dirs()
-                self._extract_tarball()
-                ret = subprocess.call(cmd, stdin=null, stdout=out, stderr=err)
-                if ret:
-                    # give up
-                    raise rtevalRuntimeError(self, f"kcompile setup failed: {ret}")
-        except KeyboardInterrupt as m:
-            self._log(Log.DEBUG, "keyboard interrupt, aborting")
-            return
-        self._log(Log.DEBUG, "ready to run")
+                self._repair_tarball()
+                ret = self.buildjobs[n].clean(sin=null, sout=out, serr=err)
+            if ret:
+                raise rtevalRuntimeError(self, f"kcompile setup failed: {ret}")
+
         if self._logging:
             os.close(out)
             os.close(err)
-        # clean up object dirs and make sure each has a config file
-        for n in self.nodes:
-            self.buildjobs[n].clean(sin=null, sout=null, serr=null)
+
         os.close(null)
+
+        self._log(Log.DEBUG, "ready to run")
         self._setReady()
 
     def _WorkloadPrepare(self):