[Buildroot] [PATCH v2 0/1] core: Use of percent_defconfig seems to impact performance

[Buildroot] [PATCH v2 0/1] core: Use of percent_defconfig seems to impact performance

[Nevo Hed via buildroot]

Noticing serious performance issues on `make X_defconfig` with growth of

number of external trees.  At 7 trees it is somewhat tolerable but at 8
trees we have seen it take 80-110 seconds.

Numbers for before and after the change in commit itself.

I do not know for sure what the underlying issue is but looking at some
`strace` and `make -d` output it feels like implosion of implicit rules
being evaluated.

Note that after I submitted v1 of this patch it was pointed out to me that
a different patch is already in the works.  But since that neither that
patch nor mine had any seen any movement in months I decided to take a
stab at addressing the feedback I received from Yann on my patch:

> ... it defers the test that the defconfig exists into the shell
> rather than the Makefile

Adding the resets as suggested here
https://lists.gnu.org/archive/html/help-make/2002-11/msg00062.html seem to
help a lot but not as much as the attached patch.

--8<------------
%: %,v
%: RCS/%,v
%: RCS/%
%: s.%
%: SCCS/s.%
--8<------------

Inlined below a script to repro this issue, not sure if this functionality
should be integrated anywhere or just left in the mailing list.  By
default the script runs the scenario from 1 tree to 8 trees.  Running with
param (1-10) will allow a single run with that many trees.


--8<------------
#!/usr/bin/env bash

declare treedirs=()

declare -r cfg="pc_x86_64_bios_defconfig"

function setup_dir {
    for tree in tree{1..10}; do
        local treedir="defconf_test/${tree}"

        mkdir -p "${treedir}/configs"
        touch "${treedir}/Config.in"
        touch "${treedir}/external.mk"
        echo -e "name: ${tree^^}\ndesc: ${tree^}" > "${treedir}/external.desc"

        cp "./configs/${cfg}" "${treedir}/configs/${tree}_${cfg}"

        treedirs+=( "${treedir}" )
    done
}

function run_one {
    local count="${1}"
    local br2_external=$(IFS=':'; echo "${treedirs[*]:0:count}")
    echo "Defconfig with ${count} trees (BR2_EXTERNAL=\"${br2_external}\")"
    /usr/bin/time -f "${count},%e" /bin/make BR2_EXTERNAL="${br2_external}" "${cfg}"
}

# Main
setup_dir
if [ -n "${1}" ]; then
    run_one "${1}"
else
    for i in {1..8}; do
        run_one $i
    done
fi
--8<------------

Changes v1 -> v2:
  - Testing for defconfig existence and error when failing to find it is
    now done at the Makefile level rather than the shell (utilizing `$(or
    ...` and `$(error ...`).

Nevo Hed (1):
  core: Use of percent_defconfig seems to impact performance

 Makefile | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

-- 
2.37.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/1] core: Use of percent_defconfig seems to impact performance

[Nevo Hed via buildroot]

Noticed a significant slowdown with rise of number of external trees
in our env.  This slowdown seemed to be related to invocations if the
percent_defconfig function (GNU Make 4.3).

While I have not do a deep dive in analyzing the performance issue, it
felt like redefining the %_defconfig rule N times impact performance.

This patch makes %_defconfig a single rule which combines uses the
first return of a wildcard expression.

Timing (seconds) of `make pc_x86_64_bios_defconfig` with 1-8 external
trees:

    #Trees    Before    After
         1      0.38     0.37
         2      0.32     0.31
         3      0.31     0.33
         4      0.36     0.32
         5      0.45     0.35
         6      1.26     0.36
         7      9.10     0.36
         8     85.93     0.42

Signed-off-by: Nevo Hed <nhed+buildroot@starry.com>
---
 Makefile | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 88f90cd2fa..ad866f1f24 100644
--- a/Makefile
+++ b/Makefile
@@ -1013,13 +1013,14 @@ oldconfig syncconfig olddefconfig: $(BUILD_DIR)/buildroot-config/conf outputmake
 defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@$(COMMON_CONFIG_ENV) $< --defconfig$(if $(DEFCONFIG),=$(DEFCONFIG)) $(CONFIG_CONFIG_IN)
 
