[poky] 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