| Message ID | 20221127214934.541723-1-richard.purdie@linuxfoundation.org |
|---|---|
| State | Accepted, archived |
| Commit | 3d96c07f9fd4ba1a84826811d14bb4e98ad58846 |
| Headers | show |
| Series | [RFC] data: Evaluate the value of export/unexport/network flags | expand |
On Sun, Nov 27, 2022 at 10:49 PM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > Currently the export/unexport/network flags are acted on only by presence > or lack of in the data store. This is deliberate due to performance reasons > since a given recipe shell task might export ~80-90 variables and this > means > expanding flags for every one of them. > > This does catch users unaware since values which expand to nothing don't > work > e.g. ${@""} and setting values to "0" wouldn't work either. > > The downside to this patch is slow down in parsing speed of around 4%. > > The network flag is easier to change and doesn't affect performance, it was > made to operate the same as export/unexport for consitency reasons. > You already know my opinion, but for others on ML I'm in favor of this change. Parsing time is negligible compared to do_compile times in most of my builds. Expanding this will allow to resolve e.g. icecc.bbclass to respect ICECC_DISABLED before enabling network in all tasks: https://lists.openembedded.org/g/openembedded-core/message/167731 I already have such change for icecc.bbclass and will send it when/if this bitbake change is merged. Thanks Cheers,
On Wed, 2023-02-15 at 11:46 +0100, Martin Jansa wrote: > On Sun, Nov 27, 2022 at 10:49 PM Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > Currently the export/unexport/network flags are acted on only by > > presence > > or lack of in the data store. This is deliberate due to performance > > reasons > > since a given recipe shell task might export ~80-90 variables and > > this means > > expanding flags for every one of them. > > > > This does catch users unaware since values which expand to nothing > > don't work > > e.g. ${@""} and setting values to "0" wouldn't work either. > > > > The downside to this patch is slow down in parsing speed of around > > 4%. > > > > The network flag is easier to change and doesn't affect > > performance, it was > > made to operate the same as export/unexport for consitency reasons. > > You already know my opinion, but for others on ML I'm in favor of > this change. > > Parsing time is negligible compared to do_compile times in most of my > builds. Comparing parsing time to do_compile isn't appropriate, they're very different things. If doing more interactive development, parsing time can hurt a lot and is one of the bigger complaints I hear about bitbake, particualrly with larger numbers of layers. > Expanding this will allow to resolve e.g. icecc.bbclass to respect > ICECC_DISABLED before enabling network in all tasks: > https://lists.openembedded.org/g/openembedded-core/message/167731 > I already have such change for icecc.bbclass and will send it when/if > this bitbake change is merged. I personally remain torn. The TSC discussed it and is overall in favour of it so I'll fix this patch, test and merge. I'm running out of ideas to speed bitbake up but I'll probably just have to stop caring. Cheers, Richard
Parsing time is very different than do_compile. Almost all operations on bitbake require parsing but I would argue very few run do_compile in comparison. I have reservations regarding any parse time degradation, as in my experience parse time is one of the biggest complaints by users. A compilation is seen as useful work while parsing, sadly, is not. Paulo Neves On 2/15/23 11:46, Martin Jansa wrote: > > Parsing time is negligible compared to do_compile times in most of my > builds.
diff --git a/bin/bitbake-worker b/bin/bitbake-worker index 3799b170cb..5a89e7c9b8 100755 --- a/bin/bitbake-worker +++ b/bin/bitbake-worker @@ -270,7 +270,7 @@ def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask): bb.utils.set_process_name("%s:%s" % (the_data.getVar("PN"), taskname.replace("do_", ""))) - if not the_data.getVarFlag(taskname, 'network', False): + if not bb.utils.to_boolean(the_data.getVarFlag(taskname, 'network')): if bb.utils.is_local_uid(uid): logger.debug("Attempting to disable network for %s" % taskname) bb.utils.disable_network(uid, gid) diff --git a/lib/bb/data.py b/lib/bb/data.py index ab1a5959ba..5299315050 100644 --- a/lib/bb/data.py +++ b/lib/bb/data.py @@ -114,8 +114,8 @@ def emit_var(var, o=sys.__stdout__, d = init(), all=False): if d.getVarFlag(var, 'python', False) and func: return False - export = d.getVarFlag(var, "export", False) - unexport = d.getVarFlag(var, "unexport", False) + export = bb.utils.to_boolean(d.getVarFlag(var, "export")) + unexport = bb.utils.to_boolean(d.getVarFlag(var, "unexport")) if not all and not export and not unexport and not func: return False @@ -188,8 +188,8 @@ def emit_env(o=sys.__stdout__, d = init(), all=False): def exported_keys(d): return (key for key in d.keys() if not key.startswith('__') and - d.getVarFlag(key, 'export', False) and - not d.getVarFlag(key, 'unexport', False)) + bb.utils.to_boolean(d.getVarFlag(key, 'export', False)) and + not bb.utils.to_boolean(d.getVarFlag(key, 'unexport', False))) def exported_vars(d): k = list(exported_keys(d)) @@ -374,7 +374,7 @@ def generate_dependencies(d, ignored_vars): mod_funcs = set(bb.codeparser.modulecode_deps.keys()) keys = set(key for key in d if not key.startswith("__")) | mod_funcs - shelldeps = set(key for key in d.getVar("__exportlist", False) if d.getVarFlag(key, "export", False) and not d.getVarFlag(key, "unexport", False)) + shelldeps = set(key for key in d.getVar("__exportlist", False) if bb.utils.to_boolean(d.getVarFlag(key, "export")) and not bb.utils.to_boolean(d.getVarFlag(key, "unexport"))) varflagsexcl = d.getVar('BB_SIGNATURE_EXCLUDE_FLAGS') deps = {}
Currently the export/unexport/network flags are acted on only by presence or lack of in the data store. This is deliberate due to performance reasons since a given recipe shell task might export ~80-90 variables and this means expanding flags for every one of them. This does catch users unaware since values which expand to nothing don't work e.g. ${@""} and setting values to "0" wouldn't work either. The downside to this patch is slow down in parsing speed of around 4%. The network flag is easier to change and doesn't affect performance, it was made to operate the same as export/unexport for consitency reasons. Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> --- bin/bitbake-worker | 2 +- lib/bb/data.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) I'm torn on this patch due to the parsing speed slow down. I'm putting the patch out there for discussion.