About the operator "??="

About the operator "??="

[Xu, Dongxiao]

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 

Re: About the operator "??="

[Xu, Dongxiao]

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

Re: About the operator "??="

[Richard Purdie]

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

Re: About the operator "??="

[Tian, Kevin]

>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

Re: About the operator "??="

[Xu, Dongxiao]

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

Re: About the operator "??="

[Tian, Kevin]

>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

Re: About the operator "??="

[Xu, Dongxiao]

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

Re: About the operator "??="

[Richard Purdie]

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

Re: About the operator "??="

[Xu, Dongxiao]

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

Re: About the operator "??="

[Tian, Kevin]

>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

Re: About the operator "??="

[Xu, Dongxiao]

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

Re: About the operator "??="

[Richard Purdie]

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

Re: About the operator "??="

[Xu, Dongxiao]

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

Re: About the operator "??="

[Richard Purdie]

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

Re: About the operator "??="

[Xu, Dongxiao]

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

Re: About the operator "??="

[Richard Purdie]

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

Re: About the operator "??="

[Chris Larson]

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

Re: About the operator "??="

[Xu, Dongxiao]

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

Re: About the operator "??="

[Scott Garman]

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