[Buildroot] [PATCH] core: add rule to dump packages' build order

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Yann E. MORIN]

When debugging hidden dependencies, the build order is very important.
Most notably, it is interesting to identify potential culprits.

Add a new top-level rule, show-biuld-order, that dumps all the packages
in the order they would get built.

Note that there are a few differences with show-targets:

  - more packages are reported, becasue show-targets does not report
    host packages that have no prompt;

  - the output is line-based, because we're using $(info $(1)); getting
    a single output line like show-targets would require we use an
    actual command, like printf '%s ' $(1); but that takes a lot of
    time, while $(info $(1)) is almost instantaneous (the time to parse
    the Makefiles);

  - rootfs targets are not reported.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Peter Korsgaard <peter@korsgaard.com>
---
 Makefile               | 2 ++
 package/pkg-generic.mk | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 941bf78..919d589 100644
--- a/Makefile
+++ b/Makefile
@@ -757,6 +757,8 @@ legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p
 show-targets:
 	@echo $(PACKAGES) $(TARGETS_ROOTFS)
 
+show-build-order: $(patsubst %,%-show-build-order,$(PACKAGES))
+
 graph-build: $(O)/build/build-time.log
 	@install -d $(GRAPHS_DIR)
 	$(foreach o,name build duration,./support/scripts/graph-build-time \
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 31dbc54..3b26e6b 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -737,6 +737,9 @@ $(1)-show-depends:
 $(1)-show-rdepends:
 			@echo $$($(2)_RDEPENDENCIES)
 
+$(1)-show-build-order: $$(patsubst %,%-show-build-order,$$($(2)_FINAL_ALL_DEPENDENCIES))
+	$$(info $(1))
+
 $(1)-graph-depends: graph-depends-requirements
 	$(call pkg-graph-depends,$(1),--direct)
 
-- 
2.9.3

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Arnout Vandecappelle]

On 02-04-17 15:03, Yann E. MORIN wrote:
> When debugging hidden dependencies, the build order is very important.
> Most notably, it is interesting to identify potential culprits.
> 
> Add a new top-level rule, show-biuld-order, that dumps all the packages
> in the order they would get built.
> 
> Note that there are a few differences with show-targets:
> 
>   - more packages are reported, becasue show-targets does not report
>     host packages that have no prompt;
> 
>   - the output is line-based, because we're using $(info $(1)); getting
>     a single output line like show-targets would require we use an
>     actual command, like printf '%s ' $(1); but that takes a lot of
>     time, while $(info $(1)) is almost instantaneous (the time to parse
>     the Makefiles);
> 
>   - rootfs targets are not reported.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> ---
>  Makefile               | 2 ++
>  package/pkg-generic.mk | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 941bf78..919d589 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -757,6 +757,8 @@ legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p
>  show-targets:
>  	@echo $(PACKAGES) $(TARGETS_ROOTFS)
>  
> +show-build-order: $(patsubst %,%-show-build-order,$(PACKAGES))
> +
>  graph-build: $(O)/build/build-time.log
>  	@install -d $(GRAPHS_DIR)
>  	$(foreach o,name build duration,./support/scripts/graph-build-time \
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 31dbc54..3b26e6b 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -737,6 +737,9 @@ $(1)-show-depends:
>  $(1)-show-rdepends:
>  			@echo $$($(2)_RDEPENDENCIES)
>  
> +$(1)-show-build-order: $$(patsubst %,%-show-build-order,$$($(2)_FINAL_ALL_DEPENDENCIES))
> +	$$(info $(1))
> +

 I don't like this...

 IMO we already have way too many of these informational targets messing up
pkg-generic.mk. This is bad for two reasons:

- it makes the code more complicated, more difficult to understand what's going
on int pkg-generic.mk;

- it's bad for performance. This adds 2000 rules to the make rules database,
which makes the dependency resolution slower (even if this rule is not used).
Just this one extra rule doesn't make much of a difference, but it's all these
extra rules that we've added over the years combined that make it slower. That
said, there is probably some refactoring that could be done to make it less bad,
so it's not too big of a factor.


 But I wonder why you need this feature anyway. If you just build, you see the
build order as well, right? And if you don't want to see the actual build, you
can do 'make -nk'. OK, it looks a lot nicer with this show-build-order target,
but I wouldn't say it's very valuable.

 Regards,
 Arnout


>  $(1)-graph-depends: graph-depends-requirements
>  	$(call pkg-graph-depends,$(1),--direct)
>  
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Yann E. MORIN]