-define percent_defconfig
-# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig
-%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
-	@$$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(1)/configs/$$@ \
-		$$< --defconfig=$(1)/configs/$$@ $$(CONFIG_CONFIG_IN)
-endef
-$(eval $(foreach d,$(call reverse,$(TOPDIR) $(BR2_EXTERNAL_DIRS)),$(call percent_defconfig,$(d))$(sep)))
+%_defconfig: $(BUILD_DIR)/buildroot-config/conf  outputmakefile
+	@defconfig=$(or \
+		$(firstword $(foreach d, \
+			$(call reverse,$(TOPDIR) $(BR2_EXTERNAL_DIRS)),$(wildcard $(d)/configs/$@))), \
+		$(error "Can't find $@") \
+	); \
+	$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$${defconfig} \
+		$< --defconfig=$${defconfig} $(CONFIG_CONFIG_IN)
 
 update-defconfig: savedefconfig
 
-- 
2.38.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] core: Use of percent_defconfig seems to impact performance

[Yann E. MORIN]

Nevo, All,

On 2023-01-04 20:57 -0500, Nevo Hed via buildroot spake thusly:
> Noticed a significant slowdown with rise of number of external trees
> in our env.  This slowdown seemed to be related to invocations if the
> percent_defconfig function (GNU Make 4.3).
> 
> While I have not do a deep dive in analyzing the performance issue, it
> felt like redefining the %_defconfig rule N times impact performance.
> 
> This patch makes %_defconfig a single rule which combines uses the
> first return of a wildcard expression.
> 
> Timing (seconds) of `make pc_x86_64_bios_defconfig` with 1-8 external
> trees:
> 
>     #Trees    Before    After
>          1      0.38     0.37
>          2      0.32     0.31
>          3      0.31     0.33
>          4      0.36     0.32
>          5      0.45     0.35
>          6      1.26     0.36
>          7      9.10     0.36
>          8     85.93     0.42
> 
> Signed-off-by: Nevo Hed <nhed+buildroot@starry.com>

Thanks for the respin, I like it better than your first iteration, and
most importantly, better than my own implementation!

I however used most of my commit log, a=exteneded it even further,
provided my reproducibility script in the commit log and redid the
timings up to 1000 trees (heck, it's manageable now, so why not!).

I also slightly reformatted the code, to split the long lines.

Finally, I also added David as the reporter, and that his bug #14996
is now fixed.

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  Makefile | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 88f90cd2fa..ad866f1f24 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1013,13 +1013,14 @@ oldconfig syncconfig olddefconfig: $(BUILD_DIR)/buildroot-config/conf outputmake
>  defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
>  	@$(COMMON_CONFIG_ENV) $< --defconfig$(if $(DEFCONFIG),=$(DEFCONFIG)) $(CONFIG_CONFIG_IN)
>  
> -define percent_defconfig
> -# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig
> -%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
> -	@$$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(1)/configs/$$@ \
> -		$$< --defconfig=$(1)/configs/$$@ $$(CONFIG_CONFIG_IN)
> -endef
> -$(eval $(foreach d,$(call reverse,$(TOPDIR) $(BR2_EXTERNAL_DIRS)),$(call percent_defconfig,$(d))$(sep)))
> +%_defconfig: $(BUILD_DIR)/buildroot-config/conf  outputmakefile
> +	@defconfig=$(or \
> +		$(firstword $(foreach d, \
> +			$(call reverse,$(TOPDIR) $(BR2_EXTERNAL_DIRS)),$(wildcard $(d)/configs/$@))), \
> +		$(error "Can't find $@") \
> +	); \
> +	$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$${defconfig} \
> +		$< --defconfig=$${defconfig} $(CONFIG_CONFIG_IN)
>  
>  update-defconfig: savedefconfig
>  
> -- 
> 2.38.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] core: Use of percent_defconfig seems to impact performance

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Nevo, All,
 > On 2023-01-04 20:57 -0500, Nevo Hed via buildroot spake thusly:
 >> Noticed a significant slowdown with rise of number of external trees
 >> in our env.  This slowdown seemed to be related to invocations if the
 >> percent_defconfig function (GNU Make 4.3).
 >> 
 >> While I have not do a deep dive in analyzing the performance issue, it
 >> felt like redefining the %_defconfig rule N times impact performance.
 >> 
 >> This patch makes %_defconfig a single rule which combines uses the
 >> first return of a wildcard expression.
 >> 
 >> Timing (seconds) of `make pc_x86_64_bios_defconfig` with 1-8 external
 >> trees:
 >> 
 >> #Trees    Before    After
 >> 1      0.38     0.37
 >> 2      0.32     0.31
 >> 3      0.31     0.33
 >> 4      0.36     0.32
 >> 5      0.45     0.35
 >> 6      1.26     0.36
 >> 7      9.10     0.36
 >> 8     85.93     0.42
 >> 
 >> Signed-off-by: Nevo Hed <nhed+buildroot@starry.com>

 > Thanks for the respin, I like it better than your first iteration, and
 > most importantly, better than my own implementation!

 > I however used most of my commit log, a=exteneded it even further,
 > provided my reproducibility script in the commit log and redid the
 > timings up to 1000 trees (heck, it's manageable now, so why not!).

 > I also slightly reformatted the code, to split the long lines.

 > Finally, I also added David as the reporter, and that his bug #14996
 > is now fixed.

 > Applied to master, thanks.

Committed to 2022.11.x and 2022.02.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] core: Use of percent_defconfig seems to impact performance

