[PATCH] find-debuginfo: Pass -j down to dwz

[PATCH] find-debuginfo: Pass -j down to dwz

[Kalev Lember]

Now that dwz 0.15 supports parallel jobs, add a way to control it from
here. find-debuginfo already has a -j parameter so we can just extend it
and pass the value down to dwz as well.

This should fix building large packages on memory constrained builders,
such as webkitgtk on s390x in Fedora koji build system, where we can now
use the -j option to tune down parallelism to avoid running out of
memory during dwz run.

Signed-off-by: Kalev Lember <klember@redhat.com>
---
 scripts/find-debuginfo.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
index b07a52f..8cc1bfd 100755
--- a/scripts/find-debuginfo.in
+++ b/scripts/find-debuginfo.in
@@ -585,7 +585,7 @@ if $run_dwz \
       dwz_multifile_suffix=".${dwz_multifile_idx}"
     done
     dwz_multifile_name="${dwz_multifile_name}${dwz_multifile_suffix}"
-    dwz_opts="-h -q -r"
+    dwz_opts="-h -q -r -j ${n_jobs}"
     [ ${#dwz_files[@]} -gt 1 ] && [ "$dwz_single_file_mode" = "false" ] \
       && dwz_opts="${dwz_opts} -m .dwz/${dwz_multifile_name}"
     mkdir -p "${RPM_BUILD_ROOT}/usr/lib/debug/.dwz"
-- 
2.39.0

Re: [PATCH] find-debuginfo: Pass -j down to dwz

[Panu Matilainen]

On 1/19/23 18:03, Kalev Lember wrote:
> Now that dwz 0.15 supports parallel jobs, add a way to control it from
> here. find-debuginfo already has a -j parameter so we can just extend it
> and pass the value down to dwz as well.
> 
> This should fix building large packages on memory constrained builders,
> such as webkitgtk on s390x in Fedora koji build system, where we can now
> use the -j option to tune down parallelism to avoid running out of
> memory during dwz run.
> 
> Signed-off-by: Kalev Lember <klember@redhat.com>
> ---
>   scripts/find-debuginfo.in | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
> index b07a52f..8cc1bfd 100755
> --- a/scripts/find-debuginfo.in
> +++ b/scripts/find-debuginfo.in
> @@ -585,7 +585,7 @@ if $run_dwz \
>         dwz_multifile_suffix=".${dwz_multifile_idx}"
>       done
>       dwz_multifile_name="${dwz_multifile_name}${dwz_multifile_suffix}"
> -    dwz_opts="-h -q -r"
> +    dwz_opts="-h -q -r -j ${n_jobs}"
>       [ ${#dwz_files[@]} -gt 1 ] && [ "$dwz_single_file_mode" = "false" ] \
>         && dwz_opts="${dwz_opts} -m .dwz/${dwz_multifile_name}"
>       mkdir -p "${RPM_BUILD_ROOT}/usr/lib/debug/.dwz"

This is so new, even Fedora 37 doesn't have it yet. I tend to think 
find-debuginfo should be a bit conservative about the assumptions it 
makes about external tools.

	- Panu -

Re: [PATCH] find-debuginfo: Pass -j down to dwz

[Kalev Lember]

[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]

On Fri, Jan 20, 2023 at 9:40 AM Panu Matilainen <pmatilai@redhat.com> wrote:

> On 1/19/23 18:03, Kalev Lember wrote:
> > Now that dwz 0.15 supports parallel jobs, add a way to control it from
> > here. find-debuginfo already has a -j parameter so we can just extend it
> > and pass the value down to dwz as well.
> >
> > This should fix building large packages on memory constrained builders,
> > such as webkitgtk on s390x in Fedora koji build system, where we can now
> > use the -j option to tune down parallelism to avoid running out of
> > memory during dwz run.
> >
> > Signed-off-by: Kalev Lember <klember@redhat.com>
> > ---
> >   scripts/find-debuginfo.in | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
> > index b07a52f..8cc1bfd 100755
> > --- a/scripts/find-debuginfo.in
> > +++ b/scripts/find-debuginfo.in
> > @@ -585,7 +585,7 @@ if $run_dwz \
> >         dwz_multifile_suffix=".${dwz_multifile_idx}"
> >       done
> >       dwz_multifile_name="${dwz_multifile_name}${dwz_multifile_suffix}"
> > -    dwz_opts="-h -q -r"
> > +    dwz_opts="-h -q -r -j ${n_jobs}"
> >       [ ${#dwz_files[@]} -gt 1 ] && [ "$dwz_single_file_mode" = "false"
> ] \
> >         && dwz_opts="${dwz_opts} -m .dwz/${dwz_multifile_name}"
> >       mkdir -p "${RPM_BUILD_ROOT}/usr/lib/debug/.dwz"
>
> This is so new, even Fedora 37 doesn't have it yet. I tend to think
> find-debuginfo should be a bit conservative about the assumptions it
> makes about external tools.
>

Can we do some kind of configure check then and require the latest dwz? Or
add this as a downstream patch in rawhide?

The problem is that new dwz is already in rawhide and it broke webkitgtk
builds due to too much memory use. The ppc64le and s390x builders just
don't have enough memory now to do dwz on webkitgtk. Passing down the -j
option would help us control the memory use to get the builds going again.

Meanwhile we worked this around by completely disabling the dwz
optimization for webkitgtk, but it would be nice to get it back to working.

See
https://src.fedoraproject.org/rpms/webkitgtk/c/2c5dd84529070a8cfb0c6599dee587ca8fbcb6d2?branch=rawhide

-- 
Thanks,
Kalev

Re: [PATCH] find-debuginfo: Pass -j down to dwz

[Panu Matilainen]

On 1/20/23 12:27, Kalev Lember wrote:
> On Fri, Jan 20, 2023 at 9:40 AM Panu Matilainen <pmatilai@redhat.com 
> <mailto:pmatilai@redhat.com>> wrote:
> 
>     On 1/19/23 18:03, Kalev Lember wrote:
>      > Now that dwz 0.15 supports parallel jobs, add a way to control it
>     from
>      > here. find-debuginfo already has a -j parameter so we can just
>     extend it
>      > and pass the value down to dwz as well.
>      >
>      > This should fix building large packages on memory constrained
>     builders,
>      > such as webkitgtk on s390x in Fedora koji build system, where we
>     can now
>      > use the -j option to tune down parallelism to avoid running out of
>      > memory during dwz run.
>      >
>      > Signed-off-by: Kalev Lember <klember@redhat.com
>     <mailto:klember@redhat.com>>
>      > ---
>      >   scripts/find-debuginfo.in <http://find-debuginfo.in> | 2 +-
>      >   1 file changed, 1 insertion(+), 1 deletion(-)
>      >
>      > diff --git a/scripts/find-debuginfo.in <http://find-debuginfo.in>
>     b/scripts/find-debuginfo.in <http://find-debuginfo.in>
>      > index b07a52f..8cc1bfd 100755
>      > --- a/scripts/find-debuginfo.in <http://find-debuginfo.in>
>      > +++ b/scripts/find-debuginfo.in <http://find-debuginfo.in>
>      > @@ -585,7 +585,7 @@ if $run_dwz \
>      >         dwz_multifile_suffix=".${dwz_multifile_idx}"
>      >       done
>      >     
>       dwz_multifile_name="${dwz_multifile_name}${dwz_multifile_suffix}"
>      > -    dwz_opts="-h -q -r"
>      > +    dwz_opts="-h -q -r -j ${n_jobs}"
>      >       [ ${#dwz_files[@]} -gt 1 ] && [ "$dwz_single_file_mode" =
>     "false" ] \
>      >         && dwz_opts="${dwz_opts} -m .dwz/${dwz_multifile_name}"
>      >       mkdir -p "${RPM_BUILD_ROOT}/usr/lib/debug/.dwz"
> 
>     This is so new, even Fedora 37 doesn't have it yet. I tend to think
>     find-debuginfo should be a bit conservative about the assumptions it
>     makes about external tools.
> 
> 
> Can we do some kind of configure check then and require the latest dwz? 
> Or add this as a downstream patch in rawhide?
> 
> The problem is that new dwz is already in rawhide and it broke webkitgtk 
> builds due to too much memory use. The ppc64le and s390x builders just 
> don't have enough memory now to do dwz on webkitgtk. Passing down the -j 
> option would help us control the memory use to get the builds going again.
> 

Oh, if rawhide needs this then rawhide needs this, I don't mind that at 
all. I just think that upstream find-debuginfo shouldn't assume "latest 
everything" any more than rpm can do so.

configure-time check seems reasonable to me.

	- Panu -

Re: [PATCH] find-debuginfo: Pass -j down to dwz

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

Hi Kalev,

On Thu, 2023-01-19 at 17:03 +0100, Kalev Lember wrote:
> Now that dwz 0.15 supports parallel jobs, add a way to control it from
> here. find-debuginfo already has a -j parameter so we can just extend it
> and pass the value down to dwz as well.
> 
> This should fix building large packages on memory constrained builders,
> such as webkitgtk on s390x in Fedora koji build system, where we can now
> use the -j option to tune down parallelism to avoid running out of
> memory during dwz run.

Aha, you want to reduce the default parallelism (processors / 2)?

> diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
> index b07a52f..8cc1bfd 100755
> --- a/scripts/find-debuginfo.in
> +++ b/scripts/find-debuginfo.in
> @@ -585,7 +585,7 @@ if $run_dwz \
>        dwz_multifile_suffix=".${dwz_multifile_idx}"
>      done
>      dwz_multifile_name="${dwz_multifile_name}${dwz_multifile_suffix}"
> -    dwz_opts="-h -q -r"
> +    dwz_opts="-h -q -r -j ${n_jobs}"
>      [ ${#dwz_files[@]} -gt 1 ] && [ "$dwz_single_file_mode" = "false" ] \
>        && dwz_opts="${dwz_opts} -m .dwz/${dwz_multifile_name}"
>      mkdir -p "${RPM_BUILD_ROOT}/usr/lib/debug/.dwz"

I agree with Panu that a configure check would be nice to make sure the
installed dwz does support -j. Would the attached patch work for you?

Thanks,

Mark

[-- Attachment #2: dwz_j.patch --]
[-- Type: text/x-patch, Size: 2047 bytes --]

diff --git a/Makefile.am b/Makefile.am
index 2060b96..4a5092d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -33,7 +33,8 @@ CLEANFILES = $(bin_SCRIPTS)
 # Some standard substitutions for scripts
 do_subst = ($(SED) -e 's,[@]PACKAGE[@],$(PACKAGE),g' \
 		   -e 's,[@]VERSION[@],$(VERSION),g' \
-		   -e 's,[@]READELF[@],$(READELF),g')
+		   -e 's,[@]READELF[@],$(READELF),g' \
+		   -e 's,[@]DWZ_J[@],$(DWZ_J),g')
 
 find-debuginfo: $(top_srcdir)/scripts/find-debuginfo.in Makefile
 	$(do_subst) < "$(top_srcdir)/scripts/$@.in" > "$@"
diff --git a/configure.ac b/configure.ac
index 6a53365..f2d1571 100644
--- a/configure.ac
+++ b/configure.ac
@@ -47,6 +47,27 @@ AC_CHECK_TOOL([LD], [ld])
 AC_CHECK_TOOL([READELF], [readelf])
 AM_MISSING_PROG(HELP2MAN, help2man)
 
+# Whether dwz support -j.
+# Make sure to compile something with -g.
+# Run dwz on it with -j1.
+DWZ_J=""
+AC_CHECK_PROG([DWZ], [dwz], [dwz])
+if test "x$DWZ" = "xdwz"; then
+  save_CFLAGS="$CFLAGS"
+  CFLAGS="$save_CFLAGS -g"
+  AC_CACHE_CHECK([whether the dwz support -j], ac_cv_dwz_j, [dnl
+    AC_LINK_IFELSE([AC_LANG_PROGRAM()],[dnl
+      ac_cv_dwz_j=yes; ${DWZ} -j1 conftest$EXEEXT 2>/dev/null || ac_cv_dwz_j=no],
+      AC_MSG_FAILURE([unexpected compile failure]))])
+  if test "x$ac_cv_dwz_j" = "xyes"; then
+    DWZ_J="-j"
+  fi
+  CFLAGS="$save_CFLAGS"
+else
+  AC_MSG_WARN([dwz not installed])
+fi
+AC_SUBST([DWZ_J])
+
 # Only really an issue on 32bit platforms. Makes sure we'll get large off_t.
 AC_SYS_LARGEFILE
 
diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
index b07a52f..8090c84 100755
--- a/scripts/find-debuginfo.in
+++ b/scripts/find-debuginfo.in
@@ -586,6 +586,7 @@ if $run_dwz \
     done
     dwz_multifile_name="${dwz_multifile_name}${dwz_multifile_suffix}"
     dwz_opts="-h -q -r"
+    [ -n "@DWZ_J@" ] && dwz_opts="${dwz_opts} -j ${n_jobs}"
     [ ${#dwz_files[@]} -gt 1 ] && [ "$dwz_single_file_mode" = "false" ] \
       && dwz_opts="${dwz_opts} -m .dwz/${dwz_multifile_name}"
     mkdir -p "${RPM_BUILD_ROOT}/usr/lib/debug/.dwz"

Re: [PATCH] find-debuginfo: Pass -j down to dwz

[Kalev Lember]

[-- Attachment #1: Type: text/plain, Size: 1419 bytes --]

On Thu, Jan 26, 2023 at 3:34 PM Mark Wielaard <mark@klomp.org> wrote:

> Hi Kalev,
>

Hi Mark,


> Aha, you want to reduce the default parallelism (processors / 2)?
>

Yes, exactly!

We have the following in webkitgtk, which basically just passes -j1 on low
memory builders:

# Require 32 GB of RAM per vCPU for debuginfo processing. 16 GB is not
enough.
%global _find_debuginfo_opts %limit_build -m 32768


> diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
> > index b07a52f..8cc1bfd 100755
> > --- a/scripts/find-debuginfo.in
> > +++ b/scripts/find-debuginfo.in
> > @@ -585,7 +585,7 @@ if $run_dwz \
> >        dwz_multifile_suffix=".${dwz_multifile_idx}"
> >      done
> >      dwz_multifile_name="${dwz_multifile_name}${dwz_multifile_suffix}"
> > -    dwz_opts="-h -q -r"
> > +    dwz_opts="-h -q -r -j ${n_jobs}"
> >      [ ${#dwz_files[@]} -gt 1 ] && [ "$dwz_single_file_mode" = "false" ]
> \
> >        && dwz_opts="${dwz_opts} -m .dwz/${dwz_multifile_name}"
> >      mkdir -p "${RPM_BUILD_ROOT}/usr/lib/debug/.dwz"
>
> I agree with Panu that a configure check would be nice to make sure the
> installed dwz does support -j. Would the attached patch work for you?
>

That looks perfect to me. Thanks a lot!

Can you also apply it to the rawhide package, please? That way I can
confirm if it actually works on actual koji builders and see if we can get
webkitgtk building right.

Thanks,
Kalev

Re: [PATCH] find-debuginfo: Pass -j down to dwz

[Mark Wielaard]

Hi Kalev,

On Thu, Jan 26, 2023 at 04:57:18PM +0100, Kalev Lember wrote:
> > I agree with Panu that a configure check would be nice to make sure the
> > installed dwz does support -j. Would the attached patch work for you?
> >
> 
> That looks perfect to me. Thanks a lot!

Pushed as:

commit 5b23e464528ef988cfcd0a87b3ec8db0520db867
Author: Kalev Lember <klember@redhat.com>
Date:   Thu Jan 19 17:03:18 2023 +0100

    find-debuginfo: Pass -j down to dwz
    
    Now that dwz 0.15 supports parallel jobs, add a way to control it from
    here. find-debuginfo already has a -j parameter so we can just extend it
    and pass the value down to dwz as well.
    
    This should fix building large packages on memory constrained builders,
    such as webkitgtk on s390x in Fedora koji build system, where we can now
    use the -j option to tune down parallelism to avoid running out of
    memory during dwz run.
    
    Add a configure check to make sure the installed dwz supports the
    -j option.
    
    Signed-off-by: Kalev Lember <klember@redhat.com>
    Signed-off-by: Mark Wielaard <mark@klomp.org>
 
> Can you also apply it to the rawhide package, please? That way I can
> confirm if it actually works on actual koji builders and see if we can get
> webkitgtk building right.

I will, but as you might have seen the rawhide buildbot just failed.
All others seem green, so it probably is the new gcc or some compiler
flag changes in fedora rawhide:
https://builder.sourceware.org/buildbot/#/changes/17730

As soon as I figure out how to get the testsuite green on rawhide I'll
also push this patch there.

Cheers,

Mark

Re: [PATCH] find-debuginfo: Pass -j down to dwz

[Mark Wielaard]

Hi Kalev,

On Fri, 2023-01-27 at 00:14 +0100, Mark Wielaard wrote:
> > Can you also apply it to the rawhide package, please? That way I can
> > confirm if it actually works on actual koji builders and see if we can get
> > webkitgtk building right.
> 
> I will, but as you might have seen the rawhide buildbot just failed.
> All others seem green, so it probably is the new gcc or some compiler
> flag changes in fedora rawhide:
> https://builder.sourceware.org/buildbot/#/changes/17730
> 
> As soon as I figure out how to get the testsuite green on rawhide I'll
> also push this patch there.

Figured it out. It was actually a bug in gcc 13.0 prerelease. Committed
a workaround (and a fix for the workaround, sigh) and ported that and
the dwz -j patch to the Fedora rawhide package debugedit-5.0-7.fc38 so
you can test it out there.

Cheers,

Mark

Re: [PATCH] find-debuginfo: Pass -j down to dwz

[Kalev Lember]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

Hi Mark,

On Fri, Jan 27, 2023 at 6:37 PM Mark Wielaard <mark@klomp.org> wrote:

> Figured it out. It was actually a bug in gcc 13.0 prerelease. Committed
> a workaround (and a fix for the workaround, sigh) and ported that and
> the dwz -j patch to the Fedora rawhide package debugedit-5.0-7.fc38 so
> you can test it out there.
>

Thank you! I did a webkitgtk scratch build and it looks like the fix works
as expected.

-- 
Kalev