Arnout, All,

On 2017-04-03 12:10 +0200, Arnout Vandecappelle spake thusly:
> On 02-04-17 15:03, Yann E. MORIN wrote:
> > When debugging hidden dependencies, the build order is very important.
> > Most notably, it is interesting to identify potential culprits.
> > 
> > Add a new top-level rule, show-biuld-order, that dumps all the packages
> > in the order they would get built.
> > 
> > Note that there are a few differences with show-targets:
> > 
> >   - more packages are reported, becasue show-targets does not report
> >     host packages that have no prompt;
> > 
> >   - the output is line-based, because we're using $(info $(1)); getting
> >     a single output line like show-targets would require we use an
> >     actual command, like printf '%s ' $(1); but that takes a lot of
> >     time, while $(info $(1)) is almost instantaneous (the time to parse
> >     the Makefiles);
> > 
> >   - rootfs targets are not reported.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > Cc: Peter Korsgaard <peter@korsgaard.com>
> > ---
> >  Makefile               | 2 ++
> >  package/pkg-generic.mk | 3 +++
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/Makefile b/Makefile
> > index 941bf78..919d589 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -757,6 +757,8 @@ legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p
> >  show-targets:
> >  	@echo $(PACKAGES) $(TARGETS_ROOTFS)
> >  
> > +show-build-order: $(patsubst %,%-show-build-order,$(PACKAGES))
> > +
> >  graph-build: $(O)/build/build-time.log
> >  	@install -d $(GRAPHS_DIR)
> >  	$(foreach o,name build duration,./support/scripts/graph-build-time \
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 31dbc54..3b26e6b 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -737,6 +737,9 @@ $(1)-show-depends:
> >  $(1)-show-rdepends:
> >  			@echo $$($(2)_RDEPENDENCIES)
> >  
> > +$(1)-show-build-order: $$(patsubst %,%-show-build-order,$$($(2)_FINAL_ALL_DEPENDENCIES))
> > +	$$(info $(1))
> > +
> 
>  I don't like this...
> 
>  IMO we already have way too many of these informational targets messing up
> pkg-generic.mk. This is bad for two reasons:
> 
> - it makes the code more complicated, more difficult to understand what's going
> on int pkg-generic.mk;
> 
> - it's bad for performance. This adds 2000 rules to the make rules database,
> which makes the dependency resolution slower (even if this rule is not used).
> Just this one extra rule doesn't make much of a difference, but it's all these
> extra rules that we've added over the years combined that make it slower. That
> said, there is probably some refactoring that could be done to make it less bad,
> so it's not too big of a factor.

Timing the run before/after this change does not yield a noticeable
difference, tested with:

    $ for((i=0;i<50;i++)); do
        /usr/bin/time make CMD 2>&1 >/dev/null |head -n 1
    done

with CMD = help, before and after the patch, and then with
CMD = show-build-order (obviously, after the patch), and using the
'user' part of the timings:


                    Help, before    Help, after     Build order
Mean                2.9612          2.9934          2.9716
Standard Error      0.00393456685   0.00505972492   0.00338496073
Mode                2.96            2.98            2.96
Median              2.96            2.98            2.97
First Quartile      2.94            2.97            2.96
Third Quartile      2.97            3.01            2.98
Variance            0.00077404081   0.00128004081   0.00057289795
Standard Deviation  0.02782158903   0.03577765806   0.02393528690
Kurtosis            0.84170374930   2.10366593785   1.32436717983
Skewness            0.95495029123   1.20563022139   0.44501404736
Range               0.12            0.19            0.13
Minimum             2.92            2.93            2.91
Maximum             3.04            3.12            3.04
Sum                 148.06          149.67          148.58
Count               50              50              50

Hmm... ;-)

[ BTW, thanks libreoffice! ;-) ]

>  But I wonder why you need this feature anyway. If you just build, you see the
> build order as well, right?

Oh yeah, and I pay the price of a three-hour build, jsut to see the
build order...

> And if you don't want to see the actual build, you
> can do 'make -nk'.

I hope you were kidding? For a config with 227 packages, this yields:

    $ make -nk 2>&1 |wc -l
    8459

And I'm not doing the statistical analysis I did above, but a few runs
give a runtime of ~5s. So it is a 66% overhead just for this, while the
new rules add just about 1% overhead (30ms for 3s).

