[PATCH 0/2] fixes for cache and parsing

[PATCH 0/2] fixes for cache and parsing

[Robert Yang]

The following changes since commit a783bd7e457f183a279dbe5a4ef3d4c17bb4c18c:

  bitbake-user-manual: Removed stray parenthesis (2018-01-16 20:38:53 +0000)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib rbt/cache
  http://git.pokylinux.org/cgit.cgi//log/?h=rbt/cache

Robert Yang (2):
  cache.py: improve debug message
  parse: don't add attempted files to dependencies

 lib/bb/cache.py                      | 5 ++++-
 lib/bb/parse/__init__.py             | 2 --
 lib/bb/parse/parse_py/BBHandler.py   | 3 ---
 lib/bb/parse/parse_py/ConfHandler.py | 2 --
 4 files changed, 4 insertions(+), 8 deletions(-)

-- 
2.7.4

[PATCH 1/2] cache.py: improve debug message

[Robert Yang]

* Print message when cachefile is found/not can help debug.
* Update "Using cache in" to "Cache dir:" since it was the same as the debug
  message of "codeparser & file checksum caches", which caused confusion. And
  whether the cache file will be used or not is still unknown at that time, so
  just print the cache dir.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 lib/bb/cache.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/bb/cache.py b/lib/bb/cache.py
index 86ce0e7..168a77a 100644
--- a/lib/bb/cache.py
+++ b/lib/bb/cache.py
@@ -395,7 +395,7 @@ class Cache(NoCache):
         self.has_cache = True
         self.cachefile = getCacheFile(self.cachedir, "bb_cache.dat", self.data_hash)
 
-        logger.debug(1, "Using cache in '%s'", self.cachedir)
+        logger.debug(1, "Cache dir: %s", self.cachedir)
         bb.utils.mkdirhier(self.cachedir)
 
         cache_ok = True
@@ -408,6 +408,8 @@ class Cache(NoCache):
             self.load_cachefile()
         elif os.path.isfile(self.cachefile):
             logger.info("Out of date cache found, rebuilding...")
+        else:
+            logger.debug(1, "Cache file %s not found, building..." % self.cachefile)
 
     def load_cachefile(self):
         cachesize = 0
@@ -424,6 +426,7 @@ class Cache(NoCache):
 
         for cache_class in self.caches_array:
             cachefile = getCacheFile(self.cachedir, cache_class.cachefile, self.data_hash)
+            logger.debug(1, 'Loading cache file: %s' % cachefile)
             with open(cachefile, "rb") as cachefile:
                 pickled = pickle.Unpickler(cachefile)
                 # Check cache version information
-- 
2.7.4

[PATCH 2/2] parse: don't add attempted files to dependencies

[Robert Yang]

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(-)

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):
-- 
2.7.4

Re: [PATCH 2/2] parse: don't add attempted files to dependencies

[Richard Purdie]

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

Re: [PATCH 2/2] parse: don't add attempted files to dependencies

[Robert Yang]

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
>