[bitbake-devel,2/2] parse: don't add attempted files to dependencies

Submitted by Robert Yang on Jan. 18, 2018, 8:57 a.m. | Patch ID: 147386

Details

Message ID 9a21e82b35b935826dd28413684f0918c00f9eb0.1516265726.git.liezhi.yang@windriver.com
State New
Headers show

Commit Message

Robert Yang Jan. 18, 2018, 8:57 a.m.
The attempts are the files that it tries to search but don't exist, it
searches the file in BBPATH until find it, so the attempts might be very long
when there are many layers, which causes bb_cache.dat very big (I have 54 layers,
the bb_cache.dat can be 119M, and even much bigger depends on the order of BBLAYERS).

Here is the testing data of 54 layers before and after the patch.

                    Before          After
Parsing time        33s             9s
Loading time        30s             5s
Cache size          119M            20M

The time and size can be more or less depends on the order of BBLAYERS before
the patch.

I checked the code, but didn't find why we need the attempts as dependencies,
the one that I can think of is the file doesn't exist when parsing, and added
to the attempts location after parsing, but the parseBaseConfiguration() can
detect and handle this well (and will cause reparse in such a case), so I think
that we can safely remove them from dependencies since they have side effects.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 lib/bb/parse/__init__.py             | 2 --
 lib/bb/parse/parse_py/BBHandler.py   | 3 ---
 lib/bb/parse/parse_py/ConfHandler.py | 2 --
 3 files changed, 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/bb/parse/__init__.py b/lib/bb/parse/__init__.py
index 2fc4002..a96124f 100644
--- a/lib/bb/parse/__init__.py
+++ b/lib/bb/parse/__init__.py
@@ -129,8 +129,6 @@  def resolve_file(fn, d):
     if not os.path.isabs(fn):
         bbpath = d.getVar("BBPATH")
         newfn, attempts = bb.utils.which(bbpath, fn, history=True)
-        for af in attempts:
-            mark_dependency(d, af)
         if not newfn:
             raise IOError(errno.ENOENT, "file %s not found in %s" % (fn, bbpath))
         fn = newfn
diff --git a/lib/bb/parse/parse_py/BBHandler.py b/lib/bb/parse/parse_py/BBHandler.py
index f89ad24..48804e9 100644
--- a/lib/bb/parse/parse_py/BBHandler.py
+++ b/lib/bb/parse/parse_py/BBHandler.py
@@ -68,9 +68,6 @@  def inherit(files, fn, lineno, d):
         if not os.path.isabs(file):
             bbpath = d.getVar("BBPATH")
             abs_fn, attempts = bb.utils.which(bbpath, file, history=True)
-            for af in attempts:
-                if af != abs_fn:
-                    bb.parse.mark_dependency(d, af)
             if abs_fn:
                 file = abs_fn
 
diff --git a/lib/bb/parse/parse_py/ConfHandler.py b/lib/bb/parse/parse_py/ConfHandler.py
index 97aa130..ea3eba0 100644
--- a/lib/bb/parse/parse_py/ConfHandler.py
+++ b/lib/bb/parse/parse_py/ConfHandler.py
@@ -95,8 +95,6 @@  def include_single_file(parentfn, fn, lineno, data, error_out):
         abs_fn, attempts = bb.utils.which(bbpath, fn, history=True)
         if abs_fn and bb.parse.check_dependency(data, abs_fn):
             logger.warning("Duplicate inclusion for %s in %s" % (abs_fn, data.getVar('FILE')))
-        for af in attempts:
-            bb.parse.mark_dependency(data, af)
         if abs_fn:
             fn = abs_fn
     elif bb.parse.check_dependency(data, fn):

Comments

Richard Purdie Jan. 19, 2018, 11:16 a.m.
On Thu, 2018-01-18 at 16:57 +0800, Robert Yang wrote:
> The attempts are the files that it tries to search but don't exist,
> it searches the file in BBPATH until find it, so the attempts might
> be very long when there are many layers, which causes bb_cache.dat
> very big (I have 54 layers, the bb_cache.dat can be 119M, and even
> much bigger depends on the order of BBLAYERS).
> 
> Here is the testing data of 54 layers before and after the patch.
> 
>                     Before          After
> Parsing time        33s             9s
> Loading time        30s             5s
> Cache size          119M            20M
> 
> The time and size can be more or less depends on the order of
> BBLAYERS before the patch.
> 
> I checked the code, but didn't find why we need the attempts as
> dependencies, the one that I can think of is the file doesn't exist
> when parsing, and added to the attempts location after parsing, but
> the parseBaseConfiguration() can detect and handle this well (and
> will cause reparse in such a case), so I think that we can safely
> remove them from dependencies since they have side effects.

If you have a memory resident bitbake you need to know which files to
watch for with inotify. If a file is created somewhere which would have
been parsed had it existed when we originally parsed, we need to
invalidate the cache and reparse. This needs to happen for the metadata
itself, not just the base configuration.

I therefore strongly suspect we do need these unfortunately. There may
be some ways we can condense the dependency information though to
reduce the cache load times.

Cheers,

Richard
Robert Yang Feb. 1, 2018, 11:53 a.m.
Hi RP,

On 01/19/2018 07:16 PM, Richard Purdie wrote:
> On Thu, 2018-01-18 at 16:57 +0800, Robert Yang wrote:
>> The attempts are the files that it tries to search but don't exist,
>> it searches the file in BBPATH until find it, so the attempts might
>> be very long when there are many layers, which causes bb_cache.dat
>> very big (I have 54 layers, the bb_cache.dat can be 119M, and even
>> much bigger depends on the order of BBLAYERS).
>>
>> Here is the testing data of 54 layers before and after the patch.
>>
>>                      Before          After
>> Parsing time        33s             9s
>> Loading time        30s             5s
>> Cache size          119M            20M
>>
>> The time and size can be more or less depends on the order of
>> BBLAYERS before the patch.
>>
>> I checked the code, but didn't find why we need the attempts as
>> dependencies, the one that I can think of is the file doesn't exist
>> when parsing, and added to the attempts location after parsing, but
>> the parseBaseConfiguration() can detect and handle this well (and
>> will cause reparse in such a case), so I think that we can safely
>> remove them from dependencies since they have side effects.
> 
> If you have a memory resident bitbake you need to know which files to
> watch for with inotify. If a file is created somewhere which would have
> been parsed had it existed when we originally parsed, we need to
> invalidate the cache and reparse. This needs to happen for the metadata
> itself, not just the base configuration.
> 
> I therefore strongly suspect we do need these unfortunately. There may
> be some ways we can condense the dependency information though to
> reduce the cache load times.

Yes, you're right, after a lot of investigations, I found that the bottle
neck is add_info() calls add_filewatch(), and add_filewatch runs too many
loops inside, but most of them are not needed, I've reduced the size
passed to add_filewatch(), which makes us save a lot of parse time,
but the size of bb_cache.dat can't be reduced since it needs save
all the attempted files. It seems that we can't get rid of
that for memory resident bitbake, but I think that it's fine since parsing
time is reduced. I will send the patches after more testing.

// Robert

> 
> Cheers,
> 
> Richard
>