* About the operator "??=" @ 2010-12-01 1:57 Xu, Dongxiao 2010-12-08 10:30 ` Richard Purdie 0 siblings, 1 reply; 19+ messages in thread From: Xu, Dongxiao @ 2010-12-01 1:57 UTC (permalink / raw) To: Richard Purdie; +Cc: poky@yoctoproject.org Hi Richard, Recently when investigating the file parsing speed, I found the implementation of "??=" is another hot spot. This operator will not apply the default assignment until the end of the parse, which is different from "?=". According to current poky, the users for "??=" is mostly the scm revisions stored in poky-default-revisions.inc. Currently the final assignment logic for "??=" is added in finalize(), which costs about 20% parsing time. The differences between "??=" and "?=" are straight forward, however I don't fully understand in which senario the "??=" should be used. Or in other word, why those scm revisions should use "??=". Could you help to clarify a bit? Thanks so much! BTW, I noticed that openembedded doesn't use this operator. Best Regards, -- Dongxiao ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-01 1:57 About the operator "??=" Xu, Dongxiao @ 2010-12-08 10:30 ` Richard Purdie 2010-12-08 11:35 ` Tian, Kevin 0 siblings, 1 reply; 19+ messages in thread From: Richard Purdie @ 2010-12-08 10:30 UTC (permalink / raw) To: Xu, Dongxiao; +Cc: poky@yoctoproject.org Hi Dongxiao, On Wed, 2010-12-01 at 09:57 +0800, Xu, Dongxiao wrote: > Recently when investigating the file parsing speed, I found the > implementation of "??=" is another hot spot. This operator will not > apply the default assignment until the end of the parse, which is > different from "?=". According to current poky, the users for "??=" is > mostly the scm revisions stored in poky-default-revisions.inc. > > Currently the final assignment logic for "??=" is added in finalize(), > which costs about 20% parsing time. Can I take a look at the graph showing that? I'm a little surprised it takes 20% of the time, that is a rather scary amount! > The differences between "??=" and "?=" are straight forward, however I > don't fully understand in which senario the "??=" should be used. Or > in other word, why those scm revisions should use "??=". Could you > help to clarify a bit? Thanks so much! You would use ??= when you are assigning a value but you want any other definition anywhere to be able to override it regardless of include/inherit ordering constraints. We created the operator to solve some problems we had with the SRCREV variables and its use hasn't grown into other areas but there is no reason it shouldn't be used in other places, it probably just didn't exist when other code was written. > BTW, I noticed that openembedded doesn't use this operator. Their loss ;-) Cheers, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-08 10:30 ` Richard Purdie @ 2010-12-08 11:35 ` Tian, Kevin 2010-12-08 11:59 ` Xu, Dongxiao 2010-12-08 12:49 ` Tian, Kevin 0 siblings, 2 replies; 19+ messages in thread From: Tian, Kevin @ 2010-12-08 11:35 UTC (permalink / raw) To: Richard Purdie, Xu, Dongxiao; +Cc: poky@yoctoproject.org >From: Richard Purdie >Sent: Wednesday, December 08, 2010 6:30 PM > >Hi Dongxiao, > >On Wed, 2010-12-01 at 09:57 +0800, Xu, Dongxiao wrote: >> Recently when investigating the file parsing speed, I found the >> implementation of "??=" is another hot spot. This operator will not >> apply the default assignment until the end of the parse, which is >> different from "?=". According to current poky, the users for "??=" is >> mostly the scm revisions stored in poky-default-revisions.inc. >> >> Currently the final assignment logic for "??=" is added in finalize(), >> which costs about 20% parsing time. > >Can I take a look at the graph showing that? I'm a little surprised it >takes 20% of the time, that is a rather scary amount! > I don't have the graph, but did do a simple test by printing the lazy value list in finalize(). There're 166 items in total. Regarding finalize() may be invoked multiple times for same recipe and we have over 700 recipes scanned in a fresh run, it may contribute obvious overhead just like what Dongxiao previously optimized for distro tracking fields. I think we could remove the lazy key from the list after grabbing it as the default value, and is now testing it... Thanks Kevin ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-08 11:35 ` Tian, Kevin @ 2010-12-08 11:59 ` Xu, Dongxiao 2010-12-08 14:37 ` Richard Purdie 2010-12-08 12:49 ` Tian, Kevin 1 sibling, 1 reply; 19+ messages in thread From: Xu, Dongxiao @ 2010-12-08 11:59 UTC (permalink / raw) To: Tian, Kevin, Richard Purdie; +Cc: poky@yoctoproject.org Tian, Kevin wrote: >> From: Richard Purdie >> Sent: Wednesday, December 08, 2010 6:30 PM >> >> Hi Dongxiao, >> >> On Wed, 2010-12-01 at 09:57 +0800, Xu, Dongxiao wrote: >>> Recently when investigating the file parsing speed, I found the >>> implementation of "??=" is another hot spot. This operator will not >>> apply the default assignment until the end of the parse, which is >>> different from "?=". According to current poky, the users for "??=" >>> is mostly the scm revisions stored in poky-default-revisions.inc. >>> >>> Currently the final assignment logic for "??=" is added in >>> finalize(), which costs about 20% parsing time. >> >> Can I take a look at the graph showing that? I'm a little surprised >> it takes 20% of the time, that is a rather scary amount! Hi Richard, Here I did a profile, see following results: Total time 39.296 secs Here I ranked the result according to "cumtime" item. See "finalize" (33.150 secs) and "finalise" (20.597 secs), there are 13s difference. A lot of time is cost on the following code: for lazykey in bb.data.getVar("__lazy_assigned", d) or (): if bb.data.getVar(lazykey, d) is None: val = bb.data.getVarFlag(lazykey, "defaultval", d) bb.data.setVar(lazykey, val, d) Ordered by: cumulative time ncalls tottime percall cumtime percall filename:lineno(function) 1 0.020 0.020 39.419 39.419 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/ui/knotty.py:33(init) 1689 0.003 0.000 39.374 0.023 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:95(waitEvent) 765 0.002 0.000 39.368 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:121(idle_commands) 765 0.001 0.000 39.115 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:159(runCommands) 765 0.002 0.000 39.114 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/command.py:78(runAsyncCommand) 765 0.002 0.000 39.110 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:766(updateCache) 764 0.054 0.000 39.058 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:995(parse_next) 764 0.007 0.000 38.166 0.050 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:181(loadData) 764 0.010 0.000 36.523 0.048 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:462(load_bbfile) 4302/764 0.016 0.000 36.435 0.048 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/__init__.py:71(handle) 4306/764 0.068 0.000 36.429 0.048 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/parse_py/BBHandler.py:109(handle) 764 0.008 0.000 33.951 0.044 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:365(multi_finalize) 957 0.308 0.000 33.150 0.035 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:303(finalize) 923 0.086 0.000 20.597 0.022 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:92(finalise) 923 1.310 0.001 19.847 0.022 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:60(_build_data) 923 1.593 0.002 16.898 0.018 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:299(generate_dependencies) 287898 2.043 0.000 12.702 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:271(build_dependencies) 928560/459862 1.961 0.000 11.680 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:86(expandWithRefs) 1517574/1198288 0.990 0.000 11.241 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:246(getVar) 525304/269546 1.332 0.000 10.041 0.000 {built-in method sub} 640662/347252 0.373 0.000 8.484 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:116(expand) 523659/318900 0.838 0.000 7.917 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:51(var_sub) 305565 1.620 0.000 7.658 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:212(setVar) 3652690 2.389 0.000 6.970 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:285(getVarFlag) 225169 0.129 0.000 6.047 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:81(setVar) 41959/34572 0.307 0.000 4.718 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:63(python_sub) 360953/338884 0.206 0.000 3.933 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:86(getVar) If replace all the "??=" with "?=", and re-run the profile, the time for finalize and finalise decrease a lot and the difference between the two functions is about 6 secs. Total time: 32.708 secs. (20% time saving) Ordered by: cumulative time ncalls tottime percall cumtime percall filename:lineno(function) 1 0.019 0.019 32.828 32.828 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/ui/knotty.py:33(init) 1689 0.003 0.000 32.785 0.019 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:95(waitEvent) 765 0.002 0.000 32.779 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:121(idle_commands) 765 0.001 0.000 32.526 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:159(runCommands) 765 0.002 0.000 32.525 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/command.py:78(runAsyncCommand) 765 0.002 0.000 32.522 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:766(updateCache) 764 0.053 0.000 32.488 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:995(parse_next) 764 0.007 0.000 31.610 0.041 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:181(loadData) 764 0.009 0.000 29.994 0.039 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:462(load_bbfile) 4302/764 0.015 0.000 29.907 0.039 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/__init__.py:71(handle) 4306/764 0.066 0.000 29.901 0.039 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/parse_py/BBHandler.py:109(handle) 764 0.008 0.000 27.475 0.036 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:365(multi_finalize) 957 0.056 0.000 26.675 0.028 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:303(finalize) 923 0.085 0.000 20.561 0.022 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:92(finalise) 923 1.290 0.001 19.810 0.021 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:60(_build_data) 923 1.601 0.002 16.901 0.018 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:299(generate_dependencies) 287898 1.845 0.000 12.768 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:271(build_dependencies) 928560/459862 1.913 0.000 11.589 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:86(expandWithRefs) 1358764/1039478 0.937 0.000 10.748 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:246(getVar) 525304/269546 1.328 0.000 9.979 0.000 {built-in method sub} 640662/347252 0.366 0.000 8.370 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:116(expand) 523659/318900 0.833 0.000 7.802 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:51(var_sub) 3337869 2.138 0.000 6.174 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:285(getVarFlag) 41959/34572 0.271 0.000 4.643 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:63(python_sub) Thanks, Dongxiao >> > > I don't have the graph, but did do a simple test by printing the lazy > value list in finalize(). There're 166 items in total. Regarding > finalize() may be invoked multiple times for same recipe and we have > over 700 recipes scanned in a fresh run, it may contribute obvious > overhead just like what Dongxiao previously optimized for distro > tracking fields. > > I think we could remove the lazy key from the list after grabbing it > as the default value, and is now testing it... > > Thanks > Kevin ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-08 11:59 ` Xu, Dongxiao @ 2010-12-08 14:37 ` Richard Purdie 2010-12-09 3:32 ` Xu, Dongxiao 0 siblings, 1 reply; 19+ messages in thread From: Richard Purdie @ 2010-12-08 14:37 UTC (permalink / raw) To: Xu, Dongxiao; +Cc: poky@yoctoproject.org On Wed, 2010-12-08 at 19:59 +0800, Xu, Dongxiao wrote: > Here I did a profile, see following results: > > Total time 39.296 secs > Here I ranked the result according to "cumtime" item. > See "finalize" (33.150 secs) and "finalise" (20.597 secs), there are 13s difference. A lot of time is cost on the following code: > > for lazykey in bb.data.getVar("__lazy_assigned", d) or (): > if bb.data.getVar(lazykey, d) is None: > val = bb.data.getVarFlag(lazykey, "defaultval", d) > bb.data.setVar(lazykey, val, d) > > > Ordered by: cumulative time > > ncalls tottime percall cumtime percall filename:lineno(function) > 1 0.020 0.020 39.419 39.419 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/ui/knotty.py:33(init) > 1689 0.003 0.000 39.374 0.023 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:95(waitEvent) > 765 0.002 0.000 39.368 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:121(idle_commands) > 765 0.001 0.000 39.115 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:159(runCommands) > 765 0.002 0.000 39.114 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/command.py:78(runAsyncCommand) > 765 0.002 0.000 39.110 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:766(updateCache) > 764 0.054 0.000 39.058 0.051 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:995(parse_next) > 764 0.007 0.000 38.166 0.050 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:181(loadData) > 764 0.010 0.000 36.523 0.048 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:462(load_bbfile) > 4302/764 0.016 0.000 36.435 0.048 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/__init__.py:71(handle) > 4306/764 0.068 0.000 36.429 0.048 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/parse_py/BBHandler.py:109(handle) > 764 0.008 0.000 33.951 0.044 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:365(multi_finalize) > 957 0.308 0.000 33.150 0.035 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:303(finalize) > 923 0.086 0.000 20.597 0.022 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:92(finalise) > 923 1.310 0.001 19.847 0.022 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:60(_build_data) > 923 1.593 0.002 16.898 0.018 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:299(generate_dependencies) > 287898 2.043 0.000 12.702 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:271(build_dependencies) > 928560/459862 1.961 0.000 11.680 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:86(expandWithRefs) > 1517574/1198288 0.990 0.000 11.241 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:246(getVar) > 525304/269546 1.332 0.000 10.041 0.000 {built-in method sub} > 640662/347252 0.373 0.000 8.484 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:116(expand) > 523659/318900 0.838 0.000 7.917 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:51(var_sub) > 305565 1.620 0.000 7.658 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:212(setVar) > 3652690 2.389 0.000 6.970 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:285(getVarFlag) > 225169 0.129 0.000 6.047 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:81(setVar) > 41959/34572 0.307 0.000 4.718 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:63(python_sub) > 360953/338884 0.206 0.000 3.933 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:86(getVar) > > > > If replace all the "??=" with "?=", and re-run the profile, the time for finalize and finalise decrease a lot and the difference between the two functions is about 6 secs. > > Total time: 32.708 secs. (20% time saving) > > Ordered by: cumulative time > > ncalls tottime percall cumtime percall filename:lineno(function) > 1 0.019 0.019 32.828 32.828 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/ui/knotty.py:33(init) > 1689 0.003 0.000 32.785 0.019 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:95(waitEvent) > 765 0.002 0.000 32.779 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:121(idle_commands) > 765 0.001 0.000 32.526 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:159(runCommands) > 765 0.002 0.000 32.525 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/command.py:78(runAsyncCommand) > 765 0.002 0.000 32.522 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:766(updateCache) > 764 0.053 0.000 32.488 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:995(parse_next) > 764 0.007 0.000 31.610 0.041 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:181(loadData) > 764 0.009 0.000 29.994 0.039 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:462(load_bbfile) > 4302/764 0.015 0.000 29.907 0.039 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/__init__.py:71(handle) > 4306/764 0.066 0.000 29.901 0.039 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/parse_py/BBHandler.py:109(handle) > 764 0.008 0.000 27.475 0.036 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:365(multi_finalize) > 957 0.056 0.000 26.675 0.028 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:303(finalize) > 923 0.085 0.000 20.561 0.022 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:92(finalise) > 923 1.290 0.001 19.810 0.021 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:60(_build_data) > 923 1.601 0.002 16.901 0.018 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:299(generate_dependencies) > 287898 1.845 0.000 12.768 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:271(build_dependencies) > 928560/459862 1.913 0.000 11.589 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:86(expandWithRefs) > 1358764/1039478 0.937 0.000 10.748 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:246(getVar) > 525304/269546 1.328 0.000 9.979 0.000 {built-in method sub} > 640662/347252 0.366 0.000 8.370 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:116(expand) > 523659/318900 0.833 0.000 7.802 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:51(var_sub) > 3337869 2.138 0.000 6.174 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:285(getVarFlag) > 41959/34572 0.271 0.000 4.643 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:63(python_sub) Thanks, that is quite a significant difference. Could you try working out how often getVar returns a "None" value? I'm wondering if we should add something to getVar that if it is about to return None, it checks for the default value flag and returns that if set. Originally ??= wasn't implemented that way do to performance concerns but I think we need to recheck that! Cheers, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-08 14:37 ` Richard Purdie @ 2010-12-09 3:32 ` Xu, Dongxiao 2010-12-09 4:57 ` Tian, Kevin 0 siblings, 1 reply; 19+ messages in thread From: Xu, Dongxiao @ 2010-12-09 3:32 UTC (permalink / raw) To: Richard Purdie; +Cc: poky@yoctoproject.org Richard Purdie wrote: > On Wed, 2010-12-08 at 19:59 +0800, Xu, Dongxiao wrote: >> Here I did a profile, see following results: >> >> Total time 39.296 secs >> Here I ranked the result according to "cumtime" item. >> See "finalize" (33.150 secs) and "finalise" (20.597 secs), there are >> 13s difference. A lot of time is cost on the following code: >> >> for lazykey in bb.data.getVar("__lazy_assigned", d) or (): >> if bb.data.getVar(lazykey, d) is None: >> val = bb.data.getVarFlag(lazykey, "defaultval", d) >> bb.data.setVar(lazykey, val, d) >> >> >> Ordered by: cumulative time >> >> ncalls tottime percall cumtime percall >> filename:lineno(function) 1 0.020 0.020 39.419 >> 39.419 >> >> >> >> >> >> >> >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/ui/knotty.py:33(init) >> 1689 0.003 0.000 39.374 0.023 >> >> >> >> >> >> >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:95(waitEvent) >> 765 0.002 0.000 39.368 0.051 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:121(idle_commands) >> 765 0.001 0.000 39.115 0.051 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:159(runCommands) >> 765 0.002 0.000 39.114 0.051 >> >> >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/command.py:78(runAsyncCommand) >> 765 0.002 0.000 39.110 0.051 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:766(updateCache) >> 764 0.054 0.000 39.058 0.051 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:995(parse_next) >> 764 0.007 0.000 38.166 0.050 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:181(loadData) >> 764 0.010 0.000 36.523 0.048 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:462(load_bbfile) >> 4302/764 0.016 0.000 36.435 0.048 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/__init__.py:71(handle) >> 4306/764 0.068 0.000 36.429 0.048 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/parse_py/BBHandler.py:109(handle) >> 764 0.008 0.000 33.951 0.044 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:365(multi_finalize) >> 957 0.308 0.000 33.150 0.035 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:303(finalize) >> 923 0.086 0.000 20.597 0.022 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:92(finalise) >> 923 1.310 0.001 19.847 0.022 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:60(_build_data) >> 923 1.593 0.002 16.898 0.018 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:299(generate_dependencies) >> 287898 2.043 0.000 12.702 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:271(build_dependencies) >> 928560/459862 1.961 0.000 11.680 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:86(expandWithRefs) >> 1517574/1198288 0.990 0.000 11.241 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:246(getVar) >> 525304/269546 1.332 0.000 10.041 0.000 {built-in method >> sub} 640662/347252 0.373 0.000 8.484 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:116(expand) >> 523659/318900 0.838 0.000 7.917 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:51(var_sub) >> 305565 1.620 0.000 7.658 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:212(setVar) >> 3652690 2.389 0.000 6.970 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:285(getVarFlag) >> 225169 0.129 0.000 6.047 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:81(setVar) >> 41959/34572 0.307 0.000 4.718 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:63(python_sub) >> 360953/338884 0.206 0.000 3.933 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:86(getVar) >> >> >> >> If replace all the "??=" with "?=", and re-run the profile, the time >> for finalize and finalise decrease a lot and the difference between >> the two functions is about 6 secs. >> >> Total time: 32.708 secs. (20% time saving) >> >> Ordered by: cumulative time >> >> ncalls tottime percall cumtime percall >> filename:lineno(function) 1 0.019 0.019 32.828 >> 32.828 >> >> >> >> >> >> >> >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/ui/knotty.py:33(init) >> 1689 0.003 0.000 32.785 0.019 >> >> >> >> >> >> >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:95(waitEvent) >> 765 0.002 0.000 32.779 0.043 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:121(idle_commands) >> 765 0.001 0.000 32.526 0.043 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:159(runCommands) >> 765 0.002 0.000 32.525 0.043 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/command.py:78(runAsyncCommand) >> 765 0.002 0.000 32.522 0.043 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:766(updateCache) >> 764 0.053 0.000 32.488 0.043 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:995(parse_next) >> 764 0.007 0.000 31.610 0.041 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:181(loadData) >> 764 0.009 0.000 29.994 0.039 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:462(load_bbfile) >> 4302/764 0.015 0.000 29.907 0.039 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/__init__.py:71(handle) >> 4306/764 0.066 0.000 29.901 0.039 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/parse_py/BBHandler.py:109(handle) >> 764 0.008 0.000 27.475 0.036 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:365(multi_finalize) >> 957 0.056 0.000 26.675 0.028 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:303(finalize) >> 923 0.085 0.000 20.561 0.022 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:92(finalise) >> 923 1.290 0.001 19.810 0.021 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:60(_build_data) >> 923 1.601 0.002 16.901 0.018 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:299(generate_dependencies) >> 287898 1.845 0.000 12.768 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:271(build_dependencies) >> 928560/459862 1.913 0.000 11.589 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:86(expandWithRefs) >> 1358764/1039478 0.937 0.000 10.748 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:246(getVar) >> 525304/269546 1.328 0.000 9.979 0.000 {built-in method >> sub} 640662/347252 0.366 0.000 8.370 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:116(expand) >> 523659/318900 0.833 0.000 7.802 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:51(var_sub) >> 3337869 2.138 0.000 6.174 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:285(getVarFlag) >> 41959/34572 0.271 0.000 4.643 0.000 >> /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:63(python_sub) > > Thanks, that is quite a significant difference. > > Could you try working out how often getVar returns a "None" value? > I'm wondering if we should add something to getVar that if it is > about to return None, it checks for the default value flag and > returns that if set. Originally ??= wasn't implemented that way do to > performance concerns but I think we need to recheck that! Hi Richard, During the file parsing process, the getVar is called "1435156/1148654" times (actually I am still confused what "/" means...), and among them, 322121 times of getVar call return "None", thus about 1/3 or 1/4 of the total calls. I tried to use the following patch to have a test. Also I paste a profile log. From the result we can see it has over 15% performance gain. (from 39.3 secs to 33.7 secs). In usermanual, the "??=" definition is: Setting a default value (??=) A ??= "somevalue" A ??= "someothervalue" If A is set before the above, it will retain that value. If A is unset prior to the above, A will be set to someothervalue. This is a lazy version of ?=, in that the assignment does not occur until the end of the parsing process, so that the last, rather than the first, ??= assignment to a given variable will be used. Our patch logic isn't strickly following the "??=" definition, since the assignment doesn't occurs in end of parsing process... Except this one, I think other behaviors for ??= do not change. diff --git a/bitbake/lib/bb/data_smart.py b/bitbake/lib/bb/data_smart.py index b9d9476..14ac305 100644 --- a/bitbake/lib/bb/data_smart.py +++ b/bitbake/lib/bb/data_smart.py @@ -246,6 +246,9 @@ class DataSmart: def getVar(self, var, exp): value = self.getVarFlag(var, "content") + if value == None: + value = self.getVarFlag(var, "defaultval") + if exp and value: return self.expand(value, var) return value diff --git a/bitbake/lib/bb/parse/ast.py b/bitbake/lib/bb/parse/ast.py index 870ae65..1ccda82 100644 --- a/bitbake/lib/bb/parse/ast.py +++ b/bitbake/lib/bb/parse/ast.py @@ -109,10 +109,8 @@ class DataNode(AstNode): if 'flag' in groupd and groupd['flag'] != None: bb.data.setVarFlag(key, groupd['flag'], val, data) elif groupd["lazyques"]: - assigned = bb.data.getVar("__lazy_assigned", data) or [] - assigned.append(key) - bb.data.setVar("__lazy_assigned", assigned, data) bb.data.setVarFlag(key, "defaultval", val, data) + bb.data.setVar(key, None, data) else: bb.data.setVar(key, val, data) @@ -301,10 +299,6 @@ def handleInherit(statements, m): statements.append(InheritNode(m.group(1))) def finalize(fn, d, variant = None): - for lazykey in bb.data.getVar("__lazy_assigned", d) or (): - if bb.data.getVar(lazykey, d) is None: - val = bb.data.getVarFlag(lazykey, "defaultval", d) - bb.data.setVar(lazykey, val, d) bb.data.expandKeys(d) bb.data.update_data(d) Thu Dec 9 11:20:53 2010 profile.log 30227175 function calls (28578863 primitive calls) in 33.762 CPU seconds Ordered by: cumulative time ncalls tottime percall cumtime percall filename:lineno(function) 1 0.019 0.019 33.883 33.883 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/ui/knotty.py:33(init) 1689 0.003 0.000 33.839 0.020 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:95(waitEvent) 765 0.002 0.000 33.833 0.044 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/server/none.py:121(idle_commands) 765 0.001 0.000 33.580 0.044 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:159(runCommands) 765 0.002 0.000 33.579 0.044 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/command.py:78(runAsyncCommand) 765 0.002 0.000 33.576 0.044 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:766(updateCache) 764 0.054 0.000 33.543 0.044 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cooker.py:995(parse_next) 764 0.007 0.000 32.654 0.043 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:181(loadData) 764 0.010 0.000 30.933 0.040 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:462(load_bbfile) 4302/764 0.016 0.000 30.845 0.040 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/__init__.py:71(handle) 4306/764 0.066 0.000 30.838 0.040 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/parse_py/BBHandler.py:109(handle) 764 0.008 0.000 28.353 0.037 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:359(multi_finalize) 957 0.056 0.000 27.526 0.029 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:301(finalize) 923 0.087 0.000 21.212 0.023 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:92(finalise) 923 1.387 0.002 20.451 0.022 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/siggen.py:60(_build_data) 923 1.598 0.002 17.359 0.019 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:299(generate_dependencies) 287898 2.104 0.000 13.220 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:271(build_dependencies) 928560/459862 1.928 0.000 11.910 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:86(expandWithRefs) 1357786/1038500 1.121 0.000 11.223 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:246(getVar) 525304/269546 1.376 0.000 10.207 0.000 {built-in method sub} 640662/347252 0.373 0.000 8.488 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:116(expand) 523659/318900 0.849 0.000 8.010 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:51(var_sub) 3503560 2.258 0.000 6.529 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:288(getVarFlag) 41959/34572 0.300 0.000 4.656 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:63(python_sub) 201090/179021 0.114 0.000 3.494 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:86(getVar) 47023/39636 0.026 0.000 3.206 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/utils.py:372(better_eval) 47023/39636 0.240 0.000 3.185 0.000 {eval} 149629 0.751 0.000 3.060 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:212(setVar) 4306/961 0.045 0.000 2.973 0.003 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:35(eval) 3710376 2.861 0.000 2.861 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:191(_findVar) 3538/1163 0.043 0.000 2.810 0.002 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/parse_py/ConfHandler.py:46(include) 2405/926 0.034 0.000 2.477 0.003 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/parse_py/BBHandler.py:69(inherit) 958 0.130 0.000 2.376 0.002 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:141(expandKeys) 116442/116213 0.063 0.000 1.913 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:137(expand) 84281 0.415 0.000 1.876 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:302(<genexpr>) 2212/867 0.004 0.000 1.812 0.002 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:260(eval) 2571 0.003 0.000 1.760 0.001 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:267(update_data) 2571 0.123 0.000 1.757 0.001 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data_smart.py:120(finalize) 1506127 1.232 0.000 1.715 0.000 /usr/lib/python2.6/copy.py:65(copy) 6021 0.055 0.000 1.707 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/utils.py:369(simple_exec) 923 0.097 0.000 1.675 0.002 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:354(handle_data) 60485 0.099 0.000 1.587 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/cache.py:96(getVar) 37601 0.114 0.000 1.520 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:85(eval) 275813 0.772 0.000 1.507 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/COW.py:97(__getitem__) 42029 1.376 0.000 1.376 0.000 {compile} 69158 0.043 0.000 1.329 0.000 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/data.py:81(setVar) 599/412 0.004 0.000 1.315 0.003 /home/dongxiao/poky/scripts/..//bitbake/lib/bb/parse/ast.py:49(eval) Thanks, Dongxiao > > Cheers, > > Richard ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-09 3:32 ` Xu, Dongxiao @ 2010-12-09 4:57 ` Tian, Kevin 2010-12-09 7:59 ` Xu, Dongxiao 0 siblings, 1 reply; 19+ messages in thread From: Tian, Kevin @ 2010-12-09 4:57 UTC (permalink / raw) To: Xu, Dongxiao, Richard Purdie; +Cc: poky@yoctoproject.org >From: Xu, Dongxiao >Sent: Thursday, December 09, 2010 11:33 AM >> >> Thanks, that is quite a significant difference. >> >> Could you try working out how often getVar returns a "None" value? >> I'm wondering if we should add something to getVar that if it is >> about to return None, it checks for the default value flag and >> returns that if set. Originally ??= wasn't implemented that way do to >> performance concerns but I think we need to recheck that! > >Hi Richard, > >During the file parsing process, the getVar is called "1435156/1148654" times (actually I am >still confused what "/" means...), and among them, 322121 times of getVar call return >"None", thus about 1/3 or 1/4 of the total calls. > >I tried to use the following patch to have a test. Also I paste a profile log. From the result we can see it has over 15% performance gain. (from 39.3 secs to 33.7 >secs). > >In usermanual, the "??=" definition is: > >Setting a default value (??=) > A ??= "somevalue" > A ??= "someothervalue" >If A is set before the above, it will retain that value. If A is unset prior to the above, A will >be set to someothervalue. This is a lazy version of ?=, in that the assignment does not >occur until the end of the parsing process, so that the last, rather than the first, ??= >assignment to a given variable will be used. > >Our patch logic isn't strickly following the "??=" definition, since the assignment doesn't >occurs in end of parsing process... >Except this one, I think other behaviors for ??= do not change. I think it is, as long as a getVar on a variable happens after all the evaluations of that variable which I think should be true or else even current design has problem. In that way when evaluating variable assignments, you keep 'defaultvalue' updated until the last "??=" assignment. Then later a getVar() on that variable happens which then you just return the right defaultvalue. :-) > >diff --git a/bitbake/lib/bb/data_smart.py b/bitbake/lib/bb/data_smart.py >index b9d9476..14ac305 100644 >--- a/bitbake/lib/bb/data_smart.py >+++ b/bitbake/lib/bb/data_smart.py >@@ -246,6 +246,9 @@ class DataSmart: > def getVar(self, var, exp): > value = self.getVarFlag(var, "content") > >+ if value == None: >+ value = self.getVarFlag(var, "defaultval") >+ > if exp and value: > return self.expand(value, var) > return value >diff --git a/bitbake/lib/bb/parse/ast.py b/bitbake/lib/bb/parse/ast.py >index 870ae65..1ccda82 100644 >--- a/bitbake/lib/bb/parse/ast.py >+++ b/bitbake/lib/bb/parse/ast.py >@@ -109,10 +109,8 @@ class DataNode(AstNode): > if 'flag' in groupd and groupd['flag'] != None: > bb.data.setVarFlag(key, groupd['flag'], val, data) > elif groupd["lazyques"]: >- assigned = bb.data.getVar("__lazy_assigned", data) or [] >- assigned.append(key) >- bb.data.setVar("__lazy_assigned", assigned, data) > bb.data.setVarFlag(key, "defaultval", val, data) >+ bb.data.setVar(key, None, data) This I think is incorrect. You actually wiped out previous assignment before ??=, and thus end up to always have default value favored unless there're other direct assignments after ??= evaluation. Just touch the flag should be enough. > else: > bb.data.setVar(key, val, data) > >@@ -301,10 +299,6 @@ def handleInherit(statements, m): > statements.append(InheritNode(m.group(1))) > > def finalize(fn, d, variant = None): >- for lazykey in bb.data.getVar("__lazy_assigned", d) or (): >- if bb.data.getVar(lazykey, d) is None: >- val = bb.data.getVarFlag(lazykey, "defaultval", d) >- bb.data.setVar(lazykey, val, d) > > bb.data.expandKeys(d) > bb.data.update_data(d) Thanks Kevin ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-09 4:57 ` Tian, Kevin @ 2010-12-09 7:59 ` Xu, Dongxiao 2010-12-09 11:24 ` Richard Purdie 0 siblings, 1 reply; 19+ messages in thread From: Xu, Dongxiao @ 2010-12-09 7:59 UTC (permalink / raw) To: Tian, Kevin, Richard Purdie; +Cc: poky@yoctoproject.org Tian, Kevin wrote: >> From: Xu, Dongxiao >> Sent: Thursday, December 09, 2010 11:33 AM >>> >>> Thanks, that is quite a significant difference. >>> >>> Could you try working out how often getVar returns a "None" value? >>> I'm wondering if we should add something to getVar that if it is >>> about to return None, it checks for the default value flag and >>> returns that if set. Originally ??= wasn't implemented that way do >>> to performance concerns but I think we need to recheck that! >> >> Hi Richard, >> >> During the file parsing process, the getVar is called >> "1435156/1148654" >> times (actually I am still confused what "/" means...), and among >> them, 322121 times of getVar call return "None", thus about 1/3 or >> 1/4 of the total calls. >> >> I tried to use the following patch to have a test. Also I paste a >> profile log. >> From the result we can see it has over 15% performance gain. (from >> 39.3 >> secs to 33.7 secs). >> >> In usermanual, the "??=" definition is: >> >> Setting a default value (??=) >> A ??= "somevalue" >> A ??= "someothervalue" >> If A is set before the above, it will retain that value. If A is >> unset >> prior to the above, A will be set to someothervalue. This is a lazy >> version of ?=, in that the assignment does not occur until the end of >> the parsing process, so that the last, rather than the first, ??= >> assignment to a given variable will be used. >> >> Our patch logic isn't strickly following the "??=" definition, since >> the assignment doesn't occurs in end of parsing process... >> Except this one, I think other behaviors for ??= do not change. > > I think it is, as long as a getVar on a variable happens after all > the evaluations of that variable which I think should be true or else > even current design has problem. In that way when evaluating variable > assignments, you keep 'defaultvalue' updated until the last "??=" > assignment. Then later a getVar() on that variable happens which then > you just return the right defaultvalue. :-) Agree. > >> >> diff --git a/bitbake/lib/bb/data_smart.py >> b/bitbake/lib/bb/data_smart.py index b9d9476..14ac305 100644 >> --- a/bitbake/lib/bb/data_smart.py >> +++ b/bitbake/lib/bb/data_smart.py >> @@ -246,6 +246,9 @@ class DataSmart: >> def getVar(self, var, exp): >> value = self.getVarFlag(var, "content") >> >> + if value == None: >> + value = self.getVarFlag(var, "defaultval") + >> if exp and value: >> return self.expand(value, var) >> return value >> diff --git a/bitbake/lib/bb/parse/ast.py >> b/bitbake/lib/bb/parse/ast.py >> index 870ae65..1ccda82 100644 >> --- a/bitbake/lib/bb/parse/ast.py >> +++ b/bitbake/lib/bb/parse/ast.py >> @@ -109,10 +109,8 @@ class DataNode(AstNode): >> if 'flag' in groupd and groupd['flag'] != None: >> bb.data.setVarFlag(key, groupd['flag'], val, data) >> elif groupd["lazyques"]: >> - assigned = bb.data.getVar("__lazy_assigned", data) or [] >> - assigned.append(key) >> - bb.data.setVar("__lazy_assigned", assigned, data) >> bb.data.setVarFlag(key, "defaultval", val, data) >> + bb.data.setVar(key, None, data) > > This I think is incorrect. You actually wiped out previous assignment > before ??=, and thus end up to always have default value favored > unless there're other direct assignments after ??= evaluation. Just > touch the flag should be enough. Ah, yes, I need to check whether the value has been setVar before, see following patch. This two lines are necessary in the following patch + if bb.data.getVarFlag(key, "content", data) is None: + bb.data.setVar(key, None, data) because setVar will add the key into override list, which could not be achieved by just calling "setVarFlag". For the later formal patch, I will add comments on the two lines. The performance for the revised patch is more or less the same as previous, so I didn't paste the profile result. Thanks, Dongxiao diff --git a/bitbake/lib/bb/data_smart.py b/bitbake/lib/bb/data_smart.py index b9d9476..771e480 100644 --- a/bitbake/lib/bb/data_smart.py +++ b/bitbake/lib/bb/data_smart.py @@ -246,6 +246,9 @@ class DataSmart: def getVar(self, var, exp): value = self.getVarFlag(var, "content") + if value is None: + value = self.getVarFlag(var, "defaultval") + if exp and value: return self.expand(value, var) return value diff --git a/bitbake/lib/bb/parse/ast.py b/bitbake/lib/bb/parse/ast.py index 870ae65..e31063d 100644 --- a/bitbake/lib/bb/parse/ast.py +++ b/bitbake/lib/bb/parse/ast.py @@ -109,10 +109,9 @@ class DataNode(AstNode): if 'flag' in groupd and groupd['flag'] != None: bb.data.setVarFlag(key, groupd['flag'], val, data) elif groupd["lazyques"]: - assigned = bb.data.getVar("__lazy_assigned", data) or [] - assigned.append(key) - bb.data.setVar("__lazy_assigned", assigned, data) bb.data.setVarFlag(key, "defaultval", val, data) + if bb.data.getVarFlag(key, "content", data) is None: + bb.data.setVar(key, None, data) else: bb.data.setVar(key, val, data) @@ -301,10 +300,6 @@ def handleInherit(statements, m): statements.append(InheritNode(m.group(1))) def finalize(fn, d, variant = None): - for lazykey in bb.data.getVar("__lazy_assigned", d) or (): - if bb.data.getVar(lazykey, d) is None: - val = bb.data.getVarFlag(lazykey, "defaultval", d) - bb.data.setVar(lazykey, val, d) bb.data.expandKeys(d) bb.data.update_data(d) > >> else: >> bb.data.setVar(key, val, data) >> >> @@ -301,10 +299,6 @@ def handleInherit(statements, m): >> statements.append(InheritNode(m.group(1))) >> >> def finalize(fn, d, variant = None): >> - for lazykey in bb.data.getVar("__lazy_assigned", d) or (): >> - if bb.data.getVar(lazykey, d) is None: >> - val = bb.data.getVarFlag(lazykey, "defaultval", d) >> - bb.data.setVar(lazykey, val, d) >> >> bb.data.expandKeys(d) >> bb.data.update_data(d) > > Thanks > Kevin ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-09 7:59 ` Xu, Dongxiao @ 2010-12-09 11:24 ` Richard Purdie 2010-12-09 12:13 ` Xu, Dongxiao 0 siblings, 1 reply; 19+ messages in thread From: Richard Purdie @ 2010-12-09 11:24 UTC (permalink / raw) To: Xu, Dongxiao, Chris Larson; +Cc: poky@yoctoproject.org On Thu, 2010-12-09 at 15:59 +0800, Xu, Dongxiao wrote: > Tian, Kevin wrote: > >> From: Xu, Dongxiao > >> Sent: Thursday, December 09, 2010 11:33 AM > >> During the file parsing process, the getVar is called > >> "1435156/1148654" > >> times (actually I am still confused what "/" means...), and among > >> them, 322121 times of getVar call return "None", thus about 1/3 or > >> 1/4 of the total calls. Ok, that is a pretty significant portion of the time. > >> I tried to use the following patch to have a test. Also I paste a > >> profile log. > >> From the result we can see it has over 15% performance gain. (from > >> 39.3 > >> secs to 33.7 secs). So despite that, it still significantly helps performance. Could you also give some raw performance numbers? By this I mean make the timing runs with profiling turned off so work out the time it takes before and after the patch with the profiling off so the numbers are "real" with "time bitbake xxx"? It always pays just to sanity check and this gives the "real" performance gain. > >> In usermanual, the "??=" definition is: > >> > >> Setting a default value (??=) > >> A ??= "somevalue" > >> A ??= "someothervalue" > >> If A is set before the above, it will retain that value. If A is > >> unset > >> prior to the above, A will be set to someothervalue. This is a lazy > >> version of ?=, in that the assignment does not occur until the end of > >> the parsing process, so that the last, rather than the first, ??= > >> assignment to a given variable will be used. > >> > >> Our patch logic isn't strickly following the "??=" definition, since > >> the assignment doesn't occurs in end of parsing process... > >> Except this one, I think other behaviors for ??= do not change. > > > > I think it is, as long as a getVar on a variable happens after all > > the evaluations of that variable which I think should be true or else > > even current design has problem. In that way when evaluating variable > > assignments, you keep 'defaultvalue' updated until the last "??=" > > assignment. Then later a getVar() on that variable happens which then > > you just return the right defaultvalue. :-) > > Agree. I also agree, I think we can adjust the definition of how this is handled slightly. There is also a change in behaviour as now, when one of these variables is looked up, it will get the default value rather than None if finalise() hasn't been called. I don't think this is much of an issue though. > >> --- a/bitbake/lib/bb/parse/ast.py > >> +++ b/bitbake/lib/bb/parse/ast.py > >> @@ -109,10 +109,8 @@ class DataNode(AstNode): > >> if 'flag' in groupd and groupd['flag'] != None: > >> bb.data.setVarFlag(key, groupd['flag'], val, data) > >> elif groupd["lazyques"]: > >> - assigned = bb.data.getVar("__lazy_assigned", data) or [] > >> - assigned.append(key) > >> - bb.data.setVar("__lazy_assigned", assigned, data) > >> bb.data.setVarFlag(key, "defaultval", val, data) > >> + bb.data.setVar(key, None, data) > > > > This I think is incorrect. You actually wiped out previous assignment > > before ??=, and thus end up to always have default value favored > > unless there're other direct assignments after ??= evaluation. Just > > touch the flag should be enough. > > Ah, yes, I need to check whether the value has been setVar before, see following patch. > > This two lines are necessary in the following patch > > + if bb.data.getVarFlag(key, "content", data) is None: > + bb.data.setVar(key, None, data) > > because setVar will add the key into override list, which could not be > achieved by just calling "setVarFlag". For the later formal patch, I > will add comments on the two lines. This is a little worrying and I think we have insight into a new bug here which is that if you just have a variable: foo_OVERA[bar] = "x" and foo is never set as a variable directly, the override is never expanded. There is a strong argument here to make setVar(...) a special case of setVarFlag(..., "content") although I worry about the performance implications. Chris: Any opinion? Cheers, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-09 11:24 ` Richard Purdie @ 2010-12-09 12:13 ` Xu, Dongxiao 2010-12-09 12:45 ` Richard Purdie 0 siblings, 1 reply; 19+ messages in thread From: Xu, Dongxiao @ 2010-12-09 12:13 UTC (permalink / raw) To: Richard Purdie, Chris Larson; +Cc: poky@yoctoproject.org Richard Purdie wrote: > On Thu, 2010-12-09 at 15:59 +0800, Xu, Dongxiao wrote: >> Tian, Kevin wrote: >>>> From: Xu, Dongxiao >>>> Sent: Thursday, December 09, 2010 11:33 AM During the file parsing >>>> process, the getVar is called "1435156/1148654" >>>> times (actually I am still confused what "/" means...), and among >>>> them, 322121 times of getVar call return "None", thus about 1/3 or >>>> 1/4 of the total calls. > > Ok, that is a pretty significant portion of the time. > >>>> I tried to use the following patch to have a test. Also I paste a >>>> profile log. From the result we can see it has over 15% >>>> performance gain. (from >>>> 39.3 >>>> secs to 33.7 secs). > > So despite that, it still significantly helps performance. Could you > also give some raw performance numbers? By this I mean make the > timing runs with profiling turned off so work out the time it takes > before and after the patch with the profiling off so the numbers are > "real" with "time bitbake xxx"? > > It always pays just to sanity check and this gives the "real" > performance gain. Hi richard, Here are some raw timing result: w/ patch: time bitbake -p real 0m26.189s user 0m25.480s sys 0m0.420s w/o patch: time bitbake -p real 0m30.940s user 0m30.230s sys 0m0.460s Thanks, Dongxiao > >>>> In usermanual, the "??=" definition is: >>>> >>>> Setting a default value (??=) >>>> A ??= "somevalue" >>>> A ??= "someothervalue" >>>> If A is set before the above, it will retain that value. If A is >>>> unset prior to the above, A will be set to someothervalue. This is >>>> a lazy version of ?=, in that the assignment does not occur until >>>> the end of the parsing process, so that the last, rather than the >>>> first, ??= assignment to a given variable will be used. >>>> >>>> Our patch logic isn't strickly following the "??=" definition, >>>> since the assignment doesn't occurs in end of parsing process... >>>> Except this one, I think other behaviors for ??= do not change. >>> >>> I think it is, as long as a getVar on a variable happens after all >>> the evaluations of that variable which I think should be true or >>> else even current design has problem. In that way when evaluating >>> variable assignments, you keep 'defaultvalue' updated until the >>> last "??=" assignment. Then later a getVar() on that variable >>> happens which then >>> you just return the right defaultvalue. :-) >> >> Agree. > > I also agree, I think we can adjust the definition of how this is > handled slightly. There is also a change in behaviour as now, when > one of these variables is looked up, it will get the default value > rather than None if finalise() hasn't been called. I don't think this > is much of an issue though. > >>>> --- a/bitbake/lib/bb/parse/ast.py >>>> +++ b/bitbake/lib/bb/parse/ast.py >>>> @@ -109,10 +109,8 @@ class DataNode(AstNode): >>>> if 'flag' in groupd and groupd['flag'] != None: >>>> bb.data.setVarFlag(key, groupd['flag'], val, data) >>>> elif groupd["lazyques"]: >>>> - assigned = bb.data.getVar("__lazy_assigned", data) or >>>> [] >>>> - assigned.append(key) >>>> - bb.data.setVar("__lazy_assigned", assigned, data) >>>> bb.data.setVarFlag(key, "defaultval", val, data) >>>> + bb.data.setVar(key, None, data) >>> >>> This I think is incorrect. You actually wiped out previous >>> assignment before ??=, and thus end up to always have default value >>> favored unless there're other direct assignments after ??= >>> evaluation. Just touch the flag should be enough. >> >> Ah, yes, I need to check whether the value has been setVar before, >> see following patch. >> >> This two lines are necessary in the following patch >> >> + if bb.data.getVarFlag(key, "content", data) is None: >> + bb.data.setVar(key, None, data) >> >> because setVar will add the key into override list, which could not >> be >> achieved by just calling "setVarFlag". For the later formal patch, I >> will add comments on the two lines. > > This is a little worrying and I think we have insight into a new bug > here which is that if you just have a variable: > > foo_OVERA[bar] = "x" > > and foo is never set as a variable directly, the override is never > expanded. > > There is a strong argument here to make setVar(...) a special case of > setVarFlag(..., "content") although I worry about the performance > implications. > > Chris: Any opinion? > > Cheers, > > Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-09 12:13 ` Xu, Dongxiao @ 2010-12-09 12:45 ` Richard Purdie 2010-12-09 14:44 ` Xu, Dongxiao 0 siblings, 1 reply; 19+ messages in thread From: Richard Purdie @ 2010-12-09 12:45 UTC (permalink / raw) To: Xu, Dongxiao; +Cc: Chris Larson, poky@yoctoproject.org On Thu, 2010-12-09 at 20:13 +0800, Xu, Dongxiao wrote: > w/ patch: > time bitbake -p > > real 0m26.189s > user 0m25.480s > sys 0m0.420s > > w/o patch: > time bitbake -p > > real 0m30.940s > user 0m30.230s > sys 0m0.460s Thanks, thats 5/31 so about 15% which is nice and a confirmed speedup. Sometimes the profiling can distort things slightly so this is a good sanity check. Can you see what the impact is to parsing if we do all the work we do in setVar in setVarFlag too? I'm definitely in favour of the patch, just a few details to make sure of :) Cheers, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-09 12:45 ` Richard Purdie @ 2010-12-09 14:44 ` Xu, Dongxiao 2010-12-10 23:42 ` Richard Purdie 0 siblings, 1 reply; 19+ messages in thread From: Xu, Dongxiao @ 2010-12-09 14:44 UTC (permalink / raw) To: Richard Purdie; +Cc: Chris Larson, poky@yoctoproject.org Richard Purdie wrote: > On Thu, 2010-12-09 at 20:13 +0800, Xu, Dongxiao wrote: >> w/ patch: >> time bitbake -p >> >> real 0m26.189s >> user 0m25.480s >> sys 0m0.420s >> >> w/o patch: >> time bitbake -p >> >> real 0m30.940s >> user 0m30.230s >> sys 0m0.460s > > Thanks, thats 5/31 so about 15% which is nice and a confirmed speedup. > Sometimes the profiling can distort things slightly so this is a good > sanity check. > > Can you see what the impact is to parsing if we do all the work we do > in setVar in setVarFlag too? I made the following patch (setVarFlag.patch) to move everything in setVar to setVarFlag, please help to review it. Please note one place that I modified, see the indent of "self._seen_overrides[override].add( var )", I thought it was a bug in original code. After adding this, the "two lines" in original "??=" optimization patch are no longer needed. + if bb.data.getVarFlag(key, "content", data) is None: + bb.data.setVar(key, None, data) I rerun "time bitbake -p" with the two patches (see below, one is setVarFlag.patch, another is the "??=" optimization patch) The result is: real 0m27.049s user 0m26.340s sys 0m0.440s The parsing time is a bit longer ( < 1 sec) than without the following "setVarFlag.patch". Thanks, Dongxiao "setVarFlag.patch" ----------------------------------------------------------------------------------------------------- diff --git a/bitbake/lib/bb/data_smart.py b/bitbake/lib/bb/data_smart.py index b9d9476..3fe2395 100644 --- a/bitbake/lib/bb/data_smart.py +++ b/bitbake/lib/bb/data_smart.py @@ -210,38 +210,7 @@ class DataSmart: self.initVar(var) def setVar(self, var, value): - self.expand_cache = {} - match = __setvar_regexp__.match(var) - if match and match.group("keyword") in __setvar_keyword__: - base = match.group('base') - keyword = match.group("keyword") - override = match.group('add') - l = self.getVarFlag(base, keyword) or [] - l.append([value, override]) - self.setVarFlag(base, keyword, l) - - # todo make sure keyword is not __doc__ or __module__ - # pay the cookie monster - try: - self._special_values[keyword].add( base ) - except KeyError: - self._special_values[keyword] = set() - self._special_values[keyword].add( base ) - - return - - if not var in self.dict: - self._makeShadowCopy(var) - - # more cookies for the cookie monster - if '_' in var: - override = var[var.rfind('_')+1:] - if override not in self._seen_overrides: - self._seen_overrides[override] = set() - self._seen_overrides[override].add( var ) - - # setting var - self.dict[var]["content"] = value + self.setVarFlag(var, "content", value) def getVar(self, var, exp): value = self.getVarFlag(var, "content") @@ -278,8 +247,38 @@ class DataSmart: self.dict[var] = {} def setVarFlag(self, var, flag, flagvalue): + + self.expand_cache = {} + match = __setvar_regexp__.match(var) + if match and match.group("keyword") in __setvar_keyword__ and flag == "content": + base = match.group('base') + keyword = match.group("keyword") + override = match.group('add') + l = self.getVarFlag(base, keyword) or [] + l.append([flagvalue, override]) + self.setVarFlag(base, keyword, l) + + # todo make sure keyword is not __doc__ or __module__ + # pay the cookie monster + try: + self._special_values[keyword].add( base ) + except KeyError: + self._special_values[keyword] = set() + self._special_values[keyword].add( base ) + + return + + if not var in self.dict: self._makeShadowCopy(var) + + # more cookies for the cookie monster + if '_' in var: + override = var[var.rfind('_')+1:] + if override not in self._seen_overrides: + self._seen_overrides[override] = set() + self._seen_overrides[override].add( var ) + self.dict[var][flag] = flagvalue def getVarFlag(self, var, flag): ----------------------------------------------------------------------------------------------------- New "??=" optimization patch: ----------------------------------------------------------------------------------------------------- diff --git a/bitbake/lib/bb/data_smart.py b/bitbake/lib/bb/data_smart.py index 3fe2395..1b8020d 100644 --- a/bitbake/lib/bb/data_smart.py +++ b/bitbake/lib/bb/data_smart.py @@ -215,6 +215,9 @@ class DataSmart: def getVar(self, var, exp): value = self.getVarFlag(var, "content") + if value is None: + value = self.getVarFlag(var, "defaultval") + if exp and value: return self.expand(value, var) return value diff --git a/bitbake/lib/bb/parse/ast.py b/bitbake/lib/bb/parse/ast.py index 870ae65..cace9ab 100644 --- a/bitbake/lib/bb/parse/ast.py +++ b/bitbake/lib/bb/parse/ast.py @@ -109,9 +109,6 @@ class DataNode(AstNode): if 'flag' in groupd and groupd['flag'] != None: bb.data.setVarFlag(key, groupd['flag'], val, data) elif groupd["lazyques"]: - assigned = bb.data.getVar("__lazy_assigned", data) or [] - assigned.append(key) - bb.data.setVar("__lazy_assigned", assigned, data) bb.data.setVarFlag(key, "defaultval", val, data) else: bb.data.setVar(key, val, data) @@ -301,10 +298,6 @@ def handleInherit(statements, m): statements.append(InheritNode(m.group(1))) def finalize(fn, d, variant = None): - for lazykey in bb.data.getVar("__lazy_assigned", d) or (): - if bb.data.getVar(lazykey, d) is None: - val = bb.data.getVarFlag(lazykey, "defaultval", d) - bb.data.setVar(lazykey, val, d) bb.data.expandKeys(d) bb.data.update_data(d) ----------------------------------------------------------------------------------------------------- > > I'm definitely in favour of the patch, just a few details to make > sure of :) > > Cheers, > > Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-09 14:44 ` Xu, Dongxiao @ 2010-12-10 23:42 ` Richard Purdie 2010-12-10 23:58 ` Chris Larson 2010-12-11 2:48 ` Xu, Dongxiao 0 siblings, 2 replies; 19+ messages in thread From: Richard Purdie @ 2010-12-10 23:42 UTC (permalink / raw) To: Xu, Dongxiao; +Cc: Chris Larson, poky@yoctoproject.org On Thu, 2010-12-09 at 22:44 +0800, Xu, Dongxiao wrote: > I made the following patch (setVarFlag.patch) to move everything in > setVar to setVarFlag, please help to review it. > > Please note one place that I modified, see the indent of > "self._seen_overrides[override].add( var )", I thought it was a bug in > original code. No, its not. Its making sure [override] exists before trying to add var to it. It could also be: if override not in self._seen_overrides: self._seen_overrides[override] = set(var) else: self._seen_overrides[override].add( var ) > After adding this, the "two lines" in original "??=" optimization > patch are no longer needed. > > + if bb.data.getVarFlag(key, "content", data) is None: > + bb.data.setVar(key, None, data) > > I rerun "time bitbake -p" with the two patches (see below, one is > setVarFlag.patch, another is the "??=" optimization patch) > The result is: > > real 0m27.049s > user 0m26.340s > sys 0m0.440s > > The parsing time is a bit longer ( < 1 sec) than without the following > "setVarFlag.patch". Patches look good to me other than the above. I also talked to Chris on irc on #oe earlier today and we agreed this is a good move for setVar bringing both into line although the whole way the datastore works could still be better. Could you send these patches through a pull request please and lets get them merged. Cheers, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-10 23:42 ` Richard Purdie @ 2010-12-10 23:58 ` Chris Larson 2010-12-13 16:24 ` Scott Garman 2010-12-11 2:48 ` Xu, Dongxiao 1 sibling, 1 reply; 19+ messages in thread From: Chris Larson @ 2010-12-10 23:58 UTC (permalink / raw) To: Richard Purdie; +Cc: poky@yoctoproject.org On Fri, Dec 10, 2010 at 4:42 PM, Richard Purdie <rpurdie@linux.intel.com> wrote: > On Thu, 2010-12-09 at 22:44 +0800, Xu, Dongxiao wrote: >> I made the following patch (setVarFlag.patch) to move everything in >> setVar to setVarFlag, please help to review it. >> >> Please note one place that I modified, see the indent of >> "self._seen_overrides[override].add( var )", I thought it was a bug in >> original code. > > No, its not. Its making sure [override] exists before trying to add var > to it. It could also be: > > if override not in self._seen_overrides: > self._seen_overrides[override] = set(var) > else: > self._seen_overrides[override].add( var ) > For master, we can use defaultdict for that sort of thing. What's poky's current python version requirement? -- Christopher Larson clarson at kergoth dot com Founder - BitBake, OpenEmbedded, OpenZaurus Maintainer - Tslib Senior Software Engineer, Mentor Graphics ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-10 23:58 ` Chris Larson @ 2010-12-13 16:24 ` Scott Garman 0 siblings, 0 replies; 19+ messages in thread From: Scott Garman @ 2010-12-13 16:24 UTC (permalink / raw) To: Chris Larson; +Cc: poky@yoctoproject.org On 12/10/2010 03:58 PM, Chris Larson wrote: > On Fri, Dec 10, 2010 at 4:42 PM, Richard Purdie<rpurdie@linux.intel.com> wrote: >> On Thu, 2010-12-09 at 22:44 +0800, Xu, Dongxiao wrote: >>> I made the following patch (setVarFlag.patch) to move everything in >>> setVar to setVarFlag, please help to review it. >>> >>> Please note one place that I modified, see the indent of >>> "self._seen_overrides[override].add( var )", I thought it was a bug in >>> original code. >> >> No, its not. Its making sure [override] exists before trying to add var >> to it. It could also be: >> >> if override not in self._seen_overrides: >> self._seen_overrides[override] = set(var) >> else: >> self._seen_overrides[override].add( var ) >> > > For master, we can use defaultdict for that sort of thing. What's > poky's current python version requirement? 2.6, so using defaultdict should be fine. Scott -- Scott Garman Embedded Linux Distro Engineer - Yocto Project ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-10 23:42 ` Richard Purdie 2010-12-10 23:58 ` Chris Larson @ 2010-12-11 2:48 ` Xu, Dongxiao 1 sibling, 0 replies; 19+ messages in thread From: Xu, Dongxiao @ 2010-12-11 2:48 UTC (permalink / raw) To: Richard Purdie; +Cc: Chris Larson, poky@yoctoproject.org Richard Purdie wrote: > On Thu, 2010-12-09 at 22:44 +0800, Xu, Dongxiao wrote: >> I made the following patch (setVarFlag.patch) to move everything in >> setVar to setVarFlag, please help to review it. >> >> Please note one place that I modified, see the indent of >> "self._seen_overrides[override].add( var )", I thought it was a bug >> in original code. > > No, its not. Its making sure [override] exists before trying to add > var to it. It could also be: > > if override not in self._seen_overrides: > self._seen_overrides[override] = set(var) > else: > self._seen_overrides[override].add( var ) Thanks, I understand now. For overrides, we only override the variable flag "content", and there is no concept for overriding the other variable flags. Does my understanding correct? If this is the case, we should add the following constraint. + if override not in self._seen_overrides: + self._seen_overrides[override] = set() + if flag == "content": + self._seen_overrides[override].add( var ) Otherwise, each time when setVarFlag() for a certain key, all flag values will will be added to self._seen_overrides[override], and for different flags, the value may be different types, and there will be errors like: NOTE: Handling BitBake files: | (0116/0765) [15 %]ERROR: cannot concatenate 'str' and 'NoneType' objects while parsing /home/dongxiao/poky/meta/recipes-devtools/dpkg/dpkg_1.15.8.5.bb. > >> After adding this, the "two lines" in original "??=" optimization >> patch are no longer needed. >> >> + if bb.data.getVarFlag(key, "content", data) is None: >> + bb.data.setVar(key, None, data) >> >> I rerun "time bitbake -p" with the two patches (see below, one is >> setVarFlag.patch, another is the "??=" optimization patch) The >> result is: >> >> real 0m27.049s >> user 0m26.340s >> sys 0m0.440s >> >> The parsing time is a bit longer ( < 1 sec) than without the >> following "setVarFlag.patch". > > Patches look good to me other than the above. I also talked to Chris > on irc on #oe earlier today and we agreed this is a good move for > setVar bringing both into line although the whole way the datastore > works could still be better. > > Could you send these patches through a pull request please and lets > get them merged. OK, I will make a pull request after the above question got clear. Thanks, Dongxiao > > Cheers, > > Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-08 11:35 ` Tian, Kevin 2010-12-08 11:59 ` Xu, Dongxiao @ 2010-12-08 12:49 ` Tian, Kevin 2010-12-08 13:02 ` Xu, Dongxiao 1 sibling, 1 reply; 19+ messages in thread From: Tian, Kevin @ 2010-12-08 12:49 UTC (permalink / raw) To: Tian, Kevin, Richard Purdie, Xu, Dongxiao; +Cc: poky@yoctoproject.org >From: Tian, Kevin >Sent: Wednesday, December 08, 2010 7:35 PM > >>From: Richard Purdie >>Sent: Wednesday, December 08, 2010 6:30 PM >> >>Hi Dongxiao, >> >>On Wed, 2010-12-01 at 09:57 +0800, Xu, Dongxiao wrote: >>> Recently when investigating the file parsing speed, I found the >>> implementation of "??=" is another hot spot. This operator will not >>> apply the default assignment until the end of the parse, which is >>> different from "?=". According to current poky, the users for "??=" is >>> mostly the scm revisions stored in poky-default-revisions.inc. >>> >>> Currently the final assignment logic for "??=" is added in finalize(), >>> which costs about 20% parsing time. >> >>Can I take a look at the graph showing that? I'm a little surprised it >>takes 20% of the time, that is a rather scary amount! >> > >I don't have the graph, but did do a simple test by printing the lazy value >list in finalize(). There're 166 items in total. Regarding finalize() may be >invoked multiple times for same recipe and we have over 700 recipes scanned >in a fresh run, it may contribute obvious overhead just like what Dongxiao >previously optimized for distro tracking fields. > >I think we could remove the lazy key from the list after grabbing it as the >default value, and is now testing it... > I have a simple patch: diff --git a/bitbake/lib/bb/parse/ast.py b/bitbake/lib/bb/parse/ast.py index 870ae65..4b98799 100644 --- a/bitbake/lib/bb/parse/ast.py +++ b/bitbake/lib/bb/parse/ast.py @@ -301,10 +301,13 @@ def handleInherit(statements, m): statements.append(InheritNode(m.group(1))) def finalize(fn, d, variant = None): + assigned = bb.data.getVar("__lazy_assigned", d) or () for lazykey in bb.data.getVar("__lazy_assigned", d) or (): if bb.data.getVar(lazykey, d) is None: val = bb.data.getVarFlag(lazykey, "defaultval", d) bb.data.setVar(lazykey, val, d) + assigned.remove(lazykey) + bb.data.setVar("__lazy_assigned", assigned, d) bb.data.expandKeys(d) bb.data.update_data(d) However it doesn't work as I expect. Every time the lazy list always contains 166 items. It looks that each finalize() is invoked in different database context, while I haven't found the global copy yet. Or did I make some silly mistake here? :/ Thanks Kevin ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: About the operator "??=" 2010-12-08 12:49 ` Tian, Kevin @ 2010-12-08 13:02 ` Xu, Dongxiao 0 siblings, 0 replies; 19+ messages in thread From: Xu, Dongxiao @ 2010-12-08 13:02 UTC (permalink / raw) To: Tian, Kevin, Richard Purdie; +Cc: poky@yoctoproject.org Tian, Kevin wrote: >> From: Tian, Kevin >> Sent: Wednesday, December 08, 2010 7:35 PM >> >>> From: Richard Purdie >>> Sent: Wednesday, December 08, 2010 6:30 PM >>> >>> Hi Dongxiao, >>> >>> On Wed, 2010-12-01 at 09:57 +0800, Xu, Dongxiao wrote: >>>> Recently when investigating the file parsing speed, I found the >>>> implementation of "??=" is another hot spot. This operator will not >>>> apply the default assignment until the end of the parse, which is >>>> different from "?=". According to current poky, the users for "??=" >>>> is mostly the scm revisions stored in poky-default-revisions.inc. >>>> >>>> Currently the final assignment logic for "??=" is added in >>>> finalize(), which costs about 20% parsing time. >>> >>> Can I take a look at the graph showing that? I'm a little surprised >>> it takes 20% of the time, that is a rather scary amount! >>> >> >> I don't have the graph, but did do a simple test by printing the lazy >> value list in finalize(). There're 166 items in total. Regarding >> finalize() may be invoked multiple times for same recipe and we have >> over 700 recipes scanned in a fresh run, it may contribute obvious >> overhead just like what Dongxiao previously optimized for distro >> tracking fields. >> >> I think we could remove the lazy key from the list after grabbing it >> as >> the default value, and is now testing it... >> > > I have a simple patch: > > diff --git a/bitbake/lib/bb/parse/ast.py > b/bitbake/lib/bb/parse/ast.py index 870ae65..4b98799 100644 --- > a/bitbake/lib/bb/parse/ast.py +++ b/bitbake/lib/bb/parse/ast.py > @@ -301,10 +301,13 @@ def handleInherit(statements, m): > statements.append(InheritNode(m.group(1))) > > def finalize(fn, d, variant = None): > + assigned = bb.data.getVar("__lazy_assigned", d) or () > for lazykey in bb.data.getVar("__lazy_assigned", d) or (): > if bb.data.getVar(lazykey, d) is None: > val = bb.data.getVarFlag(lazykey, "defaultval", d) > bb.data.setVar(lazykey, val, d) > + assigned.remove(lazykey) > + bb.data.setVar("__lazy_assigned", assigned, d) > > bb.data.expandKeys(d) > bb.data.update_data(d) > > However it doesn't work as I expect. Every time the lazy list always > contains 166 items. It looks that each finalize() is invoked in > different database context, while I haven't found the global copy > yet. Or did I make some silly mistake here? :/ Per my understanding, "d" is an object instance, each time when doing finalize, "d" is a different instance. Therefore each round "d" will always have 166 items. Thanks, Dongxiao > > Thanks > Kevin ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: About the operator "??="
@ 2010-12-08 3:00 Xu, Dongxiao
0 siblings, 0 replies; 19+ messages in thread
From: Xu, Dongxiao @ 2010-12-08 3:00 UTC (permalink / raw)
To: Richard Purdie; +Cc: poky@yoctoproject.org
Hi Richard,
Do you have comments on this one?
Thanks,
Dongxiao
Xu, Dongxiao wrote:
> Hi Richard,
>
> Recently when investigating the file parsing speed, I found the
> implementation of "??=" is another hot spot. This operator will not
> apply the default assignment until the end of the parse, which is
> different from "?=". According to current poky, the users for "??="
> is mostly the scm revisions stored in poky-default-revisions.inc.
>
> Currently the final assignment logic for "??=" is added in
> finalize(), which costs about 20% parsing time.
>
> The differences between "??=" and "?=" are straight forward, however
> I don't fully understand in which senario the "??=" should be used.
> Or in other word, why those scm revisions should use "??=". Could you
> help to clarify a bit? Thanks so much!
>
> BTW, I noticed that openembedded doesn't use this operator.
>
> Best Regards,
> -- Dongxiao
^ permalink raw reply [flat|nested] 19+ messages in threadend of thread, other threads:[~2010-12-13 16:24 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-12-01 1:57 About the operator "??=" Xu, Dongxiao 2010-12-08 10:30 ` Richard Purdie 2010-12-08 11:35 ` Tian, Kevin 2010-12-08 11:59 ` Xu, Dongxiao 2010-12-08 14:37 ` Richard Purdie 2010-12-09 3:32 ` Xu, Dongxiao 2010-12-09 4:57 ` Tian, Kevin 2010-12-09 7:59 ` Xu, Dongxiao 2010-12-09 11:24 ` Richard Purdie 2010-12-09 12:13 ` Xu, Dongxiao 2010-12-09 12:45 ` Richard Purdie 2010-12-09 14:44 ` Xu, Dongxiao 2010-12-10 23:42 ` Richard Purdie 2010-12-10 23:58 ` Chris Larson 2010-12-13 16:24 ` Scott Garman 2010-12-11 2:48 ` Xu, Dongxiao 2010-12-08 12:49 ` Tian, Kevin 2010-12-08 13:02 ` Xu, Dongxiao -- strict thread matches above, loose matches on Subject: below -- 2010-12-08 3:00 Xu, Dongxiao
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.