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