Plus, it requires non-trivial post-processing... :-(

> OK, it looks a lot nicer with this show-build-order target,
> but I wouldn't say it's very valuable.

Well, I've been tracking build-order issues for a while last WE, and
believe me, I was very happy to be able to see the build order before
I attempted a build, yes.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Arnout Vandecappelle]

On 04-04-17 20:59, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2017-04-03 12:10 +0200, Arnout Vandecappelle spake thusly:
>> On 02-04-17 15:03, Yann E. MORIN wrote:
[snip]
>>> +$(1)-show-build-order: $$(patsubst %,%-show-build-order,$$($(2)_FINAL_ALL_DEPENDENCIES))
>>> +	$$(info $(1))
>>> +
>>
>>  I don't like this...
>>
>>  IMO we already have way too many of these informational targets messing up
>> pkg-generic.mk. This is bad for two reasons:
>>
>> - it makes the code more complicated, more difficult to understand what's going
>> on int pkg-generic.mk;
>>
>> - it's bad for performance. This adds 2000 rules to the make rules database,
>> which makes the dependency resolution slower (even if this rule is not used).
>> Just this one extra rule doesn't make much of a difference, but it's all these
>> extra rules that we've added over the years combined that make it slower. That
>> said, there is probably some refactoring that could be done to make it less bad,
>> so it's not too big of a factor.
> 
> Timing the run before/after this change does not yield a noticeable
> difference

 As I wrote: "Just this one extra rule doesn't make much of a difference".
However, we have about a dozen rules like this, and added together they *do*
make a noticable difference (I did a much less thorough benchmark; after
removing all these one-shot rules that are not used in a normal build, a 'make
-qp >/dev/null' became about 15% faster).



> , tested with:
> 
>     $ for((i=0;i<50;i++)); do
>         /usr/bin/time make CMD 2>&1 >/dev/null |head -n 1

 What I'm interested in is the overhead of running make. We unfortunately don't
have a good target for that. 'make -q' after finishing a build is a nice
approximation because it doesn't actually run the target-finalize step (as
opposed to just 'make'). I usually use 'make -qp' because that's what's used by
bash-completion and that is one of my big annoyances: accidentally typing
TAB-TAB freezes the shell for almost 10 seconds.

 'make help' isn't a bad approximation either. It doesn't evaluate the .stamp_*
files so it will be a bit faster than the real overhead.


 Now, I repeat: adding this single thing is not a big deal. I just say we have
to be careful about adding things all the time that slow things down. We are
very quickly approaching the speed of bitbake, and that is one of the main
reasons I spit out OE the first time I used it...

>     done
> 
> with CMD = help, before and after the patch, and then with
> CMD = show-build-order (obviously, after the patch), and using the
> 'user' part of the timings:
> 
> 
>                     Help, before    Help, after     Build order
> Mean                2.9612          2.9934          2.9716

 So, 1% slower. That's actually worse than I expected, but consistent with the
15% I measured after removing 15 similar targets.

[snip]
>>  But I wonder why you need this feature anyway. If you just build, you see the
>> build order as well, right?
> 
> Oh yeah, and I pay the price of a three-hour build, jsut to see the
> build order...
> 
>> And if you don't want to see the actual build, you
>> can do 'make -nk'.
> 
> I hope you were kidding? For a config with 227 packages, this yields:
> 
>     $ make -nk 2>&1 |wc -l
>     8459
> 
> And I'm not doing the statistical analysis I did above, but a few runs
> give a runtime of ~5s. So it is a 66% overhead just for this, while the
> new rules add just about 1% overhead (30ms for 3s).
> 
> Plus, it requires non-trivial post-processing... :-(

TERM=dumb make -nk 2>/dev/null | grep 'Configuring"' | cut -d ' ' -f 3-4

 True, it's not trivial. But it's shorter than your patch :-) And indeed it's 4
times slower than 'make build-order'. But that's just 12 seconds for an
allyesconfig. Is that really too much?

 However, it turns out not to work :-( Any rule which contains $(MAKE) is still
going to be executed, and that will inevitably lead to errors, so any package
that depends on a package that errors out will also not be executed.

 So, I can think of no viable alternative, so there is no way to stop this patch :-)


>> OK, it looks a lot nicer with this show-build-order target,
>> but I wouldn't say it's very valuable.
> 
> Well, I've been tracking build-order issues for a while last WE, and
> believe me, I was very happy to be able to see the build order before
> I attempted a build, yes.

 That's my biggest problem with this feature: I have no idea how to use it.
