[dizzy 0/7] inotify patchset back-port

[dizzy 0/7] inotify patchset back-port

[Alex DAMIAN]

From: Alexandru DAMIAN <alexandru.damian@intel.com>


Hello,

This is the backport of the inotify patchset that allows the bitbake
server to read files (configuration and recipes) modified while it is running.

This backport allows proper Toaster experience for Yocto Project 1.7 Dizzy users.

I've tested this patch both functionally and performance-wise, and I could not
find any obvious bugs.

Thank you,
Alex

Alexandru DAMIAN (1):
  cooker: read file watches on server idle

Richard Purdie (6):
  bitbake: Add pyinotify to lib/
  cooker/cache/parse: Implement pyinofity based reconfigure
  cooker: Fix pyinotify handling of ENOENT issues
  cooker: Further optimise pyinotify
  cooker: Improve pyinotify performance
  cooker/server: Fix up 100% CPU usage at idle

 lib/bb/cache.py          |    5 +-
 lib/bb/cooker.py         |   79 +-
 lib/bb/parse/__init__.py |    5 +
 lib/bb/server/process.py |    3 +
 lib/bb/server/xmlrpc.py  |    3 +
 lib/pyinotify.py         | 2416 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 2508 insertions(+), 3 deletions(-)
 create mode 100644 lib/pyinotify.py

-- 
1.9.1

[dizzy 2/7] cooker/cache/parse: Implement pyinofity based reconfigure

[Alex DAMIAN]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

Memory resident bitbake has one current flaw, changes in the base configuration
are not noticed by bitbake. The parsing cache is also refreshed on each invocation
of bitbake (although the mtime cache is not cleared so its pointless).

This change adds in pyinotify support and adds two different watchers, one
for the base configuration and one for the parsed recipes.

Changes in the latter will trigger a reparse (and an update of the mtime cache).
The former will trigger a complete reload of the configuration.

Note that this code will also correctly handle creation of new configuration files
since the __depends and __base_depends variables already track these for cache
correctness purposes.

We could be a little more clever about parsing cache invalidation, right now we just
invalidate the whole thing and recheck. For now, its better than what we have and doesn't
seem to perform that badly though.

For education and QA purposes I can document a workflow that illustrates this:

$ source oe-init-build-env-memres
$ time bitbake bash
[base configuration is loaded, recipes are parsed, bash builds]
$ time bitbake bash
[command returns quickly since all caches are valid]
$ touch ../meta/classes/gettext.bbclass
$ time bitbake bash
[reparse is triggered, time is longer than above]
$ echo 'FOO = "1"' >> conf/local.conf
$ time bitbake bash
[reparse is triggered, but with a base configuration reload too]

As far as changes go, I like this one a lot, it makes memory resident bitbake
truly usable and may be the tweak we need to make it the default.

The new pyinotify dependency is covered in the previous commit.

(Bitbake rev: 0557d03c170fba8d7efe82be1b9641d0eb229213)

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/cache.py          |  5 ++++-
 lib/bb/cooker.py         | 44 ++++++++++++++++++++++++++++++++++++++++++--
 lib/bb/parse/__init__.py |  5 +++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/lib/bb/cache.py b/lib/bb/cache.py
index 715da07..a1dde96 100644
--- a/lib/bb/cache.py
+++ b/lib/bb/cache.py
@@ -623,10 +623,13 @@ class Cache(object):
     def mtime(cachefile):
         return bb.parse.cached_mtime_noerror(cachefile)
 
-    def add_info(self, filename, info_array, cacheData, parsed=None):
+    def add_info(self, filename, info_array, cacheData, parsed=None, watcher=None):
         if isinstance(info_array[0], CoreRecipeInfo) and (not info_array[0].skipped):
             cacheData.add_from_recipeinfo(filename, info_array)
 
+            if watcher:
+                watcher(info_array[0].file_depends)
+
         if not self.has_cache:
             return
 
diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 8e6d91b..b5a5281 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -39,6 +39,7 @@ from bb import utils, data, parse, event, cache, providers, taskdata, runqueue
 import Queue
 import signal
 import prserv.serv
+import pyinotify
 
 logger      = logging.getLogger("BitBake")
 collectlog  = logging.getLogger("BitBake.Collection")
@@ -120,7 +121,18 @@ class BBCooker:
 
         self.configuration = configuration
 
