[OE-core] [PATCH 3/3] rm_work.bbclass: clean up sooner

[Patrick Ohly]

On Wed, 2017-03-01 at 16:52 +0100, Martin Jansa wrote:
> On Thu, Feb 16, 2017 at 11:26:54AM +0100, Patrick Ohly wrote:
> > On Wed, 2017-02-15 at 19:32 +0100, Martin Jansa wrote:
> > > Are all changes necessary for this to work already in master?
> > 
> > Yes.
> > 
> > > Yesterday I've noticed that rm_work for some components which are
> > > early in the dependency (like qtbase) are executed relatively late
> > > (together with do_package_qa).
> > 
> > Could do_rm_work run before do_package_qa? rm_work.bbclass doesn't know
> > that, and therefore schedules do_rm_work after do_package_qa.
> > 
> > If yes, then adding a list of tasks that can be ignored would be
> > trivial. This can be a variable, so a recipe can even add their own
> > ones, if necessary.
> 
> That's now what I've meant.
> 
> I believe that rm_work needs to be executed after do_package_qa, but I
> don't understand the scheduler code enough (at least not yet) to say
> that higher priority of rm_work task also makes all the tasks rm_work
> depends on e.g. do_package_qa to be executed sooner.
> 
> From my observation it looked like do_package_qa is still executed
> "late", but immediately followed by rm_work thanks to its high priority
> (so it's executed as soon as it can, but it's still late in progress of
> the whole build).
> 
> Another interesting test from today was to run:
> # rm -rf tmp-glibc/*
> # bitbake -n zlib | tee log.zlib.rm_work
> # cd oe-core; git revert -1 936179754c8d0f98e1196ddc6796fdfd72c0c3b4; cd ..
> # rm -rf tmp-glibc/*
> # bitbake -n zlib | tee log.zlib.rm_work.revert
> 
> and it shows interesting difference that many rm_work tasks aren't
> executed at all:
> 
> # grep rm_work log.zlib.rm_work* | grep zlib_
> log.zlib.rm_work:NOTE: Running task 526 of 527 (/OE/build/oe-core/openembedded-core/meta/recipes-core/zlib/zlib_1.2.8.bb:do_rm_work)
> log.zlib.rm_work.revert:NOTE: Running task 128 of 721 (virtual:native:/OE/build/oe-core/openembedded-core/meta/recipes-core/zlib/zlib_1.2.8.bb:do_rm_work)
> log.zlib.rm_work.revert:NOTE: Running task 717 of 721 (/OE/build/oe-core/openembedded-core/meta/recipes-core/zlib/zlib_1.2.8.bb:do_rm_work)
> log.zlib.rm_work.revert:NOTE: Running task 721 of 721 (/OE/build/oe-core/openembedded-core/meta/recipes-core/zlib/zlib_1.2.8.bb:do_rm_work_all)
> 
> # grep rm_work log.zlib.rm_work* | grep gcc
> log.zlib.rm_work.revert:NOTE: Running task 2 of 721 (/OE/build/oe-core/openembedded-core/meta/recipes-devtools/gcc/gcc-source_6.3.bb:do_rm_work)
> log.zlib.rm_work.revert:NOTE: Running task 240 of 721 (/OE/build/oe-core/openembedded-core/meta/recipes-devtools/gcc/gcc-cross-initial_6.3.bb:do_rm_work)
> log.zlib.rm_work.revert:NOTE: Running task 250 of 721 (/OE/build/oe-core/openembedded-core/meta/recipes-devtools/gcc/libgcc-initial_6.3.bb:do_rm_work)
> log.zlib.rm_work.revert:NOTE: Running task 634 of 721 (/OE/build/oe-core/openembedded-core/meta/recipes-devtools/gcc/gcc-cross_6.3.bb:do_rm_work)
> log.zlib.rm_work.revert:NOTE: Running task 674 of 721 (/OE/build/oe-core/openembedded-core/meta/recipes-devtools/gcc/libgcc_6.3.bb:do_rm_work)
> log.zlib.rm_work.revert:NOTE: Running task 678 of 721 (/OE/build/oe-core/openembedded-core/meta/recipes-devtools/gcc/gcc-runtime_6.3.bb:do_rm_work)

I can reproduce that.

The root cause for the current behavior is that zlib:do_build does not
depend on zlib-native:do_build and thus does not enable
zlib-native:do_rm_work:

# bitbake -g zlib
# grep -e '->.*zlib-native' task-depends.dot | grep -v '^.zlib-native'
"binutils-cross-x86_64.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"rpm-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"gcc-cross-initial-x86_64.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"binutils-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"gcc-cross-x86_64.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"file-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"pigz-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"elfutils-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"python-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"libpcre-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"

Only tasks that are required by the current build target(s) are
executed. From that perspective, the current behavior makes sense
(although it is of course not desirable).

More interesting is why it worked before. The answer is this:

# grep rm_work_all log.zlib.rm_work.revert 
NOTE: Running task 721 of 721 (/work/iot-ref-kit/openembedded-core/meta/recipes-core/zlib/zlib_1.2.8.bb:do_rm_work_all)
# grep -e '->.*zlib-native' task-depends.dot | grep -v '^.zlib-native'
"elfutils-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"binutils-cross-x86_64.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"pigz-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"gcc-cross-x86_64.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"binutils-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"gcc-cross-initial-x86_64.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"python-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"rpm-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"zlib.do_rm_work_all" -> "zlib-native.do_rm_work"
"libpcre-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"
"file-native.do_prepare_recipe_sysroot" -> "zlib-native.do_populate_sysroot"

