Skip to contentSkip to conversation

Bug fixes and optimisations for hob

PATCH Applied Joshua Lock 2 participants 8 messages Aug 30, 2011 Arm STM32

lore
Share & tools
lore

Joshua Lock

author
#0 lore raw

cover Bug fixes and optimisations for hob

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

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

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 <[email protected]>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
lib/bb/ui/crumbs/tasklistmodel.py +3 -2
index cf9fc59..edb4d96 100644
@@ -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)
Signature · 1 line
1.7.6
author
#2 lore raw

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

In reply to 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 <[email protected]>
---
 lib/bb/ui/crumbs/tasklistmodel.py |   42 +++++++++++++++++++-----------------
 1 files changed, 22 insertions(+), 20 deletions(-)
lib/bb/ui/crumbs/tasklistmodel.py +22 -20
index edb4d96..8413873 100644
@@ -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
Signature · 1 line
1.7.6
author
#3 lore raw

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

In reply to 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 <[email protected]>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
lib/bb/ui/crumbs/tasklistmodel.py +2 -4
index 8413873..14a611f 100644
@@ -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
Signature · 1 line
1.7.6
author
#4 lore raw

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

In reply to Joshua Lock

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

Signed-off-by Joshua Lock <[email protected]>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
lib/bb/ui/crumbs/tasklistmodel.py +2 -3
index 14a611f..790631f 100644
@@ -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:
Signature · 1 line
1.7.6
author
#5 lore raw

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

In reply to 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 <[email protected]>
---
 lib/bb/ui/crumbs/tasklistmodel.py |   20 +++++++-------------
 1 files changed, 7 insertions(+), 13 deletions(-)
lib/bb/ui/crumbs/tasklistmodel.py +7 -13
index 790631f..518232d 100644
@@ -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
Signature · 1 line
1.7.6
author
#6 lore raw

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

In reply to Joshua Lock
Signed-off-by Joshua Lock <[email protected]>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
lib/bb/ui/crumbs/tasklistmodel.py +1 -1
index 518232d..90a7e54 100644
@@ -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(', ')
Signature · 1 line
1.7.6
Applied
#7 lore raw
In reply to Joshua Lock
Quoted 15 lines · 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

Quoted 8 lines

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