+        self.configwatcher = pyinotify.WatchManager()
+        self.confignotifier = pyinotify.Notifier(self.configwatcher, self.config_notifications)
+        self.watchmask = pyinotify.IN_CLOSE_WRITE | pyinotify.IN_CREATE | pyinotify.IN_DELETE | \
+                         pyinotify.IN_DELETE_SELF | pyinotify.IN_MODIFY | pyinotify.IN_MOVE_SELF | \
+                         pyinotify.IN_MOVED_FROM | pyinotify.IN_MOVED_TO 
+        self.watcher = pyinotify.WatchManager()
+        self.notifier = pyinotify.Notifier(self.watcher, self.notifications)
+
+
         self.initConfigurationData()
+        self.baseconfig_valid = True
+        self.parsecache_valid = False
 
         # Take a lock so only one copy of bitbake can run against a given build
         # directory at a time
@@ -156,6 +168,18 @@ class BBCooker:
         # Let SIGHUP exit as SIGTERM
         signal.signal(signal.SIGHUP, self.sigterm_exception)
 
+    def config_notifications(self, event):
+        bb.parse.update_cache(event.path)
+        self.baseconfig_valid = False
+
+    def notifications(self, event):
+        bb.parse.update_cache(event.path)
+        self.parsecache_valid = False
+
+    def add_filewatch(self, deps):
+        for i in deps:
+            self.watcher.add_watch(i[0], self.watchmask, rec=True)
+
     def sigterm_exception(self, signum, stackframe):
         if signum == signal.SIGTERM:
             bb.warn("Cooker recieved SIGTERM, shutting down...")
@@ -1292,6 +1316,18 @@ class BBCooker:
             raise bb.BBHandledException()
 
         if self.state != state.parsing:
+            for n in [self.confignotifier, self.notifier]:
+                if n.check_events(timeout=0):
+                    # read notified events and enqeue them
+                    n.read_events()
+                n.process_events()
+            if not self.baseconfig_valid:
+                logger.debug(1, "Reloading base configuration data")
+                self.initConfigurationData()
+                self.baseconfig_valid = True
+                self.parsecache_valid = False
+
+        if self.state != state.parsing and not self.parsecache_valid:
             self.parseConfiguration ()
             if CookerFeatures.SEND_SANITYEVENTS in self.featureset:
                 bb.event.fire(bb.event.SanityCheck(False), self.data)
@@ -1306,9 +1342,13 @@ class BBCooker:
             (filelist, masked) = self.collection.collect_bbfiles(self.data, self.event_data)
 
             self.data.renameVar("__depends", "__base_depends")
+            for i in self.data.getVar("__base_depends"):
+                self.wdd = self.configwatcher.add_watch(i[0], self.watchmask, rec=True)
 
             self.parser = CookerParser(self, filelist, masked)
-            self.state = state.parsing
+            self.parsecache_valid = True
+
+        self.state = state.parsing
 
         if not self.parser.parse_next():
             collectlog.debug(1, "parsing complete")
@@ -1870,7 +1910,7 @@ class CookerParser(object):
                 self.skipped += 1
                 self.cooker.skiplist[virtualfn] = SkippedPackage(info_array[0])
             self.bb_cache.add_info(virtualfn, info_array, self.cooker.recipecache,
-                                        parsed=parsed)
+                                        parsed=parsed, watcher = self.cooker.add_filewatch)
         return True
 
     def reparse(self, filename):
diff --git a/lib/bb/parse/__init__.py b/lib/bb/parse/__init__.py
index 2303f15..25effc2 100644
--- a/lib/bb/parse/__init__.py
+++ b/lib/bb/parse/__init__.py
@@ -73,6 +73,11 @@ def update_mtime(f):
     __mtime_cache[f] = os.stat(f)[stat.ST_MTIME]
     return __mtime_cache[f]
 
+def update_cache(f):
+    if f in __mtime_cache:
+        logger.debug(1, "Updating mtime cache for %s" % f)
+        update_mtime(f)
+
 def mark_dependency(d, f):
     if f.startswith('./'):
         f = "%s/%s" % (os.getcwd(), f[2:])
-- 
1.9.1

[dizzy 3/7] cooker: Fix pyinotify handling of ENOENT issues

[Alex DAMIAN]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

We try and add watches for files that don't exist but if they did, would influence
the parser. The parent directory of these files may not exist, in which case we need
to watch any parent that does exist for changes. This change implements that fallback
handling.