[Yann E. MORIN]

Nevo, All,

+Romain who reported an issue
+Peter the backport to stable branches

On 2023-01-04 20:57 -0500, Nevo Hed via buildroot spake thusly:
> Noticed a significant slowdown with rise of number of external trees
> in our env.  This slowdown seemed to be related to invocations if the
> percent_defconfig function (GNU Make 4.3).
n        
> Signed-off-by: Nevo Hed <nhed+buildroot@starry.com>
> ---
>  Makefile | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 88f90cd2fa..ad866f1f24 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1013,13 +1013,14 @@ oldconfig syncconfig olddefconfig: $(BUILD_DIR)/buildroot-config/conf outputmake
>  defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
>  	@$(COMMON_CONFIG_ENV) $< --defconfig$(if $(DEFCONFIG),=$(DEFCONFIG)) $(CONFIG_CONFIG_IN)
>  
> -define percent_defconfig
> -# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig

So, the following change:

> -%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
[--SNIP--]
> +%_defconfig: $(BUILD_DIR)/buildroot-config/conf  outputmakefile

broke the build when the output directory ends with _defconfig. For
example:

    $ make O=/some/path/meh_defconfig meh_defconfig
    $ make O=/some/path/meh_defconfig

will break toward the end, with a rather weird issue:

    Makefile:1015: *** "Can't find /some/path/meh_defconfig".  Stop.

What looked weird, is two fold;

 1. only the defconfig name should be reported, not the path
 2. it happens after the filesystems are built

Tracing the build provides some clues;

    $ make -d O=/some/path/meh_defconfig
    [...]
          Considering target file 'staging-finalize'.
           File 'staging-finalize' does not exist.
            Considering target file '/some/path/meh_defconfig/staging'.
             File '/some/path/meh_defconfig/staging' does not exist.
              Considering target file '/some/path/meh_defconfig'.
               Looking for an implicit rule for '/some/path/meh_defconfig'.
               Trying pattern rule with stem 'meh'.
               Trying rule prerequisite '/some/path/meh_defconfig/build/buildroot-config/conf'.
               Trying rule prerequisite 'outputmakefile'.
               Found an implicit rule for '/some/path/meh_defconfig'.
                Considering target file '/some/path/meh_defconfig/build/buildroot-config/conf'.
                 Looking for an implicit rule for '/some/path/meh_defconfig/build/buildroot-config/conf'.
                 Trying pattern rule with stem 'c'.
                 Found an implicit rule for '/some/path/meh_defconfig/build/buildroot-config/conf'.
                 Finished prerequisites of target file '/some/path/meh_defconfig/build/buildroot-config/conf'.
                No need to remake target '/some/path/meh_defconfig/build/buildroot-config/conf'.
                Considering target file 'outputmakefile'.
                 File 'outputmakefile' does not exist.
                 Finished prerequisites of target file 'outputmakefile'.
                Must remake target 'outputmakefile'.
    /home/ymorin/dev/buildroot/buildroot/master/support/scripts/mkmakefile /home/ymorin/dev/buildroot/buildroot/master /some/path/meh_defconfig
    Putting child 0x55d4f3385ab0 (outputmakefile) PID 1231466 on the chain.
    Live child 0x55d4f3385ab0 (outputmakefile) PID 1231466
      GEN     /some/path/meh_defconfig/Makefile
    Reaping winning child 0x55d4f3385ab0 PID 1231466
    Removing child 0x55d4f3385ab0 PID 1231466 from chain.
                Successfully remade target file 'outputmakefile'.
               Finished prerequisites of target file '/some/path/meh_defconfig'.
               Prerequisite '/some/path/meh_defconfig/build/buildroot-config/conf' is older than target '/home/ymorin/dev/buildroot/buildroot/master/test/meh_defconfig'.
               Prerequisite 'outputmakefile' of target '/some/path/meh_defconfig' does not exist.
              Must remake target '/some/path/meh_defconfig'.
    Makefile:1015: *** "Can't find /some/path/meh_defconfig".  Stop.