Could you add some explanation somewhere?

 Regards,
 Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Thomas Petazzoni]

Hello,

On Fri, 7 Apr 2017 12:30:46 +0200, Arnout Vandecappelle wrote:

> > Well, I've been tracking build-order issues for a while last WE, and
> > believe me, I was very happy to be able to see the build order before
> > I attempted a build, yes.  
> 
>  That's my biggest problem with this feature: I have no idea how to use it.
> Could you add some explanation somewhere?

Yann was tracking down the dc3dd failure. With some configurations it
was building, with others not. So he was trying to figure which package
that gets built before dc3dd causes the problem. For this, it was
really nice to be able to load a configuration, and immediately see in
which order packages would be built, and therefore which packages would
be built before dc3dd, without having to do the build itself.

Does that explain better the use case for this?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Arnout Vandecappelle]

On 07-04-17 12:43, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 7 Apr 2017 12:30:46 +0200, Arnout Vandecappelle wrote:
> 
>>> Well, I've been tracking build-order issues for a while last WE, and
>>> believe me, I was very happy to be able to see the build order before
>>> I attempted a build, yes.  
>>
>>  That's my biggest problem with this feature: I have no idea how to use it.
>> Could you add some explanation somewhere?
> 
> Yann was tracking down the dc3dd failure. With some configurations it
> was building, with others not. So he was trying to figure which package
> that gets built before dc3dd causes the problem. For this, it was
> really nice to be able to load a configuration, and immediately see in
> which order packages would be built, and therefore which packages would
> be built before dc3dd, without having to do the build itself.
> 
> Does that explain better the use case for this?

 Ack. Could you add that to the commit message while applying?

 Regards,
 Arnout

> 
> Best regards,
> 
> Thomas
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Yann E. MORIN]

Arnout, All,

On 2017-04-07 12:30 +0200, Arnout Vandecappelle spake thusly:
> On 04-04-17 20:59, Yann E. MORIN wrote:
> >> - it's bad for performance. This adds 2000 rules to the make rules database,
> >> which makes the dependency resolution slower (even if this rule is not used).
> >> Just this one extra rule doesn't make much of a difference, but it's all these
> >> extra rules that we've added over the years combined that make it slower. That
> >> said, there is probably some refactoring that could be done to make it less bad,
> >> so it's not too big of a factor.
> > 
> > Timing the run before/after this change does not yield a noticeable
> > difference
> 
>  As I wrote: "Just this one extra rule doesn't make much of a difference".
> However, we have about a dozen rules like this, and added together they *do*
> make a noticable difference (I did a much less thorough benchmark; after
> removing all these one-shot rules that are not used in a normal build, a 'make
> -qp >/dev/null' became about 15% faster).

OK, adding features makes the stuff slower; that I do understand.

However, what I argue is that the overhead added by that one extra rule
is negligible, relatively to the existing startup time.

> > , tested with:
> > 
> >     $ for((i=0;i<50;i++)); do
> >         /usr/bin/time make CMD 2>&1 >/dev/null |head -n 1
> 
>  What I'm interested in is the overhead of running make. We unfortunately don't
> have a good target for that. 'make -q' after finishing a build is a nice
> approximation because it doesn't actually run the target-finalize step (as
> opposed to just 'make'). I usually use 'make -qp' because that's what's used by
> bash-completion and that is one of my big annoyances: accidentally typing
> TAB-TAB freezes the shell for almost 10 seconds.

~6s here... Yes, it is a bit annoying, when one _accidentally_ hit
TAB-TAB. Accientatlly... ;-)

>  'make help' isn't a bad approximation either. It doesn't evaluate the .stamp_*
> files so it will be a bit faster than the real overhead.

To be clear: I too find the startup time really annoying, especially
when it is so much longer thatn the actual comamnd, like:

    make foo-patch

for a trivial package, takes more time during the startup that to do the
actual extract and patch, even from a cache-cold tree.

>  Now, I repeat: adding this single thing is not a big deal. I just say we have
> to be careful about adding things all the time that slow things down. We are
> very quickly approaching the speed of bitbake, and that is one of the main
> reasons I spit out OE the first time I used it...

Eh... The startup tiem is annoying, but it is not awfull. Yet. Howeber,
in my (arguably limited) experience of OE, the build time was much
longer overall for a similar setup. So we still win AFAICS...

