[PATCH 0/6] Bug fixes and optimisations for hob

[PATCH 0/6] Bug fixes and optimisations for hob

[Joshua Lock]

This series includes two bug fixes and a brace of patches to speed up some
operations on the gtk.ListStore that became apparent as I was reviewing the
code for the bug fixes.

Regards,
Joshua

The following changes since commit 6555a77c199f41bf35460138764e03e30c56d29f:

  data_smart.py: make use of expand cache in getVar() (2011-08-29 14:13:25 +0100)

are available in the git repository at:
  git://github.com/incandescant/bitbake hob
  https://github.com/incandescant/bitbake/tree/hob

Joshua Lock (6):
  ui/crumbs/tasklistmodel: don't add same item to binb column more than
    once
  ui/crumbs/tasklistmodel: prevent packages depending on each other
  ui/crumbs/tasklistmodel: optimise find_path_for_item()
  ui/crumbs/tasklistmodel: remove unnecessary check
  ui/crumbs/tasklistmodel: loop optimisation in include_item()
  ui/crumbs/tasklistmodel: don't add empty entries to COL_BINB

 lib/bb/ui/crumbs/tasklistmodel.py |   68 +++++++++++++++++--------------------
 1 files changed, 31 insertions(+), 37 deletions(-)

-- 
1.7.6

[PATCH 1/6] ui/crumbs/tasklistmodel: don't add same item to binb column more than once

[Joshua Lock]

In the same vein as a similar, earlier, patch where I missed the second
loop which modifies the binb column.