In other words, do_rm_work_all (the default task for "bitbake zlib" in
the previous approach) was responsible for triggering do_rm_work in
everything that zlib depended upon.

A side effect of that was that do_rm_work itself also caused more work
to be done than strictly necessary for the build:

# grep Running log.zlib.rm_work | sed 's/ [0-9]* of [0-9]*//' | grep -v rm_work | grep -v 'noexec task' | wc -l
54
# grep Running log.zlib.rm_work.revert | sed 's/ [0-9]* of [0-9]*//' | grep -v rm_work | grep -v 'noexec task' | wc -l
156

$ diff <(grep Running log.zlib.rm_work | sed 's/ [0-9]* of [0-9]*//' | grep -v rm_work | grep -v 'noexec task' | sort) <(grep Running log.zlib.rm_work.revert | sed 's/ [0-9]* of [0-9]*//' | grep -v rm_work | grep -v 'noexec task' | sort)
0a1,11
> NOTE: Running setscene task (virtual:native:/work/iot-ref-kit/openembedded-core/meta/recipes-connectivity/openssl/openssl_1.0.2k.bb:do_populate_lic_setscene)
> NOTE: Running setscene task (virtual:native:/work/iot-ref-kit/openembedded-core/meta/recipes-connectivity/openssl/openssl_1.0.2k.bb:do_populate_sysroot_setscene)
> NOTE: Running setscene task (virtual:native:/work/iot-ref-kit/openembedded-core/meta/recipes-core/expat/expat_2.2.0.bb:do_populate_lic_setscene)
> NOTE: Running setscene task (virtual:native:/work/iot-ref-kit/openembedded-core/meta/recipes-core/expat/expat_2.2.0.bb:do_populate_sysroot_setscene)
...
> NOTE: Running setscene task (/work/iot-ref-kit/openembedded-core/meta/recipes-core/glibc/glibc_2.25.bb:do_package_qa_setscene)
20a94
> NOTE: Running setscene task (/work/iot-ref-kit/openembedded-core/meta/recipes-core/glibc/glibc_2.25.bb:do_populate_lic_setscene)
22a97
> NOTE: Running setscene task (/work/iot-ref-kit/openembedded-core/meta/recipes-core/glibc/glibc-initial_2.25.bb:do_populate_lic_setscene)
...
> NOTE: Running setscene task (/work/iot-ref-kit/openembedded-core/meta/recipes-support/ptest-runner/ptest-runner_2.0.bb:do_populate_sysroot_setscene)

When rm_work.bbclass is not active, the old code did not execute these
tasks for "bitbake zlib".

Ideally, enabling rm_work.bbclass should trigger the recursive
do_rm_work (as before) but without triggering extra work (unlike
before). I just can't think of a way to do that - not with the kind of
dependencies we have in bitbake at the moment. We would have to
distinguish between "strong" dependencies (if A is active and depends on
B, then B must be executed first) and "weak" ordering (if A and B both
must run and A must run before B, then execute A first).

Given that most tasks triggered by do_rm_work on top of
do_populate_sysroot seem to be fairly light, enabling them in addition
to do_rm_work itself is probably better. Here's a tentative patch
achieving that:

# git diff
diff --git a/meta/classes/rm_work.bbclass b/meta/classes/rm_work.bbclass
index 054c93716e..eeb83d386a 100644
--- a/meta/classes/rm_work.bbclass
+++ b/meta/classes/rm_work.bbclass
@@ -102,6 +102,14 @@ do_rm_work () {
         rm -f $i
     done
 }
+do_rm_work_all () {
+    :
+}
+do_rm_work_all[recrdeptask] = "do_rm_work do_rm_work_all"
+do_rm_work_all[noexec] = "1"
+addtask rm_work_all after before do_build
+
+do_rm_work_all[recrdeptask] = "do_rm_work"
 
 do_populate_sdk[postfuncs] += "rm_work_populatesdk"
 rm_work_populatesdk () {
@@ -138,9 +146,8 @@ python inject_rm_work() {
         # do_build inherits additional runtime dependencies on
         # other recipes and thus will typically run much later than completion of
         # work in the recipe itself.
-        deps = bb.build.preceedtask('do_build', True, d)
-        if 'do_build' in deps:
-            deps.remove('do_build')
+        deps = set(bb.build.preceedtask('do_build', True, d))
+        deps.difference_update(('do_build', 'do_rm_work_all'))
         # In practice, addtask() here merely updates the dependencies.
         bb.build.addtask('do_rm_work', 'do_build', ' '.join(deps), d)
 }



# bitbake -n zlib | tee log.zlib.rm_work.patched
$ diff <(grep Running log.zlib.rm_work.patched | sed 's/ [0-9]* of [0-9]*//' | grep -v 'noexec task' | sort) <(grep Running log.zlib.rm_work.revert | sed 's/ [0-9]* of [0-9]*//' | grep -v 'noexec task' | sort)
202a203
> NOTE: Running task (/work/iot-ref-kit/openembedded-core/meta/recipes-core/zlib/zlib_1.2.8.bb:do_rm_work_all)

The only difference compared to before is because of the slight
do_rm_work_all[noexec] tweak.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.