[v4] ipk/rootfs: run sanity test of multilib in parallel

From: "seungkyun.kim" <seungkyun.kim@lge.com>

For multilib type packages, there is an additional temporary
installation before the actual installation. It makes almost doubles
the do_rootfs time if having many multilib type packages.
To avoid this overhead, run sanity test in parallel.
Installing package groups through opkg takes much more time than
copying directory.

- Changes in V2:
    Fix FileNotFoundError exception when copying rootfs
- Changes in V3:
    Removed unnecessary test call
- Changes in V4:
    Fix invalid argument when create Process
    Keep the temporary rootfs directory after sanity check

Signed-off-by: seungkyun.kim <seungkyun.kim@lge.com>
---
 meta/lib/oe/package_manager/ipk/rootfs.py | 34 +++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

The code adds significant complexity, and the patch is difficult to
understand. The commit message should explain what the temporary
installation does, and why is needed. What does 'sanity test' do, and
how is it parallelized? Is it a part of that 'temporary installation'?
How is 'Installing package groups through opkg takes much more time
than copying directory.' relevant to all of that?

Also, if it does improve performance, can you please provide some
numbers for that, based on poky?

Alex

On Thu, 16 May 2024 at 02:12, Seungkyun Kim via lists.openembedded.org
<seungkyun.kim=lge.com@lists.openembedded.org> wrote:
>
> From: "seungkyun.kim" <seungkyun.kim@lge.com>
>
> For multilib type packages, there is an additional temporary
> installation before the actual installation. It makes almost doubles
> the do_rootfs time if having many multilib type packages.
> To avoid this overhead, run sanity test in parallel.
> Installing package groups through opkg takes much more time than
> copying directory.
>
> - Changes in V2:
>     Fix FileNotFoundError exception when copying rootfs
> - Changes in V3:
>     Removed unnecessary test call
> - Changes in V4:
>     Fix invalid argument when create Process
>     Keep the temporary rootfs directory after sanity check
>
> Signed-off-by: seungkyun.kim <seungkyun.kim@lge.com>
> ---
>  meta/lib/oe/package_manager/ipk/rootfs.py | 34 +++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/meta/lib/oe/package_manager/ipk/rootfs.py b/meta/lib/oe/package_manager/ipk/rootfs.py
> index ba93eb62ea..a3842a6264 100644
> --- a/meta/lib/oe/package_manager/ipk/rootfs.py
> +++ b/meta/lib/oe/package_manager/ipk/rootfs.py
> @@ -6,7 +6,9 @@
>
>  import re
>  import filecmp
> +import multiprocessing
>  import shutil
> +import stat
>  from oe.rootfs import Rootfs
>  from oe.manifest import Manifest
>  from oe.utils import execute_pre_post_process
> @@ -197,11 +199,34 @@ class PkgRootfs(DpkgOpkgRootfs):
>                          files[key] = item
>
>      def _multilib_test_install(self, pkgs):
> +        def _copy_rootfs(src, dst):
> +            if os.path.islink(src):
> +                linkto = os.readlink(src)
> +                if os.path.isabs(linkto):
> +                    linkto = os.path.normpath(os.path.join(os.path.dirname(dst),
> +                                                           os.path.relpath(linkto, src)))
> +                os.symlink(linkto, dst)
> +            elif os.path.isfile(src):
> +                shutil.copy2(src, dst)
> +            elif stat.S_ISFIFO(os.stat(src).st_mode):
> +                os.mkfifo(dst)
> +            else:
> +                bb.warn("Skip unsupported file type: %s" % src)
> +
>          ml_temp = self.d.getVar("MULTILIB_TEMP_ROOTFS")
> +        rootfs_temp = os.path.join(ml_temp, "rootfs")
>          bb.utils.mkdirhier(ml_temp)
> +        bb.utils.remove(rootfs_temp, True)
>
> -        dirs = [self.image_rootfs]
> +        if os.path.exists(self.image_rootfs):
> +            shutil.copytree(self.image_rootfs, rootfs_temp, copy_function=_copy_rootfs)
> +        else:
> +            bb.utils.mkdirhier(rootfs_temp)
> +        dirs = [rootfs_temp]
> +        return multiprocessing.Process(target=self._multilib_test_pkg_install,
> +                                       args=(pkgs, ml_temp, dirs,))
>
> +    def _multilib_test_pkg_install(self, pkgs, ml_temp, dirs):
>          for variant in self.d.getVar("MULTILIB_VARIANTS").split():
>              ml_target_rootfs = os.path.join(ml_temp, variant)
>
> @@ -300,15 +325,20 @@ class PkgRootfs(DpkgOpkgRootfs):
>
>          for pkg_type in self.install_order:
>              if pkg_type in pkgs_to_install:
> +                sanity_test = None
>                  # For multilib, we perform a sanity test before final install
>                  # If sanity test fails, it will automatically do a bb.fatal()
>                  # and the installation will stop
>                  if pkg_type == Manifest.PKG_TYPE_MULTILIB:
> -                    self._multilib_test_install(pkgs_to_install[pkg_type])
> +                    sanity_test= self._multilib_test_install(pkgs_to_install[pkg_type])
> +                    sanity_test.start()
>
>                  self.pm.install(pkgs_to_install[pkg_type],
>                                  [False, True][pkg_type == Manifest.PKG_TYPE_ATTEMPT_ONLY])
>
> +                if sanity_test is not None:
> +                    sanity_test.join()
> +
>          if self.progress_reporter:
>              self.progress_reporter.next_stage()
>
> --
> 2.34.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#199435): https://lists.openembedded.org/g/openembedded-core/message/199435
> Mute This Topic: https://lists.openembedded.org/mt/106125799/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Seungkyun Kim
> Sent: den 16 maj 2024 02:12
> To: openembedded-core@lists.openembedded.org
> Cc: seungkyun.kim <seungkyun.kim@lge.com>
> Subject: [OE-core] [PATCH v4] ipk/rootfs: run sanity test of multilib in parallel
> 
> From: "seungkyun.kim" <seungkyun.kim@lge.com>
> 
> For multilib type packages, there is an additional temporary
> installation before the actual installation. It makes almost doubles
> the do_rootfs time if having many multilib type packages.
> To avoid this overhead, run sanity test in parallel.
> Installing package groups through opkg takes much more time than
> copying directory.
> 
> - Changes in V2:
>     Fix FileNotFoundError exception when copying rootfs
> - Changes in V3:
>     Removed unnecessary test call
> - Changes in V4:
>     Fix invalid argument when create Process
>     Keep the temporary rootfs directory after sanity check