(Bitbake rev: 979ddbe4b7340d7cf2f432f6b1eba1c58d55ff42)

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/cooker.py | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index b5a5281..614d47d 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -176,9 +176,23 @@ class BBCooker:
         bb.parse.update_cache(event.path)
         self.parsecache_valid = False
 
-    def add_filewatch(self, deps):
+    def add_filewatch(self, deps, watcher=None):
+        if not watcher:
+            watcher = self.watcher
         for i in deps:
-            self.watcher.add_watch(i[0], self.watchmask, rec=True)
+            f = i[0]
+            while True:
+                # We try and add watches for files that don't exist but if they did, would influence
+                # the parser. The parent directory of these files may not exist, in which case we need 
+                # to watch any parent that does exist for changes.
+                try:
+                    watcher.add_watch(f, self.watchmask, quiet=False)
+                    break
+                except pyinotify.WatchManagerError as e:
+                    if 'ENOENT' in str(e):
+                        f = os.path.dirname(f)
+                        continue
+                    raise
 
     def sigterm_exception(self, signum, stackframe):
         if signum == signal.SIGTERM:
@@ -1342,8 +1356,7 @@ class BBCooker:
             (filelist, masked) = self.collection.collect_bbfiles(self.data, self.event_data)
 
             self.data.renameVar("__depends", "__base_depends")
-            for i in self.data.getVar("__base_depends"):
-                self.wdd = self.configwatcher.add_watch(i[0], self.watchmask, rec=True)
+            self.add_filewatch(self.data.getVar("__base_depends"), self.configwatcher)
 
             self.parser = CookerParser(self, filelist, masked)
             self.parsecache_valid = True
-- 
1.9.1

[dizzy 4/7] cooker: Further optimise pyinotify

[Alex DAMIAN]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

We currently add crazy numbers of watches on files. The per user limit is 8192
by default and on a system handling multiple builds, this can be an issue.

We don't need to watch all files individually, we can watch the directory containing
the file instead. This gives better resource utilisation and better performance
further reverting some of the performance regression seen with the introduction
of pyinotify.

(Bitbake rev: a2d441237916a99405b800c1a3dc39f860100a8c)

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/cooker.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 614d47d..e965bcc 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -180,7 +180,10 @@ class BBCooker:
         if not watcher:
             watcher = self.watcher
         for i in deps:
-            f = i[0]
+            f = os.path.dirname(i[0])
+            if f in watcher.bbseen:
+                continue
+            watcher.bbseen.append(f)
             while True:
                 # We try and add watches for files that don't exist but if they did, would influence
                 # the parser. The parent directory of these files may not exist, in which case we need 
-- 
1.9.1

[dizzy 5/7] cooker: Improve pyinotify performance

[Alex DAMIAN]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

Benchmarks show that the introduction of pyinotify regressed
performance. This patch ensures we only call the add_watch() function
for new entries, not ones we've already processed which does improve
performance as measured by "time bitbake -p".

This doesn't completely remove the overhead but it does substantially
reduce it.

(Bitbake rev: 493361f35f6cc332d4ea359a2695622c2c91a9c2)

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/cooker.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index e965bcc..3909dc0 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -122,11 +122,13 @@ class BBCooker:
         self.configuration = configuration
 
         self.configwatcher = pyinotify.WatchManager()
+        self.configwatcher.bbseen = []
         self.confignotifier = pyinotify.Notifier(self.configwatcher, self.config_notifications)
         self.watchmask = pyinotify.IN_CLOSE_WRITE | pyinotify.IN_CREATE | pyinotify.IN_DELETE | \
                          pyinotify.IN_DELETE_SELF | pyinotify.IN_MODIFY | pyinotify.IN_MOVE_SELF | \
                          pyinotify.IN_MOVED_FROM | pyinotify.IN_MOVED_TO 
         self.watcher = pyinotify.WatchManager()
+        self.watcher.bbseen = []
         self.notifier = pyinotify.Notifier(self.watcher, self.notifications)
 
 
@@ -180,7 +182,7 @@ class BBCooker:
         if not watcher:
             watcher = self.watcher
         for i in deps:
-            f = os.path.dirname(i[0])
+            f = i[0]
             if f in watcher.bbseen:
                 continue
             watcher.bbseen.append(f)
@@ -194,6 +196,7 @@ class BBCooker:
                 except pyinotify.WatchManagerError as e:
                     if 'ENOENT' in str(e):
                         f = os.path.dirname(f)
