core: Use of percent_defconfig seems to impact performance

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 | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Nevo, All,

On 2022-10-19 13:51 -0400, Nevo Hed 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).

We already have a pending patch to address this issue:
     https://patchwork.ozlabs.org/project/buildroot/patch/20220920194645.670432-1-yann.morin.1998@free.fr/

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

Please, no first-person sentences in commit log (i.e., no "I").

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

I can certainly see that this looks a bit simpler than my proposal.

Hoever, I am not too fond of it, because it defers the test that the
defconfig exists into the shell rather than the Makefile. I don't know,
maybe it is in fact better because we can print a more user-friendly
error message...

> 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

Weird. For me, starting at 6 external trees, the time to completion
directly exploded above the hour.

One interesting point, though is that the duration does not seem to be
much impacted by the number of bréexternal trees. Could you test with up
to 1000 trees, like I did?

Regards,
Yann E. MORIN.

> Signed-off-by: Nevo Hed <nhed+buildroot@starry.com>
> ---
>  Makefile | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index ec7c034ac1..a8298dd5fd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1010,13 +1010,12 @@ 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=$(firstword $(foreach d,\
> +		$(call reverse,$(TOPDIR) $(BR2_EXTERNAL_DIRS)),$(wildcard $(d)/configs/$@))); \
> +	if [ -z "$${defconfig}" ]; then echo "Can't find $@" >&2; exit 1; fi; \
> +	$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$${defconfig} \
> +		$< --defconfig=$${defconfig} $(CONFIG_CONFIG_IN)
>  
>  update-defconfig: savedefconfig
>  
> -- 
> 2.37.3
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

Yann, All

On Wed, Oct 19, 2022 at 2:25 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Nevo, All,
>
> On 2022-10-19 13:51 -0400, Nevo Hed 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).
>
> We already have a pending patch to address this issue:
>      https://patchwork.ozlabs.org/project/buildroot/patch/20220920194645.670432-1-yann.morin.1998@free.fr/

Sorry, I forgot to look in patchwork and just saw that it is still not
addressed in master (my actual project uses the 2022.02 tag)

> > This patch makes %_defconfig a single rule which combines uses the
> > first return of a wildcard expression.
>
> I can certainly see that this looks a bit simpler than my proposal.
>
> Hoever, I am not too fond of it, because it defers the test that the
> defconfig exists into the shell rather than the Makefile. I don't know,
> maybe it is in fact better because we can print a more user-friendly
> error message...

Agreed on that front - I was even toying using `vpath` but it didn't pan out
(probably because I have not used that feature in a long time).

> > 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
>
> Weird. For me, starting at 6 external trees, the time to completion
> directly exploded above the hour.
>
> One interesting point, though is that the duration does not seem to be
> much impacted by the number of bréexternal trees. Could you test with up
> to 1000 trees, like I did?

For 1000, I got 15.68 - so the same neighborhood as yours.
Maybe deltas in the 6-7 trees might be hardware related (cache sizes,
FS types etc - strace-ing the make in the many-trees case is a wild
spam storm)

Addendum:
I originally prepared a cover letter but am not a frequent user of
git-send-email and it didn't send.  I'll paste it below - please note
the option of some implicit rule resets.


===========================================================

From 6a2fa74edf5bd3b254fc7c49ebe465280e8e8902 Mon Sep 17 00:00:00 2001
From: Nevo Hed <nhed+buildroot@starry.com>
Date: Wed, 19 Oct 2022 13:32:14 -0400
Subject: [PATCH 0/1] core: Use of percent_defconfig seems to impact performance


Noticing serious perforate 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 an implosion of implicit rules
being evaluated.

Adding the resets as suggested here
https://lists.gnu.org/archive/html/help-make/2002-11/msg00062.html seems 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<------------