Fixes [YOCTO #1420]

Signed-off-by: Joshua Lock <josh@linux.intel.com>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/bb/ui/crumbs/tasklistmodel.py b/lib/bb/ui/crumbs/tasklistmodel.py
index cf9fc59..edb4d96 100644
--- a/lib/bb/ui/crumbs/tasklistmodel.py
+++ b/lib/bb/ui/crumbs/tasklistmodel.py
@@ -462,8 +462,9 @@ class TaskListModel(gtk.ListStore):
                     continue
                 if dep_included:
                     bin = self[path][self.COL_BINB].split(', ')
-                    bin.append(name)
-                    self[path][self.COL_BINB] = ', '.join(bin).lstrip(', ')
+                    if not name in bin:
+                        bin.append(name)
+                        self[path][self.COL_BINB] = ', '.join(bin).lstrip(', ')
                 else:
                     self.include_item(path, binb=name, image_contents=image_contents)
 
-- 
1.7.6

[PATCH 2/6] ui/crumbs/tasklistmodel: prevent packages depending on each other

[Joshua Lock]

Don't add y to x's COL_BINB if x is in y's COL_BINB - prevent circular
dependencies.

Further this patch improves the variable naming to make this code easier to
follow.

Fixes [YOCTO #1423]

Signed-off-by: Joshua Lock <josh@linux.intel.com>
---
 lib/bb/ui/crumbs/tasklistmodel.py |   42 +++++++++++++++++++-----------------
 1 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/lib/bb/ui/crumbs/tasklistmodel.py b/lib/bb/ui/crumbs/tasklistmodel.py
index edb4d96..8413873 100644
--- a/lib/bb/ui/crumbs/tasklistmodel.py
+++ b/lib/bb/ui/crumbs/tasklistmodel.py
@@ -434,39 +434,41 @@ class TaskListModel(gtk.ListStore):
     Add this item, and any of its dependencies, to the image contents
     """
     def include_item(self, item_path, binb="", image_contents=False):
-        name = self[item_path][self.COL_NAME]
-        deps = self[item_path][self.COL_DEPS]
-        cur_inc = self[item_path][self.COL_INC]
-        if not cur_inc:
+        item_name = self[item_path][self.COL_NAME]
+        item_deps = self[item_path][self.COL_DEPS]
+        item_inc = self[item_path][self.COL_INC]
+        if not item_inc:
             self[item_path][self.COL_INC] = True
 
-        bin = self[item_path][self.COL_BINB].split(', ')
-        if not binb in bin:
-            bin.append(binb)
-            self[item_path][self.COL_BINB] = ', '.join(bin).lstrip(', ')
+        item_bin = self[item_path][self.COL_BINB].split(', ')
+        if not binb in item_bin:
+            item_bin.append(binb)
+            self[item_path][self.COL_BINB] = ', '.join(item_bin).lstrip(', ')
 
         # We want to do some magic with things which are brought in by the
         # base image so tag them as so
         if image_contents:
             self[item_path][self.COL_IMG] = True
             if self[item_path][self.COL_TYPE] == 'image':
-                self.selected_image = name
+                self.selected_image = item_name
 
-        if deps:
+        if item_deps:
             # add all of the deps and set their binb to this item
-            for dep in deps.split(" "):
+            for dep in item_deps.split(" "):
                 # If the contents model doesn't already contain dep, add it
                 dep_included = self.contents_includes_name(dep)
-                path = self.find_path_for_item(dep)
-                if not path:
+                dep_path = self.find_path_for_item(dep)
+                if not dep_path:
                     continue
-                if dep_included:
-                    bin = self[path][self.COL_BINB].split(', ')
-                    if not name in bin:
-                        bin.append(name)
-                        self[path][self.COL_BINB] = ', '.join(bin).lstrip(', ')
-                else:
-                    self.include_item(path, binb=name, image_contents=image_contents)
+                if dep_included and not dep in item_bin:
+                    # don't set the COL_BINB to this item if the target is an
+                    # item in our own COL_BINB
+                    dep_bin = self[dep_path][self.COL_BINB].split(', ')
+                    if not item_name in dep_bin:
+                        dep_bin.append(item_name)
+                        self[dep_path][self.COL_BINB] = ', '.join(dep_bin).lstrip(', ')
+                elif not dep_included:
+                    self.include_item(dep_path, binb=item_name, image_contents=image_contents)
 
     """
     Find the model path for the item_name
-- 
1.7.6

[PATCH 3/6] ui/crumbs/tasklistmodel: optimise find_path_for_item()

[Joshua Lock]

Rather than calling get_path() for each iterated value use the get_value()
method to lookup the COL_NAME value and only call get_path() for a match.

This should save some time by potentially removing N-1 calls to get_path()
from the loop.

Signed-off-by: Joshua Lock <josh@linux.intel.com>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/bb/ui/crumbs/tasklistmodel.py b/lib/bb/ui/crumbs/tasklistmodel.py
index 8413873..14a611f 100644
--- a/lib/bb/ui/crumbs/tasklistmodel.py
+++ b/lib/bb/ui/crumbs/tasklistmodel.py
@@ -481,11 +481,9 @@ class TaskListModel(gtk.ListStore):
             return None
 
         it = self.get_iter_first()
-        path = None
         while it:
-            path = self.get_path(it)
-            if (self[path][self.COL_NAME] == item_name):
-                return path
+            if (self.get_value(it, self.COL_NAME) == item_name):
+                return self.get_path(it)
             else:
                 it = self.iter_next(it)
         return None
-- 
1.7.6

[PATCH 4/6] ui/crumbs/tasklistmodel: remove unnecessary check

[Joshua Lock]

Cheaper to set COL_INC to True regardless of whether it's already set.

Signed-off-by: Joshua Lock <josh@linux.intel.com>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/bb/ui/crumbs/tasklistmodel.py b/lib/bb/ui/crumbs/tasklistmodel.py
index 14a611f..790631f 100644
--- a/lib/bb/ui/crumbs/tasklistmodel.py
+++ b/lib/bb/ui/crumbs/tasklistmodel.py
@@ -436,9 +436,8 @@ class TaskListModel(gtk.ListStore):
     def include_item(self, item_path, binb="", image_contents=False):
         item_name = self[item_path][self.COL_NAME]
         item_deps = self[item_path][self.COL_DEPS]
-        item_inc = self[item_path][self.COL_INC]
-        if not item_inc:
-            self[item_path][self.COL_INC] = True
+
+        self[item_path][self.COL_INC] = True
 
         item_bin = self[item_path][self.COL_BINB].split(', ')
         if not binb in item_bin:
-- 
1.7.6

[PATCH 5/6] ui/crumbs/tasklistmodel: loop optimisation in include_item()

[Joshua Lock]

Rather than trying to iterate the model to find whether the item is already
included and then iterate the model again to find the items path attempt to
find the path first and if the path is found test whether the COL_INC of
the row is set.

Signed-off-by: Joshua Lock <josh@linux.intel.com>
---
 lib/bb/ui/crumbs/tasklistmodel.py |   20 +++++++-------------
 1 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/lib/bb/ui/crumbs/tasklistmodel.py b/lib/bb/ui/crumbs/tasklistmodel.py
index 790631f..518232d 100644
--- a/lib/bb/ui/crumbs/tasklistmodel.py
+++ b/lib/bb/ui/crumbs/tasklistmodel.py
@@ -417,18 +417,10 @@ class TaskListModel(gtk.ListStore):
                 it = self.contents.iter_next(it)
 
     """
-    Check the self.contents gtk.TreeModel for an item
-    where COL_NAME matches item_name
-    Returns True if a match is found, False otherwise
+    Check whether the item at item_path is included or not
     """
-    def contents_includes_name(self, item_name):
-        it = self.contents.get_iter_first()
-        while it:
-            path = self.contents.get_path(it)
-            if self.contents[path][self.COL_NAME] == item_name:
-                return True
-            it = self.contents.iter_next(it)
-        return False
+    def contents_includes_path(self, item_path):
+        return self[item_path][self.COL_INC]
 
     """
     Add this item, and any of its dependencies, to the image contents
@@ -452,13 +444,15 @@ class TaskListModel(gtk.ListStore):
                 self.selected_image = item_name
 
         if item_deps:
-            # add all of the deps and set their binb to this item
+            # Ensure all of the items deps are included and, where appropriate,
+            # add this item to their COL_BINB
             for dep in item_deps.split(" "):
                 # If the contents model doesn't already contain dep, add it
-                dep_included = self.contents_includes_name(dep)
                 dep_path = self.find_path_for_item(dep)
                 if not dep_path:
                     continue
+                dep_included = self.contents_includes_path(dep_path)
+
                 if dep_included and not dep in item_bin:
                     # don't set the COL_BINB to this item if the target is an
                     # item in our own COL_BINB
-- 
1.7.6

[PATCH 6/6] ui/crumbs/tasklistmodel: don't add empty entries to COL_BINB

[Joshua Lock]

Signed-off-by: Joshua Lock <josh@linux.intel.com>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/bb/ui/crumbs/tasklistmodel.py b/lib/bb/ui/crumbs/tasklistmodel.py
index 518232d..90a7e54 100644
--- a/lib/bb/ui/crumbs/tasklistmodel.py
+++ b/lib/bb/ui/crumbs/tasklistmodel.py
@@ -432,7 +432,7 @@ class TaskListModel(gtk.ListStore):
         self[item_path][self.COL_INC] = True
 
         item_bin = self[item_path][self.COL_BINB].split(', ')
-        if not binb in item_bin:
+        if binb and not binb in item_bin:
             item_bin.append(binb)
             self[item_path][self.COL_BINB] = ', '.join(item_bin).lstrip(', ')
 
-- 
1.7.6

Re: [PATCH 0/6] Bug fixes and optimisations for hob

[Richard Purdie]

On Tue, 2011-08-30 at 11:08 -0700, Joshua Lock wrote:
> This series includes two bug fixes and a brace of patches to speed up some
> operations on the gtk.ListStore that became apparent as I was reviewing the
> code for the bug fixes.
> 
> Regards,
> Joshua
> 
> The following changes since commit 6555a77c199f41bf35460138764e03e30c56d29f:
> 
>   data_smart.py: make use of expand cache in getVar() (2011-08-29 14:13:25 +0100)
> 
> are available in the git repository at:
>   git://github.com/incandescant/bitbake hob
>   https://github.com/incandescant/bitbake/tree/hob
> 
> Joshua Lock (6):
>   ui/crumbs/tasklistmodel: don't add same item to binb column more than
>     once
>   ui/crumbs/tasklistmodel: prevent packages depending on each other
>   ui/crumbs/tasklistmodel: optimise find_path_for_item()
>   ui/crumbs/tasklistmodel: remove unnecessary check
>   ui/crumbs/tasklistmodel: loop optimisation in include_item()
>   ui/crumbs/tasklistmodel: don't add empty entries to COL_BINB

Merged to master, thanks.

Richard