The above list of changes related to the review process should be moved to 
after the --- line below as they should not be part of the actual commit 
message.

> 
> Signed-off-by: seungkyun.kim <seungkyun.kim@lge.com>
> ---
>  meta/lib/oe/package_manager/ipk/rootfs.py | 34 +++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/lib/oe/package_manager/ipk/rootfs.py b/meta/lib/oe/package_manager/ipk/rootfs.py
> index ba93eb62ea..a3842a6264 100644
> --- a/meta/lib/oe/package_manager/ipk/rootfs.py
> +++ b/meta/lib/oe/package_manager/ipk/rootfs.py
> @@ -6,7 +6,9 @@
> 
>  import re
>  import filecmp
> +import multiprocessing
>  import shutil
> +import stat
>  from oe.rootfs import Rootfs
>  from oe.manifest import Manifest
>  from oe.utils import execute_pre_post_process
> @@ -197,11 +199,34 @@ class PkgRootfs(DpkgOpkgRootfs):
>                          files[key] = item
> 
>      def _multilib_test_install(self, pkgs):
> +        def _copy_rootfs(src, dst):
> +            if os.path.islink(src):
> +                linkto = os.readlink(src)
> +                if os.path.isabs(linkto):
> +                    linkto = os.path.normpath(os.path.join(os.path.dirname(dst),
> +                                              os.path.relpath(linkto, src)))
> +                os.symlink(linkto, dst)
> +            elif os.path.isfile(src):
> +                shutil.copy2(src, dst)
> +            elif stat.S_ISFIFO(os.stat(src).st_mode):
> +                os.mkfifo(dst)
> +            else:
> +                bb.warn("Skip unsupported file type: %s" % src)
> +
>          ml_temp = self.d.getVar("MULTILIB_TEMP_ROOTFS")
> +        rootfs_temp = os.path.join(ml_temp, "rootfs")
>          bb.utils.mkdirhier(ml_temp)
> +        bb.utils.remove(rootfs_temp, True)
> 
> -        dirs = [self.image_rootfs]
> +        if os.path.exists(self.image_rootfs):
> +            shutil.copytree(self.image_rootfs, rootfs_temp, copy_function=_copy_rootfs)
> +        else:
> +            bb.utils.mkdirhier(rootfs_temp)
> +        dirs = [rootfs_temp]
> +        return multiprocessing.Process(target=self._multilib_test_pkg_install,
> +                                       args=(pkgs, ml_temp, dirs,))
> 
> +    def _multilib_test_pkg_install(self, pkgs, ml_temp, dirs):
>          for variant in self.d.getVar("MULTILIB_VARIANTS").split():
>              ml_target_rootfs = os.path.join(ml_temp, variant)
> 
> @@ -300,15 +325,20 @@ class PkgRootfs(DpkgOpkgRootfs):
> 
>          for pkg_type in self.install_order:
>              if pkg_type in pkgs_to_install:
> +                sanity_test = None
>                  # For multilib, we perform a sanity test before final install
>                  # If sanity test fails, it will automatically do a bb.fatal()
>                  # and the installation will stop
>                  if pkg_type == Manifest.PKG_TYPE_MULTILIB:
> -                    self._multilib_test_install(pkgs_to_install[pkg_type])
> +                    sanity_test= self._multilib_test_install(pkgs_to_install[pkg_type])

Add a space before the equal sign.

> +                    sanity_test.start()
> 
>                  self.pm.install(pkgs_to_install[pkg_type],
>                                  [False, True][pkg_type == Manifest.PKG_TYPE_ATTEMPT_ONLY])
> 
> +                if sanity_test is not None:
> +                    sanity_test.join()
> +
>          if self.progress_reporter:
>              self.progress_reporter.next_stage()
> 
> --
> 2.34.1

//Peter