Skip to contentSkip to conversation

fixes for cache and parsing

PATCH In review Robert Yang 2 participants 5 messages Jan 18, 2018 – Feb 1, 2018

lore
Share & tools
lore
#0 lore raw

cover fixes for cache and parsing

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(-)
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/2 cache.py: improve debug message

* 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.

---
 lib/bb/cache.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
lib/bb/cache.py +4 -1
index 86ce0e7..168a77a 100644
@@ -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
Signature · 1 line
2.7.4
#2 lore raw

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

In reply to 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.

---
 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(-)
lib/bb/parse/__init__.py +0 -2
index 2fc4002..a96124f 100644
@@ -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
lib/bb/parse/parse_py/BBHandler.py +0 -3
index f89ad24..48804e9 100644
@@ -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
 
lib/bb/parse/parse_py/ConfHandler.py +0 -2
index 97aa130..ea3eba0 100644
@@ -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):
Signature · 1 line
2.7.4
#3 lore raw

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

Quoted 16 lines · 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.

Quoted 6 lines

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

#4 lore raw

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

Hi RP,

Quoted 30 lines · Richard Purdie
Quoted 16 lines · 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.

Quoted 6 lines

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.

Quoted 3 lines

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

Quoted 5 lines

Cheers,

Richard