The only thing that would be a killer feature is TLPB (as long as it
yields a correct build, of course).

> >> And if you don't want to see the actual build, you
> >> can do 'make -nk'.
[--SNIP--]
> > Plus, it requires non-trivial post-processing... :-(
> TERM=dumb make -nk 2>/dev/null | grep 'Configuring"' | cut -d ' ' -f 3-4
> 
>  True, it's not trivial. But it's shorter than your patch :-) And indeed it's 4
> times slower than 'make build-order'. But that's just 12 seconds for an
> allyesconfig. Is that really too much?
> 
>  However, it turns out not to work :-( Any rule which contains $(MAKE) is still
> going to be executed, and that will inevitably lead to errors, so any package
> that depends on a package that errors out will also not be executed.
> 
>  So, I can think of no viable alternative, so there is no way to stop this patch :-)

What about the following (just proof-of-concept):

diff --git a/Makefile b/Makefile
index 919d589..a1540fc 100644
--- a/Makefile
+++ b/Makefile
@@ -759,6 +759,14 @@ show-targets:
 
 show-build-order: $(patsubst %,%-show-build-order,$(PACKAGES))
 
+define show-build-order-deps
+$(foreach p,$($(call UP    PERCASE,$(1))_FINAL_ALL_DEPENDENCIES),\
+$(call show-build-order-deps       -deps,$(p))) $(1)
+endef
+
+show-build-order-2:
+ at ./toto $(foreach p,$  (PACKAGES),$(call
show-build-order-deps-deps,$(p)))
+
 graph-build: $(O)/build/build-time.log
 	@install -d $(GRAPHS_DIR)
 	$(foreach o,name build duration,./support/scripts/graph-build-time \
diff --git a/toto b/toto
new file mode 100755
index 0000000..6962083
--- /dev/null
+++ b/toto
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+# input on stdin: list of packages, each rpreceded by its
+# direct dependencies; so, packages may be listed more than
+# once, but at least the build order is gurranteed for the
+# first occurence of each package.
+
+# We first output each item of the list on its own line.
+# Then we number those lisnee.
+# We sort by package name as first key, and on line number
+# as second key.
+# For each package, we keep only the first occurence, which
+# is the one with the lowest line number.
+# We re-sort on the line number.
+# And eventually, we remove the line number and only keep
+# the package name.
+
+# The output is thus the build order.
+
+printf "%s\n" "${@}"
+|cat -n \
+|sort -k 2,2 -k 1,1n \
+|while read n p; do
+    if [ "${p}" != "${prev}" ]; then
+        printf "%d %s\n" "${n}" "${p}"
+        prev="${p}"
+    fi
+done \
+|sort -n \
+|sed -r -e 's/^[[:digit:]]+ //'

This has the advantage that it adds a single rule, and the price of
expansion is paid only when the rule is actually called.

Plus, the runtime is not worse than my initial solution.

> >> OK, it looks a lot nicer with this show-build-order target,
> >> but I wouldn't say it's very valuable.
> > 
> > Well, I've been tracking build-order issues for a while last WE, and
> > believe me, I was very happy to be able to see the build order before
> > I attempted a build, yes.
> 
>  That's my biggest problem with this feature: I have no idea how to use it.
> Could you add some explanation somewhere?

I hope the explanations from Thomas are enough? ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Yann E. MORIN]

Arnout, All,

On 2017-04-07 21:24 +0200, Yann E. MORIN spake thusly:
> >  So, I can think of no viable alternative, so there is no way to stop this patch :-)
> What about the following (just proof-of-concept):

Hmmm, it seems there was a bad copy-paste failure.

Here it is again:

diff --git a/Makefile b/Makefile
index 919d589..a1540fc 100644
--- a/Makefile
+++ b/Makefile
@@ -759,6 +759,14 @@ show-targets:

 show-build-order: $(patsubst %,%-show-build-order,$(PACKAGES))