+                        watcher.bbseen.append(f)
                         continue
                     raise
 
-- 
1.9.1

[dizzy 6/7] cooker: read file watches on server idle

[Alex DAMIAN]

From: Alexandru DAMIAN <alexandru.damian@intel.com>

The inotify facility monitoring changes to the config files
could be overwhelmed by massive changes to the watched files
while server is running.

This patch adds verification the notification watches to the
server idle functions, in addition to the cooker updateCache
command which executes only infrequently, thus preventing
overflowing the notification buffer.

[YOCTO #7316]

(Bitbake rev: 996e663fd5c254292f44eca46f5fdc95af897f98)

Signed-off-by: Alexandru DAMIAN <alexandru.damian@intel.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/cooker.py | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 3909dc0..2f2a852 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -133,6 +133,19 @@ class BBCooker:
 
 
         self.initConfigurationData()
+
+        self.inotify_modified_files = []
+
+        def _process_inotify_updates(server, notifier_list, abort):
+            for n in notifier_list:
+                if n.check_events(timeout=0):
+                    # read notified events and enqeue them
+                    n.read_events()
+                    n.process_events()
+            return True
+
+        self.configuration.server_register_idlecallback(_process_inotify_updates, [self.confignotifier, self.notifier])
+
         self.baseconfig_valid = True
         self.parsecache_valid = False
 
@@ -171,11 +184,13 @@ class BBCooker:
         signal.signal(signal.SIGHUP, self.sigterm_exception)
 
     def config_notifications(self, event):
-        bb.parse.update_cache(event.path)
+        if not event.path in self.inotify_modified_files:
+            self.inotify_modified_files.append(event.path)
         self.baseconfig_valid = False
 
     def notifications(self, event):
-        bb.parse.update_cache(event.path)
+        if not event.path in self.inotify_modified_files:
+            self.inotify_modified_files.append(event.path)
         self.parsecache_valid = False
 
     def add_filewatch(self, deps, watcher=None):
@@ -1336,11 +1351,12 @@ class BBCooker:
             raise bb.BBHandledException()
 
         if self.state != state.parsing:
-            for n in [self.confignotifier, self.notifier]:
-                if n.check_events(timeout=0):
-                    # read notified events and enqeue them
-                    n.read_events()
-                n.process_events()
+
+            # reload files for which we got notifications
+            for p in self.inotify_modified_files:
+                bb.parse.update_cache(p)
+            self.inotify_modified_files = []
+
             if not self.baseconfig_valid:
                 logger.debug(1, "Reloading base configuration data")
                 self.initConfigurationData()
-- 
1.9.1

[dizzy 7/7] cooker/server: Fix up 100% CPU usage at idle

[Alex DAMIAN]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

The recent inotify changes are causing a 100% cpu usage issue in the
idle handlers. To avoid this, we update the idle functions to optionally
report a float value which is the delay before the function needs to be
called again. 1 second is fine for the inotify handler, in reality its
more like 0.1s due to the default idle function sleep.

This reverts performance regressions of 1.5 minutes on a kernel build
and ~5-6 minutes on a image from scratch.

(Bitbake rev: 0e0ba408c2dce14a0fabd3fdf61d8465a031495b)

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/cooker.py         | 2 +-
 lib/bb/server/process.py | 3 +++
 lib/bb/server/xmlrpc.py  | 3 +++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 2f2a852..aeb3f71 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -142,7 +142,7 @@ class BBCooker:
                     # read notified events and enqeue them
                     n.read_events()
                     n.process_events()
-            return True
+            return 1.0
 
         self.configuration.server_register_idlecallback(_process_inotify_updates, [self.confignotifier, self.notifier])
 
diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index d362f8d..302ee5f 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -135,6 +135,9 @@ class ProcessServer(Process, BaseImplServer):
                     nextsleep = None
                 elif retval is True:
                     nextsleep = None
+                elif isinstance(retval, float):
+                    if (retval < nextsleep):
+                        nextsleep = retval
                 elif nextsleep is None:
                     continue
                 else:
diff --git a/lib/bb/server/xmlrpc.py b/lib/bb/server/xmlrpc.py
index 4205a4c..10d4b5c 100644
--- a/lib/bb/server/xmlrpc.py
+++ b/lib/bb/server/xmlrpc.py
@@ -241,6 +241,9 @@ class XMLRPCServer(SimpleXMLRPCServer, BaseImplServer):
                         del self._idlefuns[function]
                     elif retval is True:
                         nextsleep = 0
+                    elif isinstance(retval, float):
+                        if (retval < nextsleep):
+                            nextsleep = retval
                     else:
                         fds = fds + retval
                 except SystemExit:
-- 
1.9.1

Re: [dizzy 0/7] inotify patchset back-port

[akuster808]

Alex,

thanks, I will be stage thiese for dizzy-next

- armin

On 04/10/2015 07:57 AM, Alex DAMIAN wrote:
> From: Alexandru DAMIAN <alexandru.damian@intel.com>
>
>
> Hello,
>
> This is the backport of the inotify patchset that allows the bitbake
> server to read files (configuration and recipes) modified while it is running.
>
> This backport allows proper Toaster experience for Yocto Project 1.7 Dizzy users.
>
> I've tested this patch both functionally and performance-wise, and I could not
> find any obvious bugs.
>
> Thank you,
> Alex
>
> Alexandru DAMIAN (1):
>    cooker: read file watches on server idle
>
> Richard Purdie (6):
>    bitbake: Add pyinotify to lib/
>    cooker/cache/parse: Implement pyinofity based reconfigure
>    cooker: Fix pyinotify handling of ENOENT issues
>    cooker: Further optimise pyinotify
>    cooker: Improve pyinotify performance
>    cooker/server: Fix up 100% CPU usage at idle
>
>   lib/bb/cache.py          |    5 +-
>   lib/bb/cooker.py         |   79 +-
>   lib/bb/parse/__init__.py |    5 +
>   lib/bb/server/process.py |    3 +
>   lib/bb/server/xmlrpc.py  |    3 +
>   lib/pyinotify.py         | 2416 ++++++++++++++++++++++++++++++++++++++++++++++
>   6 files changed, 2508 insertions(+), 3 deletions(-)
>   create mode 100644 lib/pyinotify.py
>

Re: [dizzy 0/7] inotify patchset back-port

[akuster808]

Is there a [dizzy 1/7] ?

- armin
On 04/10/2015 07:57 AM, Alex DAMIAN wrote:
> From: Alexandru DAMIAN <alexandru.damian@intel.com>
>
>
> Hello,
>
> This is the backport of the inotify patchset that allows the bitbake
> server to read files (configuration and recipes) modified while it is running.
>
> This backport allows proper Toaster experience for Yocto Project 1.7 Dizzy users.
>
> I've tested this patch both functionally and performance-wise, and I could not
> find any obvious bugs.
>
> Thank you,
> Alex
>
> Alexandru DAMIAN (1):
>    cooker: read file watches on server idle
>
> Richard Purdie (6):
>    bitbake: Add pyinotify to lib/
>    cooker/cache/parse: Implement pyinofity based reconfigure
>    cooker: Fix pyinotify handling of ENOENT issues
>    cooker: Further optimise pyinotify
>    cooker: Improve pyinotify performance
>    cooker/server: Fix up 100% CPU usage at idle
>
>   lib/bb/cache.py          |    5 +-
>   lib/bb/cooker.py         |   79 +-
>   lib/bb/parse/__init__.py |    5 +
>   lib/bb/server/process.py |    3 +
>   lib/bb/server/xmlrpc.py  |    3 +
>   lib/pyinotify.py         | 2416 ++++++++++++++++++++++++++++++++++++++++++++++
>   6 files changed, 2508 insertions(+), 3 deletions(-)
>   create mode 100644 lib/pyinotify.py
>

Re: [dizzy 0/7] inotify patchset back-port

[akuster808]

I found your contrib branch and I will pull the seven changes from there.
- armin

On 04/10/2015 07:57 AM, Alex DAMIAN wrote:
> cooker: read file watches on server idle

Re: [dizzy 0/7] inotify patchset back-port

[Damian, Alexandru]

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

Hi,

Thanks for pulling this.

Alex

On Sat, Apr 11, 2015 at 3:12 AM, akuster808 <akuster808@gmail.com> wrote:

> I found your contrib branch and I will pull the seven changes from there.
> - armin
>
> On 04/10/2015 07:57 AM, Alex DAMIAN wrote:
>
>> cooker: read file watches on server idle
>>
> --
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/bitbake-devel
>



-- 
Alex Damian
Yocto Project
SSG / OTC

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