uninative and recipe parsing

uninative and recipe parsing

[Cuero Bugot]

Hi all,

When you add several layers, recipe parsing can take a (very) long time. In our case it takes more than a couple minutes [1]. Fortunately it is supposed to happens once, except when you use uninative (poky's default) where it happens twice (the two first times you build).
I think this is not an intentional behavior so I dug it a little bit and here is what I found:

When inheriting meta/classes/uninative.bbclass, it registers 2 functions on build events: one to fetch the uninative tarball (at bb.event.BuildStarted) and the other one to set variables in the datastore (at bb.event.ConfigParsed).
The function that set the variables [2] to the datastore first check that the uninative blob is in the build tree, so even though it is supposed to happen at recipe parsing, the variable are only really set when the build really start (bb.event.BuildStarted), after the recipes have been parsed!

So I think there is bug in the current behavior as:
	* Either the uninative variables are not important for the recipe parsing, and then they should be added in BB_HASHCONFIG_WHITELIST
	* Or the variables should matter for the recipe parsing so they should be set before the parsing and not in between parsing and build.

I assumed the later, so a simple fix is to register the two functions on the same event: bb.event.ConfigParsed.

Note: We are currently using pyro, but I checked that the master branch should exhibit the same behavior (same code).

[1]: it matters to us as we are doing Continuous Integration and do clean builds (with sstate cache) on every pull requests and master branch commits. The automatic test full cycle take about 20 minutes. We launch 2 bitbake commands during that process. Parsing recipe take about 2-4 minutes, which is significant enough when trying to reduce the waiting and parallel builds/tests.
[2]: the uninative changed variables are: NATIVELSBSTRING, SSTATEPOSTUNPACKFUNCS

Proposed patch:

diff --git a/meta/classes/uninative.bbclass b/meta/classes/uninative.bbclass
index a410647..5713bb8 100644
--- a/meta/classes/uninative.bbclass
+++ b/meta/classes/uninative.bbclass
@@ -9,7 +9,7 @@ UNINATIVE_TARBALL ?= "${BUILD_ARCH}-nativesdk-libc.tar.bz2"
 UNINATIVE_DLDIR ?= "${DL_DIR}/uninative/"
 
 addhandler uninative_event_fetchloader
-uninative_event_fetchloader[eventmask] = "bb.event.BuildStarted"
+uninative_event_fetchloader[eventmask] = "bb.event.ConfigParsed"
 
 addhandler uninative_event_enable
 uninative_event_enable[eventmask] = "bb.event.ConfigParsed"

Re: uninative and recipe parsing

[Richard Purdie]

On Mon, 2018-03-12 at 11:04 +0000, Cuero Bugot wrote:
> Hi all,
> 
> When you add several layers, recipe parsing can take a (very) long
> time. In our case it takes more than a couple minutes [1].
> Fortunately it is supposed to happens once, except when you use
> uninative (poky's default) where it happens twice (the two first
> times you build).
> I think this is not an intentional behavior so I dug it a little bit
> and here is what I found:
> 
> When inheriting meta/classes/uninative.bbclass, it registers 2
> functions on build events: one to fetch the uninative tarball (at
> bb.event.BuildStarted) and the other one to set variables in the
> datastore (at bb.event.ConfigParsed).
> The function that set the variables [2] to the datastore first check
> that the uninative blob is in the build tree, so even though it is
> supposed to happen at recipe parsing, the variable are only really
> set when the build really start (bb.event.BuildStarted), after the
> recipes have been parsed!
> 
> So I think there is bug in the current behavior as:
> 	* Either the uninative variables are not important for the
> recipe parsing, and then they should be added in
> BB_HASHCONFIG_WHITELIST
> 	* Or the variables should matter for the recipe parsing so they
> should be set before the parsing and not in between parsing and
> build.
> 
> I assumed the later, so a simple fix is to register the two functions
> on the same event: bb.event.ConfigParsed.
> 
> Note: We are currently using pyro, but I checked that the master
> branch should exhibit the same behavior (same code).
> 
> [1]: it matters to us as we are doing Continuous Integration and do
> clean builds (with sstate cache) on every pull requests and master
> branch commits. The automatic test full cycle take about 20 minutes.
> We launch 2 bitbake commands during that process. Parsing recipe take
> about 2-4 minutes, which is significant enough when trying to reduce
> the waiting and parallel builds/tests.
> [2]: the uninative changed variables are: NATIVELSBSTRING,
> SSTATEPOSTUNPACKFUNCS
> 
> Proposed patch:
> 
> diff --git a/meta/classes/uninative.bbclass
> b/meta/classes/uninative.bbclass
> index a410647..5713bb8 100644
> --- a/meta/classes/uninative.bbclass
> +++ b/meta/classes/uninative.bbclass
> @@ -9,7 +9,7 @@ UNINATIVE_TARBALL ?= "${BUILD_ARCH}-nativesdk-
> libc.tar.bz2"
>  UNINATIVE_DLDIR ?= "${DL_DIR}/uninative/"
>  
>  addhandler uninative_event_fetchloader
> -uninative_event_fetchloader[eventmask] = "bb.event.BuildStarted"
> +uninative_event_fetchloader[eventmask] = "bb.event.ConfigParsed"
>  
>  addhandler uninative_event_enable
>  uninative_event_enable[eventmask] = "bb.event.ConfigParsed"

I have kind of noticed this and agree its something we should fix.
Thanks for digging into it. I think a long time ago we tried the above
change and it does cause a problem, perhaps fetching during parsing
which is bad.

Have you been able to figure out what changes in the data store for the
cache to be invalidated? I think whitelisting something may be the best
solution, or perhaps only activating certain changes in the build code
paths after parsing in all cases.

Cheers,

Richard

Re: uninative and recipe parsing

[Cuero Bugot]

> I have kind of noticed this and agree its something we should fix.
> Thanks for digging into it. I think a long time ago we tried the above change and it does cause a problem, perhaps fetching during parsing which is bad.
>
> Have you been able to figure out what changes in the data store for the cache to be invalidated? I think whitelisting something may be the best solution, or perhaps only activating certain changes in the build code > paths after parsing in all cases.
>

Yes absolutely the variables and content diff is as follow:
830c830
< ('NATIVELSBSTRING', 'neon-16.04')
---
> ('NATIVELSBSTRING', 'universal')
1447c1447,1449
< ('SSTATEPOSTUNPACKFUNCS', 'sstate_hardcode_path_unpack')
---
> ('SSTATEPOSTUNPACKFUNCS', 'sstate_hardcode_path_unpack uninative_changeinterp')
> ('SSTATEPOSTUNPACKFUNCS[_append]', [[' uninative_changeinterp', None]])
> ('SSTATEPOSTUNPACKFUNCS[vardepvalueexclude]', '| uninative_changeinterp')

I did a PR (PATCH) for the 1st alternative (fetching earlier) here: https://patchwork.openembedded.org/series/11369/
But I need to do a v2 to fix the short description to make shorter. if you ever decided that it could be way to go. (FWIW I did test it on our setup and yes it is fetching the uninative binaries early on or maybe during the parsing, see output bellow):

	cbugot@neon:~/devs/yocto/build$ bitbake some-image
	--2018-03-13 18:25:36--  http://build-cache-server/cache//downloads/x86_64-nativesdk-libc.tar.bz2                                                                                                                                                                         
	Resolving build-cache-server (build-cache-server)... 10.41.51.220                                                                                                                                                                                                              
	Connecting to build-cache-server (build-cache-server)|10.41.51.220|:80... connected.                                                                                                                                                                                           
	HTTP request sent, awaiting response... 200 OK                                                                                                                                                                                                                                 
	Length: 2286285 (2.2M) [application/x-bzip2]                                                                                                                                                                                                                                   
	Saving to: ‘/home/cbugot/devs/unity/build/downloads/uninative/ed033c868b87852b07957a4400f3b744c00aef5d6470346ea1a59b6d3e03075e/x86_64-nativesdk-libc.tar.bz2’                                                                                                                                                                                                                                                                                                                        
	2018-03-13 18:25:36 (11.2 MB/s) - ‘/home/cbugot/devs/unity/build/downloads/uninative/ed033c868b87852b07957a4400f3b744c00aef5d6470346ea1a59b6d3e03075e/x86_64-nativesdk-libc.tar.bz2’ saved [2286285/2286285]                                                                                                                                                                                                                                                                                                                               
	NOTE: Fetching uninative binary shim from http://downloads.yoctoproject.org/releases/uninative/1.7/x86_64-nativesdk-libc.tar.bz2;sha256sum=ed033c868b87852b07957a4400f3b744c00aef5d6470346ea1a59b6d3e03075e                                                                    
	Parsing recipes:  37% |#######################################################################################                                                                                                                                                  | ETA:  0:06:30



However, if you think putting the two variables (NATIVELSBSTRING, SSTATEPOSTUNPACKFUNCS)  in BB_HASHCONFIG_WHITELIST is better/safer, I can also submit a PATCH request for that alternative.

Personally I do not have enough yocto experience to decide which one would be the best (just inferred that WHITELIST could lead to weird behavior if someone ever depended on one of this variable in his recipe); so I would be happy either way 😊.

Cheers,
Cuero