+define show-build-order-deps
+	$(foreach p,$($(call UPPERCASE,$(1))_FINAL_ALL_DEPENDENCIES),\
+		$(call show-build-order-deps-deps,$(p))) $(1)
+endef
+
+show-build-order-2:
+	@./toto $(foreach p,$(PACKAGES),$(call show-build-order-deps-deps,$(p)))
+
 graph-build: $(O)/build/build-time.log
 	@install -d $(GRAPHS_DIR)
 	$(foreach o,name build duration,./support/scripts/graph-build-time \
diff --git a/toto b/toto
new file mode 100755
index 0000000..6962083
--- /dev/null
+++ b/toto
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+# input on stdin: list of packages, each rpreceded by its
+# direct dependencies; so, packages may be listed more than
+# once, but at least the build order is gurranteed for the
+# first occurence of each package.
+
+# We first output each item of the list on its own line.
+# Then we number those lines.
+# We sort by package name as first key, and on line number
+# as second key.
+# For each package, we keep only the first occurence, which
+# is the one with the lowest line number.
+# We re-sort on the line number.
+# And eventually, we remove the line number and only keep
+# the package name.
+
+# The output is thus the build order.
+
+printf "%s\n" "${@}"
+|cat -n \
+|sort -k 2,2 -k 1,1n \
+|while read n p; do
+    if [ "${p}" != "${prev}" ]; then
+        printf "%d %s\n" "${n}" "${p}"
+        prev="${p}"
+    fi
+done \
+|sort -n \
+|sed -r -e 's/^[[:digit:]]+ //'


-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Arnout Vandecappelle]

On 07-04-17 21:24, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2017-04-07 12:30 +0200, Arnout Vandecappelle spake thusly:
>> On 04-04-17 20:59, Yann E. MORIN wrote:
>>>> - it's bad for performance. This adds 2000 rules to the make rules database,
>>>> which makes the dependency resolution slower (even if this rule is not used).
>>>> Just this one extra rule doesn't make much of a difference, but it's all these
>>>> extra rules that we've added over the years combined that make it slower. That
>>>> said, there is probably some refactoring that could be done to make it less bad,
>>>> so it's not too big of a factor.
>>>
>>> Timing the run before/after this change does not yield a noticeable
>>> difference
>>
>>  As I wrote: "Just this one extra rule doesn't make much of a difference".
>> However, we have about a dozen rules like this, and added together they *do*
>> make a noticable difference (I did a much less thorough benchmark; after
>> removing all these one-shot rules that are not used in a normal build, a 'make
>> -qp >/dev/null' became about 15% faster).
> 
> OK, adding features makes the stuff slower; that I do understand.
> 
> However, what I argue is that the overhead added by that one extra rule
> is negligible, relatively to the existing startup time.

 Indeed, which is why I gave the patch my Reviewed-by.

 My point is: we should be conservative about adding new per-package rules like
this. A single addition has negligible effect, but added together they do have
an impact and they're turning us into bitbake.
[snip]
>>  Now, I repeat: adding this single thing is not a big deal. I just say we have
>> to be careful about adding things all the time that slow things down. We are
>> very quickly approaching the speed of bitbake, and that is one of the main
>> reasons I spit out OE the first time I used it...
> 
> Eh... The startup tiem is annoying, but it is not awfull. Yet. Howeber,
> in my (arguably limited) experience of OE, the build time was much
> longer overall for a similar setup. So we still win AFAICS...
> 
> The only thing that would be a killer feature is TLPB (as long as it
> yields a correct build, of course).

 In my experience, there are two big reasons why an OE build is much slower:

1. A typical configuration is much bigger (more packages selected).

2. They do per-package staging in order to enable TLPB.

 We still win on point 1, but since we're going towards PPS (and host and target
too), we're going to loose our advantage on point 2. Well, not entirely, because
IIUC they have a pretty inefficient way of building the PPS so we might do better.