So, it happens when trying to do staging-finalize. And this is becasue
we have (elided and reordered):

    staging-finalize: $(STAGING_DIR_SYMLINK)
    $(STAGING_DIR_SYMLINK): | $(BASE_DIR)
    BASE_DIR := $(CANONICAL_O)
    CANONICAL_O := $(shell mkdir -p $(O) >/dev/null 2>&1)$(realpath $(O))

So, at some point, we need to have something to generate
/some/path/meh_defconfig, and thus the %_defconfig rule kick in.

The rule is %_defconfig, and the stem is now /some/path/meh, while the
way we wrote the rule, it expects a plain name (i.e. without any '/').

And of course, there is no defconfig named '/some/path/meh_defconfig',
so the error path kicks in and boom.

But then, why did it not trigger before?

Thats because, before, the defconfig rule was something like:

    %_defconfig: blabla foo/%_defconfig blabla

So the stem was on both side of the rule, and it would not kick in
because it was not an "implicit rule" (Makefile is not trivial, so maybe
I use the wrong terminology here), and because there would be an actual
file with that stem in its name.

So, I think we have two options:

  1. quickly find a fix that is not totally hackish (but with Romain, we
     could not find something trivial that worked)
  2. revert this change, and resurect my alternate patch [0]

[0] https://lore.kernel.org/buildroot/20220920194645.670432-1-yann.morin.1998@free.fr/

Regards,
Yann E. MORIN.

> +	@defconfig=$(or \
> +		$(firstword $(foreach d, \
> +			$(call reverse,$(TOPDIR) $(BR2_EXTERNAL_DIRS)),$(wildcard $(d)/configs/$@))), \
> +		$(error "Can't find $@") \
> +	); \
> +	$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$${defconfig} \
> +		$< --defconfig=$${defconfig} $(CONFIG_CONFIG_IN)
>  
>  update-defconfig: savedefconfig
>  
> -- 
> 2.38.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] core: Use of percent_defconfig seems to impact performance

[Yann E. MORIN]

Nevo, All,

On 2023-01-21 23:27 +0100, Yann E. MORIN spake thusly:
> On 2023-01-04 20:57 -0500, Nevo Hed via buildroot spake thusly:
> > Noticed a significant slowdown with rise of number of external trees
> > in our env.  This slowdown seemed to be related to invocations if the
> > percent_defconfig function (GNU Make 4.3).
> So, the following change:
> > -%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
> [--SNIP--]
> > +%_defconfig: $(BUILD_DIR)/buildroot-config/conf  outputmakefile
> broke the build when the output directory ends with _defconfig. For
> example:
>     Makefile:1015: *** "Can't find /some/path/meh_defconfig".  Stop.
[--SNIP--]
> So, I think we have two options:
>   1. quickly find a fix that is not totally hackish (but with Romain, we
>      could not find something trivial that worked)

Hopefully I've found a non-hackish fix:

    https://lore.kernel.org/buildroot/20230122090947.1369673-1-yann.morin.1998@free.fr/T/#u

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot