2022-06-14 05:59:52

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2] kbuild: remove sed command from cmd_ar_builtin

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

Signed-off-by: Masahiro Yamada <[email protected]>
---

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


2022-06-14 19:16:20

by Nick Desaulniers

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

On Mon, Jun 13, 2022 at 10:53 PM Masahiro Yamada <[email protected]> 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 <[email protected]>

>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> 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

2022-06-15 03:48:59

by Masahiro Yamada

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

On Wed, Jun 15, 2022 at 3:59 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Mon, Jun 13, 2022 at 10:53 PM Masahiro Yamada <[email protected]> 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

2022-06-15 16:48:37

by Jeff Johnson

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

On 6/14/2022 8:03 PM, Masahiro Yamada wrote:
> On Wed, Jun 15, 2022 at 3:59 AM Nick Desaulniers
> <[email protected]> wrote:
>>
>> On Mon, Jun 13, 2022 at 10:53 PM Masahiro Yamada <[email protected]> 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 :)