[PATCH v2] kbuild: remove sed command from cmd_ar_builtin

[PATCH v2] kbuild: remove sed command from cmd_ar_builtin

[Masahiro Yamada]

Replace a pipeline of echo and sed with printf to decrease process forks.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - Avoid the pipeline if there is no object to put in the archive

 scripts/Makefile.build | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index cac070aee791..784f46d41959 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -358,9 +358,8 @@ $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
 
 quiet_cmd_ar_builtin = AR      $@
       cmd_ar_builtin = rm -f $@; \
-		echo $(patsubst $(obj)/%,%,$(real-prereqs)) | \
-		sed -E 's:([^ ]+):$(obj)/\1:g' | \
-		xargs $(AR) cDPrST $@
+	$(if $(real-prereqs), printf "$(obj)/%s " $(patsubst $(obj)/%,%,$(real-prereqs)) | xargs) \
+	$(AR) cDPrST $@
 
 $(obj)/built-in.a: $(real-obj-y) FORCE
 	$(call if_changed,ar_builtin)
-- 
2.32.0

Re: [PATCH v2] kbuild: remove sed command from cmd_ar_builtin

[Nick Desaulniers]

On Mon, Jun 13, 2022 at 10:53 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Replace a pipeline of echo and sed with printf to decrease process forks.

If you're trying to minimize process forks, is it possible to remove
the use of xargs as well and just invoke $(AR) with the parameters
splatted out? I don't know myself, but maybe you're creative enough?

Otherwise,
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> Changes in v2:
>   - Avoid the pipeline if there is no object to put in the archive
>
>  scripts/Makefile.build | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index cac070aee791..784f46d41959 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -358,9 +358,8 @@ $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
>
>  quiet_cmd_ar_builtin = AR      $@
>        cmd_ar_builtin = rm -f $@; \
> -               echo $(patsubst $(obj)/%,%,$(real-prereqs)) | \
> -               sed -E 's:([^ ]+):$(obj)/\1:g' | \
> -               xargs $(AR) cDPrST $@
> +       $(if $(real-prereqs), printf "$(obj)/%s " $(patsubst $(obj)/%,%,$(real-prereqs)) | xargs) \
> +       $(AR) cDPrST $@
>
>  $(obj)/built-in.a: $(real-obj-y) FORCE
>         $(call if_changed,ar_builtin)
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2] kbuild: remove sed command from cmd_ar_builtin

[Masahiro Yamada]

On Wed, Jun 15, 2022 at 3:59 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Jun 13, 2022 at 10:53 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Replace a pipeline of echo and sed with printf to decrease process forks.
>
> If you're trying to minimize process forks, is it possible to remove
> the use of xargs as well and just invoke $(AR) with the parameters
> splatted out? I don't know myself, but maybe you're creative enough?


If I remove xargs, we will go back to the situation
before cd968b97c49214e6557381bddddacbd0e0fb696e.

This patch tries to avoid "too long argument error"
without forking too many processes.
Maybe I am too worried about the potential issue, though...






-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2] kbuild: remove sed command from cmd_ar_builtin

[Jeff Johnson]

On 6/14/2022 8:03 PM, Masahiro Yamada wrote:
> On Wed, Jun 15, 2022 at 3:59 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
>>
>> On Mon, Jun 13, 2022 at 10:53 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>>
>>> Replace a pipeline of echo and sed with printf to decrease process forks.
>>
>> If you're trying to minimize process forks, is it possible to remove
>> the use of xargs as well and just invoke $(AR) with the parameters
>> splatted out? I don't know myself, but maybe you're creative enough?
> 
> 
> If I remove xargs, we will go back to the situation
> before cd968b97c49214e6557381bddddacbd0e0fb696e.
> 
> This patch tries to avoid "too long argument error"
> without forking too many processes.
> Maybe I am too worried about the potential issue, though...

 From my very clouded perspective avoiding "too long argument error" 
should take priority :)