> 
>>>> And if you don't want to see the actual build, you
>>>> can do 'make -nk'.
> [--SNIP--]
>>> Plus, it requires non-trivial post-processing... :-(
>> TERM=dumb make -nk 2>/dev/null | grep 'Configuring"' | cut -d ' ' -f 3-4
>>
>>  True, it's not trivial. But it's shorter than your patch :-) And indeed it's 4
>> times slower than 'make build-order'. But that's just 12 seconds for an
>> allyesconfig. Is that really too much?
>>
>>  However, it turns out not to work :-( Any rule which contains $(MAKE) is still
>> going to be executed, and that will inevitably lead to errors, so any package
>> that depends on a package that errors out will also not be executed.
>>
>>  So, I can think of no viable alternative, so there is no way to stop this patch :-)
> 
> What about the following (just proof-of-concept):
[pasted correct version]
> diff --git a/Makefile b/Makefile
> index 919d589..a1540fc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -759,6 +759,14 @@ show-targets:
> 
>  show-build-order: $(patsubst %,%-show-build-order,$(PACKAGES))
> 
> +define show-build-order-deps
> +	$(foreach p,$($(call UPPERCASE,$(1))_FINAL_ALL_DEPENDENCIES),\
> +		$(call show-build-order-deps-deps,$(p))) $(1)
> +endef
> +
> +show-build-order-2:
> +	@./toto $(foreach p,$(PACKAGES),$(call show-build-order-deps-deps,$(p)))
> +
>  graph-build: $(O)/build/build-time.log
>  	@install -d $(GRAPHS_DIR)
>  	$(foreach o,name build duration,./support/scripts/graph-build-time \
> diff --git a/toto b/toto
> new file mode 100755
> index 0000000..6962083
> --- /dev/null
> +++ b/toto
> @@ -0,0 +1,30 @@
> +#!/bin/bash
> +
> +# input on stdin: list of packages, each rpreceded by its
> +# direct dependencies; so, packages may be listed more than
> +# once, but at least the build order is gurranteed for the
> +# first occurence of each package.
> +
> +# We first output each item of the list on its own line.
> +# Then we number those lines.
> +# We sort by package name as first key, and on line number
> +# as second key.
> +# For each package, we keep only the first occurence, which
> +# is the one with the lowest line number.
> +# We re-sort on the line number.
> +# And eventually, we remove the line number and only keep
> +# the package name.
> +
> +# The output is thus the build order.
> +
> +printf "%s\n" "${@}"
> +|cat -n \
> +|sort -k 2,2 -k 1,1n \
> +|while read n p; do
> +    if [ "${p}" != "${prev}" ]; then
> +        printf "%d %s\n" "${n}" "${p}"
> +        prev="${p}"
> +    fi
> +done \
> +|sort -n \
> +|sed -r -e 's/^[[:digit:]]+ //'

 OK, my first and main reason to reject the patch was that it made the code more
complicated. That bit is even worse (by an order of magnitude) in this proposal.

> 
> This has the advantage that it adds a single rule, and the price of
> expansion is paid only when the rule is actually called.
> 
> Plus, the runtime is not worse than my initial solution.
> 
>>>> OK, it looks a lot nicer with this show-build-order target,
>>>> but I wouldn't say it's very valuable.
>>>
>>> Well, I've been tracking build-order issues for a while last WE, and
>>> believe me, I was very happy to be able to see the build order before
>>> I attempted a build, yes.
>>
>>  That's my biggest problem with this feature: I have no idea how to use it.
>> Could you add some explanation somewhere?
> 
> I hope the explanations from Thomas are enough? ;-)

 Yep, as I replied to him: please add that to the commit message while applying.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Thomas Petazzoni]

Hello,

On Mon, 10 Apr 2017 11:28:57 +0200, Arnout Vandecappelle wrote:

>  In my experience, there are two big reasons why an OE build is much slower:
> 
> 1. A typical configuration is much bigger (more packages selected).
> 
> 2. They do per-package staging in order to enable TLPB.

This point (2) is in fact not correct, at least according to what I
understood. My colleague Alexandre can confirm, but apparently,
per-package staging has only been very recently introduced in OE. And
yes, top-level parallel build was causing some spurious build failures.

Alex, if you have more details, we're interested :)

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Alexandre Belloni]

Hi,

On 10/04/2017 at 13:44:40 +0200, Thomas Petazzoni wrote:
> On Mon, 10 Apr 2017 11:28:57 +0200, Arnout Vandecappelle wrote:
> 
> >  In my experience, there are two big reasons why an OE build is much slower:
> > 
> > 1. A typical configuration is much bigger (more packages selected).
> > 
> > 2. They do per-package staging in order to enable TLPB.
> 
> This point (2) is in fact not correct, at least according to what I
> understood. My colleague Alexandre can confirm, but apparently,
> per-package staging has only been very recently introduced in OE. And
> yes, top-level parallel build was causing some spurious build failures.
> 
> Alex, if you have more details, we're interested :)
> 

I'd point to
http://cgit.openembedded.org/openembedded-core/commit/?id=809746f56df4b91af014bf6a3f28997d6698ac78

It adds RSS (recipe specific sysroots) applied in january. Some issues
are still being sorted out but it generally builds fine.

