[PATCH] cooker: bitbake memres server should not always reload the cache

[PATCH] cooker: bitbake memres server should not always reload the cache

[Cristiana Voicu]

Due to commit dd15648fc2654b8d7c3e00ea7ab3dbf04f24f24b, after an async
command is executed, the state of the server is set back to state.initial,
so the cache is always reloaded.
This patch adds a sanity check on the validity of the cache and then only
reload if something has changed.
depends_cache variable is needed in this case, so the del statement needs
to be removed.

[YOCTO #5603]
Signed-off-by: Cristiana Voicu <cristiana.voicu@intel.com>
---
 bitbake/lib/bb/cache.py  |    2 --
 bitbake/lib/bb/cooker.py |   32 ++++++++++++++++++++++----------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/bitbake/lib/bb/cache.py b/bitbake/lib/bb/cache.py
index 318781b..da9ad5d 100644
--- a/bitbake/lib/bb/cache.py
+++ b/bitbake/lib/bb/cache.py
@@ -612,8 +612,6 @@ class Cache(object):
                     cache_class_name = cache_class.__name__
                     file_dict[cache_class_name].close()
 
-        del self.depends_cache
-
     @staticmethod
     def mtime(cachefile):
         return bb.parse.cached_mtime_noerror(cachefile)
diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py
index 07202e3..c90bb44 100644
--- a/bitbake/lib/bb/cooker.py
+++ b/bitbake/lib/bb/cooker.py
@@ -1265,21 +1265,33 @@ class BBCooker:
             raise bb.BBHandledException()
 
         if self.state != state.parsing:
-            self.parseConfiguration ()
+            check_cache = False
+            if self.parser:
+                self.parser.bb_cache.checked = set()
+                for filename in self.parser.filelist:
+                    appends = self.collection.get_file_appends(filename)
+                    if not self.parser.bb_cache.cacheValid(filename, appends):
+                        check_cache = True
+                        break
+            else:
+                check_cache = True
 
-            ignore = self.data.getVar("ASSUME_PROVIDED", True) or ""
-            self.recipecache.ignored_dependencies = set(ignore.split())
+            if check_cache:
+                self.parseConfiguration ()
+
+                ignore = self.data.getVar("ASSUME_PROVIDED", True) or ""
+                self.recipecache.ignored_dependencies = set(ignore.split())
 
-            for dep in self.configuration.extra_assume_provided:
-                self.recipecache.ignored_dependencies.add(dep)
+                for dep in self.configuration.extra_assume_provided:
+                    self.recipecache.ignored_dependencies.add(dep)
 
-            self.collection = CookerCollectFiles(self.recipecache.bbfile_config_priorities)
-            (filelist, masked) = self.collection.collect_bbfiles(self.data, self.event_data)
+                self.collection = CookerCollectFiles(self.recipecache.bbfile_config_priorities)
+                (filelist, masked) = self.collection.collect_bbfiles(self.data, self.event_data)
 
-            self.data.renameVar("__depends", "__base_depends")
+                self.data.renameVar("__depends", "__base_depends")
 
-            self.parser = CookerParser(self, filelist, masked)
-            self.state = state.parsing
+                self.parser = CookerParser(self, filelist, masked)
+                self.state = state.parsing
 
         if not self.parser.parse_next():
             collectlog.debug(1, "parsing complete")
-- 
1.7.9.5

Re: [PATCH] cooker: bitbake memres server should not always reload the cache

[Alex Damian]

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

Please hold merging this change.

I think we need to discuss a bit on this. My expectation is that the
Bitbake server will reinit its state between builds, re-parsing the
configuration files and reload the cache.
I am not sure why we would consider maintaining the hot cache copy in
memory between builds.

The patch for bug
https://bugzilla.yoctoproject.org/show_bug.cgi?id=5535will re-create
the exact same scenario that we try to change through this
patch, so
I think we need to clarify the issue before moving forward.

Thanks,
Alex


On Wed, Mar 26, 2014 at 8:05 AM, Cristiana Voicu
<cristiana.voicu@intel.com>wrote:

> Due to commit dd15648fc2654b8d7c3e00ea7ab3dbf04f24f24b, after an async
> command is executed, the state of the server is set back to state.initial,
> so the cache is always reloaded.
> This patch adds a sanity check on the validity of the cache and then only
> reload if something has changed.
> depends_cache variable is needed in this case, so the del statement needs
> to be removed.
>
> [YOCTO #5603]
> Signed-off-by: Cristiana Voicu <cristiana.voicu@intel.com>
> ---
>  bitbake/lib/bb/cache.py  |    2 --
>  bitbake/lib/bb/cooker.py |   32 ++++++++++++++++++++++----------
>  2 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/bitbake/lib/bb/cache.py b/bitbake/lib/bb/cache.py
> index 318781b..da9ad5d 100644
> --- a/bitbake/lib/bb/cache.py
> +++ b/bitbake/lib/bb/cache.py
> @@ -612,8 +612,6 @@ class Cache(object):
>                      cache_class_name = cache_class.__name__
>                      file_dict[cache_class_name].close()
>
> -        del self.depends_cache
> -
>      @staticmethod
>      def mtime(cachefile):
>          return bb.parse.cached_mtime_noerror(cachefile)
> diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py
> index 07202e3..c90bb44 100644
> --- a/bitbake/lib/bb/cooker.py
> +++ b/bitbake/lib/bb/cooker.py
> @@ -1265,21 +1265,33 @@ class BBCooker:
>              raise bb.BBHandledException()
>
>          if self.state != state.parsing:
> -            self.parseConfiguration ()
> +            check_cache = False
> +            if self.parser:
> +                self.parser.bb_cache.checked = set()
> +                for filename in self.parser.filelist:
> +                    appends = self.collection.get_file_appends(filename)
> +                    if not self.parser.bb_cache.cacheValid(filename,
> appends):
> +                        check_cache = True
> +                        break
> +            else:
> +                check_cache = True
>
> -            ignore = self.data.getVar("ASSUME_PROVIDED", True) or ""
> -            self.recipecache.ignored_dependencies = set(ignore.split())
> +            if check_cache:
> +                self.parseConfiguration ()
> +
> +                ignore = self.data.getVar("ASSUME_PROVIDED", True) or ""
> +                self.recipecache.ignored_dependencies =
> set(ignore.split())
>
> -            for dep in self.configuration.extra_assume_provided:
> -                self.recipecache.ignored_dependencies.add(dep)
> +                for dep in self.configuration.extra_assume_provided:
> +                    self.recipecache.ignored_dependencies.add(dep)
>
> -            self.collection =
> CookerCollectFiles(self.recipecache.bbfile_config_priorities)
> -            (filelist, masked) =
> self.collection.collect_bbfiles(self.data, self.event_data)
> +                self.collection =
> CookerCollectFiles(self.recipecache.bbfile_config_priorities)
> +                (filelist, masked) =
> self.collection.collect_bbfiles(self.data, self.event_data)
>
> -            self.data.renameVar("__depends", "__base_depends")
> +                self.data.renameVar("__depends", "__base_depends")
>
> -            self.parser = CookerParser(self, filelist, masked)
> -            self.state = state.parsing
> +                self.parser = CookerParser(self, filelist, masked)
> +                self.state = state.parsing
>
>          if not self.parser.parse_next():
>              collectlog.debug(1, "parsing complete")
> --
> 1.7.9.5
>
> --
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/bitbake-devel
>

[-- Attachment #2: Type: text/html, Size: 5361 bytes --]

Re: [PATCH] cooker: bitbake memres server should not always reload the cache

[Jason Wessel]

[-- Attachment #1: Type: text/plain, Size: 5471 bytes --]

On 03/28/2014 06:43 AM, Alex Damian wrote:
> Please hold merging this change.
>
> I think we need to discuss a bit on this. My expectation is that the Bitbake server will reinit its state between builds, re-parsing the configuration files and reload the cache.
> I am not sure why we would consider maintaining the hot cache copy in memory between builds.
>


It is about increasing the interactivity of the system.  There are certainly times you need to soft reset, but perhaps that is what we need to expose.  If I want to run a dev shell and have done a build, this is something I'd like to be instantaneous, or if I am working on the package and want to re-run the compile and package steps, there really is no need to parse. 

There is a trade off for the individual package work vs the work of rebuilding the entire image.  In the case of the entire image you most certainly want the soft reset behavior.

Just some food for thought.

Cheers,
Jason.


> The patch for bug https://bugzilla.yoctoproject.org/show_bug.cgi?id=5535 will re-create the exact same scenario that we try to change through this patch, so
> I think we need to clarify the issue before moving forward.
>
> Thanks,
> Alex
>
>
> On Wed, Mar 26, 2014 at 8:05 AM, Cristiana Voicu <cristiana.voicu@intel.com <mailto:cristiana.voicu@intel.com>> wrote:
>
>     Due to commit dd15648fc2654b8d7c3e00ea7ab3dbf04f24f24b, after an async
>     command is executed, the state of the server is set back to state.initial,
>     so the cache is always reloaded.
>     This patch adds a sanity check on the validity of the cache and then only
>     reload if something has changed.
>     depends_cache variable is needed in this case, so the del statement needs
>     to be removed.
>
>     [YOCTO #5603]
>     Signed-off-by: Cristiana Voicu <cristiana.voicu@intel.com <mailto:cristiana.voicu@intel.com>>
>     ---
>      bitbake/lib/bb/cache.py  |    2 --
>      bitbake/lib/bb/cooker.py |   32 ++++++++++++++++++++++----------
>      2 files changed, 22 insertions(+), 12 deletions(-)
>
>     diff --git a/bitbake/lib/bb/cache.py b/bitbake/lib/bb/cache.py
>     index 318781b..da9ad5d 100644
>     --- a/bitbake/lib/bb/cache.py
>     +++ b/bitbake/lib/bb/cache.py
>     @@ -612,8 +612,6 @@ class Cache(object):
>                          cache_class_name = cache_class.__name__
>                          file_dict[cache_class_name].close()
>
>     -        del self.depends_cache
>     -
>          @staticmethod
>          def mtime(cachefile):
>              return bb.parse.cached_mtime_noerror(cachefile)
>     diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py
>     index 07202e3..c90bb44 100644
>     --- a/bitbake/lib/bb/cooker.py
>     +++ b/bitbake/lib/bb/cooker.py
>     @@ -1265,21 +1265,33 @@ class BBCooker:
>                  raise bb.BBHandledException()
>
>              if self.state != state.parsing:
>     -            self.parseConfiguration ()
>     +            check_cache = False
>     +            if self.parser:
>     +                self.parser.bb_cache.checked = set()
>     +                for filename in self.parser.filelist:
>     +                    appends = self.collection.get_file_appends(filename)
>     +                    if not self.parser.bb_cache.cacheValid(filename, appends):
>     +                        check_cache = True
>     +                        break
>     +            else:
>     +                check_cache = True
>
>     -            ignore = self.data.getVar("ASSUME_PROVIDED", True) or ""
>     -            self.recipecache.ignored_dependencies = set(ignore.split())
>     +            if check_cache:
>     +                self.parseConfiguration ()
>     +
>     +                ignore = self.data.getVar("ASSUME_PROVIDED", True) or ""
>     +                self.recipecache.ignored_dependencies = set(ignore.split())
>
>     -            for dep in self.configuration.extra_assume_provided:
>     -                self.recipecache.ignored_dependencies.add(dep)
>     +                for dep in self.configuration.extra_assume_provided:
>     +                    self.recipecache.ignored_dependencies.add(dep)
>
>     -            self.collection = CookerCollectFiles(self.recipecache.bbfile_config_priorities)
>     -            (filelist, masked) = self.collection.collect_bbfiles(self.data, self.event_data)
>     +                self.collection = CookerCollectFiles(self.recipecache.bbfile_config_priorities)
>     +                (filelist, masked) = self.collection.collect_bbfiles(self.data, self.event_data)
>
>     -            self.data.renameVar("__depends", "__base_depends")
>     +                self.data.renameVar("__depends", "__base_depends")
>
>     -            self.parser = CookerParser(self, filelist, masked)
>     -            self.state = state.parsing
>     +                self.parser = CookerParser(self, filelist, masked)
>     +                self.state = state.parsing
>
>              if not self.parser.parse_next():
>                  collectlog.debug(1, "parsing complete")
>     --
>     1.7.9.5
>
>     --
>     _______________________________________________
>     bitbake-devel mailing list
>     bitbake-devel@lists.openembedded.org <mailto:bitbake-devel@lists.openembedded.org>
>     http://lists.openembedded.org/mailman/listinfo/bitbake-devel
>
>
>
>


[-- Attachment #2: Type: text/html, Size: 10268 bytes --]

Re: [PATCH] cooker: bitbake memres server should not always reload the cache

[Paul Eggleton]

On Friday 28 March 2014 07:56:43 Jason Wessel wrote:
> On 03/28/2014 06:43 AM, Alex Damian wrote:
> > Please hold merging this change.
> > 
> > I think we need to discuss a bit on this. My expectation is that the
> > Bitbake server will reinit its state between builds, re-parsing the
> > configuration files and reload the cache. I am not sure why we would
> > consider maintaining the hot cache copy in memory between builds.
>
> It is about increasing the interactivity of the system.  There are certainly
> times you need to soft reset, but perhaps that is what we need to expose. 
> If I want to run a dev shell and have done a build, this is something I'd
> like to be instantaneous, or if I am working on the package and want to
> re-run the compile and package steps, there really is no need to parse.
> 
> There is a trade off for the individual package work vs the work of
> rebuilding the entire image.  In the case of the entire image you most
> certainly want the soft reset behavior.

To my mind it depends on the likelihood of something having changed and 
whether that change might affect what you would be doing with the action that 
follows. For example. if you're trying to fix the execution of a particular 
task, you would expect to see any change in the recipe / configuration to take 
effect when re-running the task.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH] cooker: bitbake memres server should not always reload the cache

[Jason Wessel]

On 03/28/2014 09:22 AM, Paul Eggleton wrote:
> On Friday 28 March 2014 07:56:43 Jason Wessel wrote:
>> On 03/28/2014 06:43 AM, Alex Damian wrote:
>>> Please hold merging this change.
>>>
>>> I think we need to discuss a bit on this. My expectation is that the
>>> Bitbake server will reinit its state between builds, re-parsing the
>>> configuration files and reload the cache. I am not sure why we would
>>> consider maintaining the hot cache copy in memory between builds.
>> It is about increasing the interactivity of the system.  There are certainly
>> times you need to soft reset, but perhaps that is what we need to expose. 
>> If I want to run a dev shell and have done a build, this is something I'd
>> like to be instantaneous, or if I am working on the package and want to
>> re-run the compile and package steps, there really is no need to parse.
>>
>> There is a trade off for the individual package work vs the work of
>> rebuilding the entire image.  In the case of the entire image you most
>> certainly want the soft reset behavior.
> To my mind it depends on the likelihood of something having changed and 
> whether that change might affect what you would be doing with the action that 
> follows. For example. if you're trying to fix the execution of a particular 
> task, you would expect to see any change in the recipe / configuration to take 
> effect when re-running the task.

I think there should be a lighter weight way to check then.  I have to believe we could actually check all the time stamps pretty quickly if we have them stored away as a part of the parse for all the files that were touched.  I admit I have not looked a whole lot at what determines when to re-parse / soft reset, but I have to believe we can come up with a light enough way in mem-res mode so as not to pay a reparse toll like we are doing presently.  Perhaps this is what the cache check already does today?

Perhaps you know what all is going on when the bitbake cache is doing its load (which takes ~ 2 seconds or so) for the set of recipes I was interested in.  My goal here is merely to improve the interactivity, assuming it is possible.

Cheers,
Jason.