Regarding build time, I can point to that one which interestingly
concludes that it doesn't add much to the build time (or more correctly
that a simple optimisation made up for all the lost time):
http://cgit.openembedded.org/openembedded-core/commit/?id=e02716ca29b744fde5a45dabe79fef11df772d92


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Arnout Vandecappelle]

On 02-04-17 15:03, Yann E. MORIN wrote:
> When debugging hidden dependencies, the build order is very important.
> Most notably, it is interesting to identify potential culprits.
> 
> Add a new top-level rule, show-biuld-order, that dumps all the packages
                                 build
> in the order they would get built.
> 
> Note that there are a few differences with show-targets:
> 
>   - more packages are reported, becasue show-targets does not report
                                  because
>     host packages that have no prompt;
> 
>   - the output is line-based, because we're using $(info $(1)); getting
>     a single output line like show-targets would require we use an
>     actual command, like printf '%s ' $(1); but that takes a lot of
>     time, while $(info $(1)) is almost instantaneous (the time to parse
>     the Makefiles);
> 
>   - rootfs targets are not reported.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> ---
>  Makefile               | 2 ++
>  package/pkg-generic.mk | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 941bf78..919d589 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -757,6 +757,8 @@ legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p
>  show-targets:
>  	@echo $(PACKAGES) $(TARGETS_ROOTFS)
>  
> +show-build-order: $(patsubst %,%-show-build-order,$(PACKAGES))
> +
>  graph-build: $(O)/build/build-time.log
>  	@install -d $(GRAPHS_DIR)
>  	$(foreach o,name build duration,./support/scripts/graph-build-time \
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 31dbc54..3b26e6b 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -737,6 +737,9 @@ $(1)-show-depends:
>  $(1)-show-rdepends:
>  			@echo $$($(2)_RDEPENDENCIES)
>  
> +$(1)-show-build-order: $$(patsubst %,%-show-build-order,$$($(2)_FINAL_ALL_DEPENDENCIES))
> +	$$(info $(1))
> +
>  $(1)-graph-depends: graph-depends-requirements
>  	$(call pkg-graph-depends,$(1),--direct)
>  
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Arnout Vandecappelle]

On 07-04-17 12:31, Arnout Vandecappelle wrote:
> 
> 
> On 02-04-17 15:03, Yann E. MORIN wrote:
>> When debugging hidden dependencies, the build order is very important.
>> Most notably, it is interesting to identify potential culprits.
>>
>> Add a new top-level rule, show-biuld-order, that dumps all the packages
>                                  build
>> in the order they would get built.
>>
>> Note that there are a few differences with show-targets:
>>
>>   - more packages are reported, becasue show-targets does not report
>                                   because
>>     host packages that have no prompt;
>>
>>   - the output is line-based, because we're using $(info $(1)); getting
>>     a single output line like show-targets would require we use an
>>     actual command, like printf '%s ' $(1); but that takes a lot of
>>     time, while $(info $(1)) is almost instantaneous (the time to parse
>>     the Makefiles);
>>
>>   - rootfs targets are not reported.
>>
>> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 I've now actually started using this patch and it has a real benefit over
show-targets, because it shows all targets, not just the ones appearing in
.config. Ideal if you need to find all host packages...

 Perhaps now show-targets can be eliminated? It is not documented, it's only
used in graph-depends, and AFAICS from there it can easily be converted into
show-build-order. Well, for that -show-build-order should be added to
fs/common.mk but that's not rocket science :-)

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] core: add rule to dump packages' build order

[Thomas Petazzoni]

Hello,

On Sun,  2 Apr 2017 15:03:38 +0200, Yann E. MORIN wrote:
> When debugging hidden dependencies, the build order is very important.
> Most notably, it is interesting to identify potential culprits.
> 
> Add a new top-level rule, show-biuld-order, that dumps all the packages
> in the order they would get built.
> 
> Note that there are a few differences with show-targets:
> 
>   - more packages are reported, becasue show-targets does not report
>     host packages that have no prompt;
> 
>   - the output is line-based, because we're using $(info $(1)); getting
>     a single output line like show-targets would require we use an
>     actual command, like printf '%s ' $(1); but that takes a lot of
>     time, while $(info $(1)) is almost instantaneous (the time to parse
>     the Makefiles);
> 
>   - rootfs targets are not reported.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> ---
>  Makefile               | 2 ++
>  package/pkg-generic.mk | 3 +++
>  2 files changed, 5 insertions(+)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com