All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] Improvements to scripts/buildstats-diff
@ 2016-09-29 14:27 Markus Lehtonen
  2016-09-29 14:27 ` [PATCH 01/11] scripts/buildstats-diff: check that the given directory exists Markus Lehtonen
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:27 UTC (permalink / raw)
  To: openembedded-core

Improve the buildstats-diff in different ways:
- slightly improved error handling and output
- make it possible to compare other attributes of the buiildstats than just
  cputime (block io statistics and wall clock time added)
- support averaging over multiple buildstats


The following changes since commit cdaafc3729700778d95afc2413553d7b41c1317b:

  oeqa/sstatetests: Ensure we cover deb packaging backend for sstate test (2016-09-28 10:15:55 +0100)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib marquiz/buildstats-diff
  http://git.openembedded.org/openembedded-core-contrib/log/?h=marquiz/buildstats-diff


Markus Lehtonen (11):
  scripts/buildstats-diff: check that the given directory exists
  scripts/buildstats-diff: rename --min-time and --min-timediff args
  scripts/buildstats-diff: implement BSTask class
  scripts/buildstats-diff: do not hardcode field widths in output
  scripts/buildstats-diff: introduce --diff-attr
  scripts/buildstats-diff: add read_bytes and write_bytes to --diff-attr
  scripts/buildstats-diff: add read_ops and write_ops to --diff-attr
  scripts/buildstats-diff: add walltime to --diff-attr
  scripts/buildstats-diff: use exception for internal error handling
  scripts/buildstats-diff: make logger msg format a bit more readable
  scripts/buildstats-diff: implement --multi option

 scripts/buildstats-diff | 397 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 275 insertions(+), 122 deletions(-)

-- 
2.6.6



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 01/11] scripts/buildstats-diff: check that the given directory exists
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
@ 2016-09-29 14:27 ` Markus Lehtonen
  2016-09-29 14:27 ` [PATCH 02/11] scripts/buildstats-diff: rename --min-time and --min-timediff args Markus Lehtonen
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:27 UTC (permalink / raw)
  To: openembedded-core

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index 8ee2aaf..d0cd766 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -115,11 +115,14 @@ def read_buildstats_dir(bs_dir):
 
     if os.path.isfile(os.path.join(bs_dir, 'build_stats')):
         top_dir = bs_dir
-    else:
+    elif os.path.exists(bs_dir):
         subdirs = sorted(glob.glob(bs_dir + '/*'))
         if len(subdirs) > 1:
             log.warning("Multiple buildstats found, using the first one")
         top_dir = subdirs[0]
+    else:
+        log.error("No such directory: %s", bs_dir)
+        sys.exit(1)
     log.debug("Reading buildstats directory %s", top_dir)
     subdirs = os.listdir(top_dir)
 
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 02/11] scripts/buildstats-diff: rename --min-time and --min-timediff args
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
  2016-09-29 14:27 ` [PATCH 01/11] scripts/buildstats-diff: check that the given directory exists Markus Lehtonen
@ 2016-09-29 14:27 ` Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 03/11] scripts/buildstats-diff: implement BSTask class Markus Lehtonen
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:27 UTC (permalink / raw)
  To: openembedded-core

Rename these arguments to --min-val and --min-absdiff in preparation for
supporting other "quantities" than just cputime.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index d0cd766..7728b6d 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -260,7 +260,7 @@ def task_time(task):
     return cputime
 
 
-def print_task_diff(bs1, bs2, min_cputime=0, min_timediff=0, sort_by=('absdiff',)):
+def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
     """Diff task execution times"""
     tasks_diff = []
     pkg_maxlen = 0
@@ -296,10 +296,10 @@ def print_task_diff(bs1, bs2, min_cputime=0, min_timediff=0, sort_by=('absdiff',
 
             tasks_diff.append(TaskDiff(pkg, pkg_op, task, task_op, t1, t2, t2-t1, reldiff))
 
-    if min_cputime:
-        print("Ignoring tasks shorter than {}s".format(min_cputime))
-    if min_timediff:
-        print("Ignoring time differences shorter than {}s".format(min_timediff))
+    if min_val:
+        print("Ignoring tasks shorter than {}s".format(min_val))
+    if min_absdiff:
+        print("Ignoring time differences shorter than {}s".format(min_absdiff))
 
     print()
     print("  {:{pkg_maxlen}}   {:{task_maxlen}} {:>8} {:>10} {:>10}    {}".format(
@@ -317,8 +317,8 @@ def print_task_diff(bs1, bs2, min_cputime=0, min_timediff=0, sort_by=('absdiff',
 
     for diff in tasks_diff:
         cputime = max(diff.cputime1, diff.cputime2)
-        if cputime > min_cputime:
-            if abs(diff.absdiff) > min_timediff:
+        if cputime > min_val:
+            if abs(diff.absdiff) > min_absdiff:
                 task_prefix = diff.task_op if diff.pkg_op == '  ' else '  '
                 print("{}{:{pkg_maxlen}} {}{:{task_maxlen}} {:+7.1f}s {:+9.1f}% {:9.1f}s -> {:.1f}s".format(
                         diff.pkg_op, diff.pkg, task_prefix, diff.task, diff.absdiff, diff.reldiff, diff.cputime1, diff.cputime2,
@@ -369,11 +369,11 @@ Script for comparing buildstats of two separate builds."""
                         help="Verbose logging")
     parser.add_argument('--ver-diff', action='store_true',
                         help="Show package version differences and exit")
-    parser.add_argument('--min-time', default=3.0, type=float,
-                        help="Filter out tasks shorter than MIN_TIME seconds")
-    parser.add_argument('--min-timediff', default=1.0, type=float,
+    parser.add_argument('--min-val', default=3.0, type=float,
+                        help="Filter out tasks shorter than MIN_VAL seconds")
+    parser.add_argument('--min-absdiff', default=1.0, type=float,
                         help="Filter out tasks whose difference in cputime is "
-                             "less that  MIN_TIMEDIFF seconds")
+                             "less that  MIN_ABSDIFF seconds")
     parser.add_argument('--sort-by', default='absdiff',
                         help="Comma-separated list of field sort order. "
                              "Prepend the field name with '-' for reversed sort. "
@@ -405,7 +405,7 @@ def main(argv=None):
     if args.ver_diff:
         print_ver_diff(bs1, bs2)
     else:
-        print_task_diff(bs1, bs2, args.min_time, args.min_timediff, sort_by)
+        print_task_diff(bs1, bs2, args.min_val, args.min_absdiff, sort_by)
         print_timediff_summary(bs1, bs2)
 
     return 0
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 03/11] scripts/buildstats-diff: implement BSTask class
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
  2016-09-29 14:27 ` [PATCH 01/11] scripts/buildstats-diff: check that the given directory exists Markus Lehtonen
  2016-09-29 14:27 ` [PATCH 02/11] scripts/buildstats-diff: rename --min-time and --min-timediff args Markus Lehtonen
@ 2016-09-29 14:28 ` Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 04/11] scripts/buildstats-diff: do not hardcode field widths in output Markus Lehtonen
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:28 UTC (permalink / raw)
  To: openembedded-core

New class representing buildstats data of a single task.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 51 +++++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index 7728b6d..22c44ff 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -62,11 +62,26 @@ def to_datetime_obj(obj):
         return datetime.utcfromtimestamp(obj).replace(tzinfo=TIMEZONES['UTC'])
 
 
+class BSTask(dict):
+    def __init__(self, *args, **kwargs):
+        self['start_time'] = None
+        self['elapsed_time'] = None
+        self['status'] = None
+        self['iostat'] = {}
+        self['rusage'] = {}
+        self['child_rusage'] = {}
+        super(BSTask, self).__init__(*args, **kwargs)
+
+    @property
+    def cputime(self):
+        """Sum of user and system time taken by the task"""
+        return self['rusage']['ru_stime'] + self['rusage']['ru_utime'] + \
+               self['child_rusage']['ru_stime'] + self['child_rusage']['ru_utime']
+
+
 def read_buildstats_file(buildstat_file):
     """Convert buildstat text file into dict/json"""
-    bs_json = {'iostat': {},
-               'rusage': {},
-               'child_rusage': {}}
+    bs_task = BSTask()
     log.debug("Reading task buildstats from %s", buildstat_file)
     with open(buildstat_file) as fobj:
         for line in fobj.readlines():
@@ -74,12 +89,12 @@ def read_buildstats_file(buildstat_file):
             val = val.strip()
             if key == 'Started':
                 start_time = to_datetime_obj(float(val))
-                bs_json['start_time'] = start_time
+                bs_task['start_time'] = start_time
             elif key == 'Ended':
                 end_time = to_datetime_obj(float(val))
             elif key.startswith('IO '):
                 split = key.split()
-                bs_json['iostat'][split[1]] = int(val)
+                bs_task['iostat'][split[1]] = int(val)
             elif key.find('rusage') >= 0:
                 split = key.split()
                 ru_key = split[-1]
@@ -89,11 +104,11 @@ def read_buildstats_file(buildstat_file):
                     val = int(val)
                 ru_type = 'rusage' if split[0] == 'rusage' else \
                                                   'child_rusage'
-                bs_json[ru_type][ru_key] = val
+                bs_task[ru_type][ru_key] = val
             elif key == 'Status':
-                bs_json['status'] = val
-    bs_json['elapsed_time'] = end_time - start_time
-    return bs_json
+                bs_task['status'] = val
+    bs_task['elapsed_time'] = end_time - start_time
+    return bs_task
 
 
 def read_buildstats_dir(bs_dir):
@@ -170,6 +185,10 @@ def read_buildstats_json(path):
             recipe_bs['nevr'] = "{}-{}-{}".format(recipe_bs['name'], recipe_bs['version'], recipe_bs['revision'])
         else:
             recipe_bs['nevr'] = "{}-{}_{}-{}".format(recipe_bs['name'], recipe_bs['epoch'], recipe_bs['version'], recipe_bs['revision'])
+
+        for task, data in recipe_bs['tasks'].copy().items():
+            recipe_bs['tasks'][task] = BSTask(data)
+
         buildstats[recipe_bs['name']] = recipe_bs
 
     return buildstats
@@ -253,12 +272,6 @@ def print_ver_diff(bs1, bs2):
             print(fmt_str.format(pkg, field1, field2, maxlen=maxlen))
 
 
-def task_time(task):
-    """Calculate sum of user and system time taken by a task"""
-    cputime = task['rusage']['ru_stime'] + task['rusage']['ru_utime'] + \
-              task['child_rusage']['ru_stime'] + task['child_rusage']['ru_utime']
-    return cputime
-
 
 def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
     """Diff task execution times"""
@@ -283,8 +296,8 @@ def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
             if len(task) > task_maxlen:
                 task_maxlen = len(task)
 
-            t1 = task_time(bs1[pkg]['tasks'][task]) if task in tasks1 else 0
-            t2 = task_time(bs2[pkg]['tasks'][task]) if task in tasks2 else 0
+            t1 = bs1[pkg]['tasks'][task].cputime if task in tasks1 else 0
+            t2 = bs2[pkg]['tasks'][task].cputime if task in tasks2 else 0
             task_op = '  '
             if t1 == 0:
                 reldiff = float('inf')
@@ -334,8 +347,8 @@ def print_timediff_summary(bs1, bs2):
     def total_cputime(buildstats):
         sum = 0.0
         for recipe_data in buildstats.values():
-            for task_data in recipe_data['tasks'].values():
-                sum += task_time(task_data)
+            for bs_task in recipe_data['tasks'].values():
+                sum += bs_task.cputime
         return sum
 
     def hms_time(secs):
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 04/11] scripts/buildstats-diff: do not hardcode field widths in output
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
                   ` (2 preceding siblings ...)
  2016-09-29 14:28 ` [PATCH 03/11] scripts/buildstats-diff: implement BSTask class Markus Lehtonen
@ 2016-09-29 14:28 ` Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 05/11] scripts/buildstats-diff: introduce --diff-attr Markus Lehtonen
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:28 UTC (permalink / raw)
  To: openembedded-core

Dynamically adjust the width of all fields in task diff output. Makes
it easier to print other units than cputime, too.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 60 ++++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index 22c44ff..56ac840 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -276,13 +276,14 @@ def print_ver_diff(bs1, bs2):
 def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
     """Diff task execution times"""
     tasks_diff = []
-    pkg_maxlen = 0
-    task_maxlen = 0
+
+    if min_val:
+        print("Ignoring tasks shorter than {}s".format(min_val))
+    if min_absdiff:
+        print("Ignoring time differences shorter than {}s".format(min_absdiff))
 
     pkgs = set(bs1.keys()).union(set(bs2.keys()))
     for pkg in pkgs:
-        if len(pkg) > pkg_maxlen:
-            pkg_maxlen = len(pkg)
         tasks1 = bs1[pkg]['tasks'] if pkg in bs1 else {}
         tasks2 = bs2[pkg]['tasks'] if pkg in bs2 else {}
         if not tasks1:
@@ -293,9 +294,6 @@ def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
             pkg_op = '  '
 
         for task in set(tasks1.keys()).union(set(tasks2.keys())):
-            if len(task) > task_maxlen:
-                task_maxlen = len(task)
-
             t1 = bs1[pkg]['tasks'][task].cputime if task in tasks1 else 0
             t2 = bs2[pkg]['tasks'][task].cputime if task in tasks2 else 0
             task_op = '  '
@@ -307,18 +305,15 @@ def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
                 if t2 == 0:
                     task_op = '- '
 
+            cputime = max(t1, t2)
+            if cputime < min_val:
+                log.debug("Filtering out %s:%s (%0.1fs)", pkg, task, cputime)
+                continue
+            if abs(t2-t1) < min_absdiff:
+                log.debug("Filtering out %s:%s (difference of %0.1fs)", pkg, task, t2-t1)
+                continue
             tasks_diff.append(TaskDiff(pkg, pkg_op, task, task_op, t1, t2, t2-t1, reldiff))
 
-    if min_val:
-        print("Ignoring tasks shorter than {}s".format(min_val))
-    if min_absdiff:
-        print("Ignoring time differences shorter than {}s".format(min_absdiff))
-
-    print()
-    print("  {:{pkg_maxlen}}   {:{task_maxlen}} {:>8} {:>10} {:>10}    {}".format(
-            'PKG', 'TASK', 'ABSDIFF', 'RELDIFF', 'CPUTIME1', 'CPUTIME2',
-            pkg_maxlen=pkg_maxlen, task_maxlen=task_maxlen))
-
     # Sort our list
     for field in reversed(sort_by):
         if field.startswith('-'):
@@ -328,18 +323,27 @@ def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
             reverse = False
         tasks_diff = sorted(tasks_diff, key=attrgetter(field), reverse=reverse)
 
+    linedata = [('  ', 'PKG', '  ', 'TASK', 'ABSDIFF', 'RELDIFF', 'CPUTIME1', 'CPUTIME2')]
+    field_lens = dict([('len_{}'.format(i), len(f)) for i, f in enumerate(linedata[0])])
+
+    # Prepare fields in string format and measure field lengths
     for diff in tasks_diff:
-        cputime = max(diff.cputime1, diff.cputime2)
-        if cputime > min_val:
-            if abs(diff.absdiff) > min_absdiff:
-                task_prefix = diff.task_op if diff.pkg_op == '  ' else '  '
-                print("{}{:{pkg_maxlen}} {}{:{task_maxlen}} {:+7.1f}s {:+9.1f}% {:9.1f}s -> {:.1f}s".format(
-                        diff.pkg_op, diff.pkg, task_prefix, diff.task, diff.absdiff, diff.reldiff, diff.cputime1, diff.cputime2,
-                        pkg_maxlen=pkg_maxlen, task_maxlen=task_maxlen))
-            else:
-                log.debug("Filtering out %s (difference of %0.1fs)", task, diff.absdiff)
-        else:
-            log.debug("Filtering out %s (%0.1fs)", task, cputime)
+        task_prefix = diff.task_op if diff.pkg_op == '  ' else '  '
+        linedata.append((diff.pkg_op, diff.pkg, task_prefix, diff.task,
+                         '{:+.1f}'.format(diff.absdiff),
+                         '{:+.1f}%'.format(diff.reldiff),
+                         '{:.1f}s'.format(diff.cputime1),
+                         '{:.1f}s'.format(diff.cputime2)))
+        for i, field in enumerate(linedata[-1]):
+            key = 'len_{}'.format(i)
+            if len(field) > field_lens[key]:
+                field_lens[key] = len(field)
+
+    # Print data
+    print()
+    for fields in linedata:
+        print("{:{len_0}}{:{len_1}}  {:{len_2}}{:{len_3}}  {:>{len_4}}  {:>{len_5}}  {:>{len_6}} -> {:{len_7}}".format(
+                *fields, **field_lens))
 
 
 def print_timediff_summary(bs1, bs2):
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 05/11] scripts/buildstats-diff: introduce --diff-attr
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
                   ` (3 preceding siblings ...)
  2016-09-29 14:28 ` [PATCH 04/11] scripts/buildstats-diff: do not hardcode field widths in output Markus Lehtonen
@ 2016-09-29 14:28 ` Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 06/11] scripts/buildstats-diff: add read_bytes and write_bytes to --diff-attr Markus Lehtonen
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:28 UTC (permalink / raw)
  To: openembedded-core

A new command line option for choosing which "attribute" of the
buildstats to compare. At first, the already supported 'cputime' is the
only available option. But, refactoring done in this patch should make
it easy to add new attribute types.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 140 +++++++++++++++++++++++++++++-------------------
 1 file changed, 84 insertions(+), 56 deletions(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index 56ac840..05bf74c 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -48,8 +48,7 @@ TIMEZONES = {'UTC': TimeZone(0, 'UTC'),
              'EET': TimeZone(7200, 'EET'),
              'EEST': TimeZone(10800, 'EEST')}
 
-
-taskdiff_fields = ('pkg', 'pkg_op', 'task', 'task_op', 'cputime1', 'cputime2',
+taskdiff_fields = ('pkg', 'pkg_op', 'task', 'task_op', 'value1', 'value2',
                    'absdiff', 'reldiff')
 TaskDiff = namedtuple('TaskDiff', ' '.join(taskdiff_fields))
 
@@ -272,16 +271,46 @@ def print_ver_diff(bs1, bs2):
             print(fmt_str.format(pkg, field1, field2, maxlen=maxlen))
 
 
-
-def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
+def print_task_diff(bs1, bs2, val_type, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
     """Diff task execution times"""
+    def val_to_str(val, human_readable=False):
+        """Convert raw value to printable string"""
+        def hms_time(secs):
+            """Get time in human-readable HH:MM:SS format"""
+            h = int(secs / 3600)
+            m = int((secs % 3600) / 60)
+            s = secs % 60
+            if h == 0:
+                return "{:02d}:{:04.1f}".format(m, s)
+            else:
+                return "{:d}:{:02d}:{:04.1f}".format(h, m, s)
+
+        if val_type == 'cputime':
+            if human_readable:
+                return hms_time(val)
+            else:
+                return "{:.1f}s".format(val)
+        else:
+            return str(val)
+
+    def sum_vals(buildstats):
+        """Get cumulative sum of all tasks"""
+        total = 0.0
+        for recipe_data in buildstats.values():
+            for bs_task in recipe_data['tasks'].values():
+                total += getattr(bs_task, val_type)
+        return total
+
     tasks_diff = []
 
     if min_val:
-        print("Ignoring tasks shorter than {}s".format(min_val))
+        print("Ignoring tasks less than {} ({})".format(
+                val_to_str(min_val, True), val_to_str(min_val)))
     if min_absdiff:
-        print("Ignoring time differences shorter than {}s".format(min_absdiff))
+        print("Ignoring differences less than {} ({})".format(
+                val_to_str(min_absdiff, True), val_to_str(min_absdiff)))
 
+    # Prepare the data
     pkgs = set(bs1.keys()).union(set(bs2.keys()))
     for pkg in pkgs:
         tasks1 = bs1[pkg]['tasks'] if pkg in bs1 else {}
@@ -294,25 +323,27 @@ def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
             pkg_op = '  '
 
         for task in set(tasks1.keys()).union(set(tasks2.keys())):
-            t1 = bs1[pkg]['tasks'][task].cputime if task in tasks1 else 0
-            t2 = bs2[pkg]['tasks'][task].cputime if task in tasks2 else 0
+            val1 = getattr(bs1[pkg]['tasks'][task], val_type) if task in tasks1 else 0
+            val2 = getattr(bs2[pkg]['tasks'][task], val_type) if task in tasks2 else 0
             task_op = '  '
-            if t1 == 0:
+            if val1 == 0:
                 reldiff = float('inf')
                 task_op = '+ '
             else:
-                reldiff = 100 * (t2 - t1) / t1
-                if t2 == 0:
+                reldiff = 100 * (val2 - val1) / val1
+                if val2 == 0:
                     task_op = '- '
 
-            cputime = max(t1, t2)
-            if cputime < min_val:
-                log.debug("Filtering out %s:%s (%0.1fs)", pkg, task, cputime)
+            if max(val1, val2) < min_val:
+                log.debug("Filtering out %s:%s (%s)", pkg, task,
+                          val_to_str(max(val1, val2)))
                 continue
-            if abs(t2-t1) < min_absdiff:
-                log.debug("Filtering out %s:%s (difference of %0.1fs)", pkg, task, t2-t1)
+            if abs(val2 - val1) < min_absdiff:
+                log.debug("Filtering out %s:%s (difference of %s)", pkg, task,
+                          val_to_str(val2-val1))
                 continue
-            tasks_diff.append(TaskDiff(pkg, pkg_op, task, task_op, t1, t2, t2-t1, reldiff))
+            tasks_diff.append(TaskDiff(pkg, pkg_op, task, task_op, val1, val2,
+                                       val2-val1, reldiff))
 
     # Sort our list
     for field in reversed(sort_by):
@@ -323,17 +354,18 @@ def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
             reverse = False
         tasks_diff = sorted(tasks_diff, key=attrgetter(field), reverse=reverse)
 
-    linedata = [('  ', 'PKG', '  ', 'TASK', 'ABSDIFF', 'RELDIFF', 'CPUTIME1', 'CPUTIME2')]
+    linedata = [('  ', 'PKG', '  ', 'TASK', 'ABSDIFF', 'RELDIFF',
+                val_type.upper() + '1', val_type.upper() + '2')]
     field_lens = dict([('len_{}'.format(i), len(f)) for i, f in enumerate(linedata[0])])
 
     # Prepare fields in string format and measure field lengths
     for diff in tasks_diff:
         task_prefix = diff.task_op if diff.pkg_op == '  ' else '  '
         linedata.append((diff.pkg_op, diff.pkg, task_prefix, diff.task,
-                         '{:+.1f}'.format(diff.absdiff),
+                         val_to_str(diff.absdiff),
                          '{:+.1f}%'.format(diff.reldiff),
-                         '{:.1f}s'.format(diff.cputime1),
-                         '{:.1f}s'.format(diff.cputime2)))
+                         val_to_str(diff.value1),
+                         val_to_str(diff.value2)))
         for i, field in enumerate(linedata[-1]):
             key = 'len_{}'.format(i)
             if len(field) > field_lens[key]:
@@ -345,33 +377,14 @@ def print_task_diff(bs1, bs2, min_val=0, min_absdiff=0, sort_by=('absdiff',)):
         print("{:{len_0}}{:{len_1}}  {:{len_2}}{:{len_3}}  {:>{len_4}}  {:>{len_5}}  {:>{len_6}} -> {:{len_7}}".format(
                 *fields, **field_lens))
 
-
-def print_timediff_summary(bs1, bs2):
-    """Print summary of the timediffs"""
-    def total_cputime(buildstats):
-        sum = 0.0
-        for recipe_data in buildstats.values():
-            for bs_task in recipe_data['tasks'].values():
-                sum += bs_task.cputime
-        return sum
-
-    def hms_time(secs):
-        """Get time in human-readable HH:MM:SS format"""
-        h = int(secs / 3600)
-        m = int((secs % 3600) / 60)
-        s = secs % 60
-        if h == 0:
-            return "{:02d}:{:04.1f}".format(m, s)
-        else:
-            return "{:d}:{:02d}:{:04.1f}".format(h, m, s)
-
-    total1 = total_cputime(bs1)
-    total2 = total_cputime(bs2)
-    print("\nCumulative CPU Time:")
-    print ("  {:+.1f}s    {:+.1f}%    {} ({:.1f}s) -> {} ({:.1f}s)".format(
-                total2 - total1, 100 * (total2-total1) / total1,
-                hms_time(total1), total1, hms_time(total2), total2))
-
+    # Print summary of the diffs
+    total1 = sum_vals(bs1)
+    total2 = sum_vals(bs2)
+    print("\nCumulative {}:".format(val_type))
+    print ("  {}    {:+.1f}%    {} ({}) -> {} ({})".format(
+                val_to_str(total2 - total1), 100 * (total2-total1) / total1,
+                val_to_str(total1, True), val_to_str(total1),
+                val_to_str(total2, True), val_to_str(total2)))
 
 
 def parse_args(argv):
@@ -382,15 +395,21 @@ Script for comparing buildstats of two separate builds."""
             formatter_class=argparse.ArgumentDefaultsHelpFormatter,
             description=description)
 
+    min_val_defaults = {'cputime': 3.0}
+    min_absdiff_defaults = {'cputime': 1.0}
+
     parser.add_argument('--debug', '-d', action='store_true',
                         help="Verbose logging")
     parser.add_argument('--ver-diff', action='store_true',
                         help="Show package version differences and exit")
-    parser.add_argument('--min-val', default=3.0, type=float,
-                        help="Filter out tasks shorter than MIN_VAL seconds")
-    parser.add_argument('--min-absdiff', default=1.0, type=float,
-                        help="Filter out tasks whose difference in cputime is "
-                             "less that  MIN_ABSDIFF seconds")
+    parser.add_argument('--diff-attr', default='cputime', choices=('cputime',),
+                        help="Buildstat attribute which to compare")
+    parser.add_argument('--min-val', default=min_val_defaults, type=float,
+                        help="Filter out tasks less than MIN_VAL. "
+                             "Default depends on --diff-attr.")
+    parser.add_argument('--min-absdiff', default=min_absdiff_defaults, type=float,
+                        help="Filter out tasks whose difference is less than "
+                             "MIN_ABSDIFF, Default depends on --diff-attr.")
     parser.add_argument('--sort-by', default='absdiff',
                         help="Comma-separated list of field sort order. "
                              "Prepend the field name with '-' for reversed sort. "
@@ -398,7 +417,16 @@ Script for comparing buildstats of two separate builds."""
     parser.add_argument('buildstats1', metavar='BUILDSTATS1', help="'Left' buildstat")
     parser.add_argument('buildstats2', metavar='BUILDSTATS2', help="'Right' buildstat")
 
-    return parser.parse_args(argv)
+    args = parser.parse_args(argv)
+
+    # Handle defaults for the filter arguments
+    if args.min_val is min_val_defaults:
+        args.min_val = min_val_defaults[args.diff_attr]
+    if args.min_absdiff is min_absdiff_defaults:
+        args.min_absdiff = min_absdiff_defaults[args.diff_attr]
+
+    return args
+
 
 def main(argv=None):
     """Script entry point"""
@@ -422,8 +450,8 @@ def main(argv=None):
     if args.ver_diff:
         print_ver_diff(bs1, bs2)
     else:
-        print_task_diff(bs1, bs2, args.min_val, args.min_absdiff, sort_by)
-        print_timediff_summary(bs1, bs2)
+        print_task_diff(bs1, bs2, args.diff_attr, args.min_val,
+                        args.min_absdiff, sort_by)
 
     return 0
 
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 06/11] scripts/buildstats-diff: add read_bytes and write_bytes to --diff-attr
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
                   ` (4 preceding siblings ...)
  2016-09-29 14:28 ` [PATCH 05/11] scripts/buildstats-diff: introduce --diff-attr Markus Lehtonen
@ 2016-09-29 14:28 ` Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 07/11] scripts/buildstats-diff: add read_ops and write_ops " Markus Lehtonen
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:28 UTC (permalink / raw)
  To: openembedded-core

These are I/O counter values from /proc/<pid>/io and represent the
number of bytes read from / written to the storage layer. Default values
for --min-val and --min-absdiff limits are set to 512kB and 128kB,
respectively.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 46 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index 05bf74c..3c894cb 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -17,6 +17,7 @@ import argparse
 import glob
 import json
 import logging
+import math
 import os
 import re
 import sys
@@ -77,6 +78,15 @@ class BSTask(dict):
         return self['rusage']['ru_stime'] + self['rusage']['ru_utime'] + \
                self['child_rusage']['ru_stime'] + self['child_rusage']['ru_utime']
 
+    @property
+    def read_bytes(self):
+        """Bytes read from the block layer"""
+        return self['iostat']['read_bytes']
+
+    @property
+    def write_bytes(self):
+        """Bytes written to the block layer"""
+        return self['iostat']['write_bytes']
 
 def read_buildstats_file(buildstat_file):
     """Convert buildstat text file into dict/json"""
@@ -290,8 +300,13 @@ def print_task_diff(bs1, bs2, val_type, min_val=0, min_absdiff=0, sort_by=('absd
                 return hms_time(val)
             else:
                 return "{:.1f}s".format(val)
-        else:
-            return str(val)
+        elif 'bytes' in val_type and human_readable:
+                prefix = ['', 'Ki', 'Mi', 'Gi', 'Ti', 'Pi']
+                dec = int(math.log(val, 2) / 10)
+                prec = 1 if dec > 0 else 0
+                return "{:.{prec}f}{}B".format(val / (2 ** (10 * dec)),
+                                               prefix[dec], prec=prec)
+        return str(int(val))
 
     def sum_vals(buildstats):
         """Get cumulative sum of all tasks"""
@@ -323,16 +338,22 @@ def print_task_diff(bs1, bs2, val_type, min_val=0, min_absdiff=0, sort_by=('absd
             pkg_op = '  '
 
         for task in set(tasks1.keys()).union(set(tasks2.keys())):
-            val1 = getattr(bs1[pkg]['tasks'][task], val_type) if task in tasks1 else 0
-            val2 = getattr(bs2[pkg]['tasks'][task], val_type) if task in tasks2 else 0
             task_op = '  '
+            if task in tasks1:
+                val1 = getattr(bs1[pkg]['tasks'][task], val_type)
+            else:
+                task_op = '+ '
+                val1 = 0
+            if task in tasks2:
+                val2 = getattr(bs2[pkg]['tasks'][task], val_type)
+            else:
+                val2 = 0
+                task_op = '- '
+
             if val1 == 0:
                 reldiff = float('inf')
-                task_op = '+ '
             else:
                 reldiff = 100 * (val2 - val1) / val1
-                if val2 == 0:
-                    task_op = '- '
 
             if max(val1, val2) < min_val:
                 log.debug("Filtering out %s:%s (%s)", pkg, task,
@@ -395,14 +416,19 @@ Script for comparing buildstats of two separate builds."""
             formatter_class=argparse.ArgumentDefaultsHelpFormatter,
             description=description)
 
-    min_val_defaults = {'cputime': 3.0}
-    min_absdiff_defaults = {'cputime': 1.0}
+    min_val_defaults = {'cputime': 3.0,
+                        'read_bytes': 524288,
+                        'write_bytes': 524288}
+    min_absdiff_defaults = {'cputime': 1.0,
+                            'read_bytes': 131072,
+                            'write_bytes': 131072}
 
     parser.add_argument('--debug', '-d', action='store_true',
                         help="Verbose logging")
     parser.add_argument('--ver-diff', action='store_true',
                         help="Show package version differences and exit")
-    parser.add_argument('--diff-attr', default='cputime', choices=('cputime',),
+    parser.add_argument('--diff-attr', default='cputime',
+                        choices=('cputime', 'read_bytes', 'write_bytes'),
                         help="Buildstat attribute which to compare")
     parser.add_argument('--min-val', default=min_val_defaults, type=float,
                         help="Filter out tasks less than MIN_VAL. "
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 07/11] scripts/buildstats-diff: add read_ops and write_ops to --diff-attr
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
                   ` (5 preceding siblings ...)
  2016-09-29 14:28 ` [PATCH 06/11] scripts/buildstats-diff: add read_bytes and write_bytes to --diff-attr Markus Lehtonen
@ 2016-09-29 14:28 ` Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 08/11] scripts/buildstats-diff: add walltime " Markus Lehtonen
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:28 UTC (permalink / raw)
  To: openembedded-core

Two new options, making it possible to compare the number of filesystem
operations of tasks. Defaults for --min-val and --min-absdiff are set to
more or less arbitrary 500 and 50 operations, respectively.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index 3c894cb..c6fa803 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -88,6 +88,17 @@ class BSTask(dict):
         """Bytes written to the block layer"""
         return self['iostat']['write_bytes']
 
+    @property
+    def read_ops(self):
+        """Number of read operations on the block layer"""
+        return self['rusage']['ru_inblock'] + self['child_rusage']['ru_inblock']
+
+    @property
+    def write_ops(self):
+        """Number of write operations on the block layer"""
+        return self['rusage']['ru_oublock'] + self['child_rusage']['ru_oublock']
+
+
 def read_buildstats_file(buildstat_file):
     """Convert buildstat text file into dict/json"""
     bs_task = BSTask()
@@ -306,6 +317,12 @@ def print_task_diff(bs1, bs2, val_type, min_val=0, min_absdiff=0, sort_by=('absd
                 prec = 1 if dec > 0 else 0
                 return "{:.{prec}f}{}B".format(val / (2 ** (10 * dec)),
                                                prefix[dec], prec=prec)
+        elif 'ops' in val_type and human_readable:
+                prefix = ['', 'k', 'M', 'G', 'T', 'P']
+                dec = int(math.log(val, 1000))
+                prec = 1 if dec > 0 else 0
+                return "{:.{prec}f}{}ops".format(val / (1000 ** dec),
+                                                 prefix[dec], prec=prec)
         return str(int(val))
 
     def sum_vals(buildstats):
@@ -418,17 +435,21 @@ Script for comparing buildstats of two separate builds."""
 
     min_val_defaults = {'cputime': 3.0,
                         'read_bytes': 524288,
-                        'write_bytes': 524288}
+                        'write_bytes': 524288,
+                        'read_ops': 500,
+                        'write_ops': 500}
     min_absdiff_defaults = {'cputime': 1.0,
                             'read_bytes': 131072,
-                            'write_bytes': 131072}
+                            'write_bytes': 131072,
+                            'read_ops': 50,
+                            'write_ops': 50}
 
     parser.add_argument('--debug', '-d', action='store_true',
                         help="Verbose logging")
     parser.add_argument('--ver-diff', action='store_true',
                         help="Show package version differences and exit")
     parser.add_argument('--diff-attr', default='cputime',
-                        choices=('cputime', 'read_bytes', 'write_bytes'),
+                        choices=min_val_defaults.keys(),
                         help="Buildstat attribute which to compare")
     parser.add_argument('--min-val', default=min_val_defaults, type=float,
                         help="Filter out tasks less than MIN_VAL. "
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 08/11] scripts/buildstats-diff: add walltime to --diff-attr
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
                   ` (6 preceding siblings ...)
  2016-09-29 14:28 ` [PATCH 07/11] scripts/buildstats-diff: add read_ops and write_ops " Markus Lehtonen
@ 2016-09-29 14:28 ` Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 09/11] scripts/buildstats-diff: use exception for internal error handling Markus Lehtonen
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:28 UTC (permalink / raw)
  To: openembedded-core

For comparing the elapsed wall clock time of tests. Default values for
--min-val and --min-absdiff are 5 seconds and 2 seconds.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index c6fa803..f26a6c1 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -79,6 +79,11 @@ class BSTask(dict):
                self['child_rusage']['ru_stime'] + self['child_rusage']['ru_utime']
 
     @property
+    def walltime(self):
+        """Elapsed wall clock time"""
+        return self['elapsed_time'].total_seconds()
+
+    @property
     def read_bytes(self):
         """Bytes read from the block layer"""
         return self['iostat']['read_bytes']
@@ -306,7 +311,7 @@ def print_task_diff(bs1, bs2, val_type, min_val=0, min_absdiff=0, sort_by=('absd
             else:
                 return "{:d}:{:02d}:{:04.1f}".format(h, m, s)
 
-        if val_type == 'cputime':
+        if 'time' in val_type:
             if human_readable:
                 return hms_time(val)
             else:
@@ -437,12 +442,14 @@ Script for comparing buildstats of two separate builds."""
                         'read_bytes': 524288,
                         'write_bytes': 524288,
                         'read_ops': 500,
-                        'write_ops': 500}
+                        'write_ops': 500,
+                        'walltime': 5}
     min_absdiff_defaults = {'cputime': 1.0,
                             'read_bytes': 131072,
                             'write_bytes': 131072,
                             'read_ops': 50,
-                            'write_ops': 50}
+                            'write_ops': 50,
+                            'walltime': 2}
 
     parser.add_argument('--debug', '-d', action='store_true',
                         help="Verbose logging")
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 09/11] scripts/buildstats-diff: use exception for internal error handling
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
                   ` (7 preceding siblings ...)
  2016-09-29 14:28 ` [PATCH 08/11] scripts/buildstats-diff: add walltime " Markus Lehtonen
@ 2016-09-29 14:28 ` Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 10/11] scripts/buildstats-diff: make logger msg format a bit more readable Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 11/11] scripts/buildstats-diff: implement --multi option Markus Lehtonen
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:28 UTC (permalink / raw)
  To: openembedded-core

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index f26a6c1..3c6cb1e 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -30,6 +30,11 @@ logging.basicConfig(level=logging.INFO)
 log = logging.getLogger()
 
 
+class ScriptError(Exception):
+    """Exception for internal error handling of this script"""
+    pass
+
+
 class TimeZone(tzinfo):
     """Simple fixed-offset tzinfo"""
     def __init__(self, seconds, name):
@@ -161,8 +166,7 @@ def read_buildstats_dir(bs_dir):
             log.warning("Multiple buildstats found, using the first one")
         top_dir = subdirs[0]
     else:
-        log.error("No such directory: %s", bs_dir)
-        sys.exit(1)
+        raise ScriptError("No such directory: {}".format(bs_dir))
     log.debug("Reading buildstats directory %s", top_dir)
     subdirs = os.listdir(top_dir)
 
@@ -187,9 +191,8 @@ def read_buildstats_dir(bs_dir):
             recipe_bs['tasks'][task] = read_buildstats_file(
                     os.path.join(recipe_dir, task))
         if name in buildstats:
-            log.error("Cannot handle multiple versions of the same package (%s)",
-                      name)
-            sys.exit(1)
+            raise ScriptError("Cannot handle multiple versions of the same "
+                              "package ({})".format(name))
         buildstats[name] = recipe_bs
 
     return buildstats
@@ -202,9 +205,8 @@ def read_buildstats_json(path):
         bs_json = json.load(fobj)
     for recipe_bs in bs_json:
         if recipe_bs['name'] in buildstats:
-            log.error("Cannot handle multiple versions of the same package (%s)",
-                      recipe_bs['name'])
-            sys.exit(1)
+            raise ScriptError("Cannot handle multiple versions of the same "
+                              "package ({})".format(recipe_bs['name']))
 
         if recipe_bs['epoch'] is None:
             recipe_bs['nevr'] = "{}-{}-{}".format(recipe_bs['name'], recipe_bs['version'], recipe_bs['revision'])
@@ -497,16 +499,18 @@ def main(argv=None):
             sys.exit(1)
         sort_by.append(field)
 
+    try:
+        bs1 = read_buildstats(args.buildstats1)
+        bs2 = read_buildstats(args.buildstats2)
 
-    bs1 = read_buildstats(args.buildstats1)
-    bs2 = read_buildstats(args.buildstats2)
-
-    if args.ver_diff:
-        print_ver_diff(bs1, bs2)
-    else:
-        print_task_diff(bs1, bs2, args.diff_attr, args.min_val,
-                        args.min_absdiff, sort_by)
-
+        if args.ver_diff:
+            print_ver_diff(bs1, bs2)
+        else:
+            print_task_diff(bs1, bs2, args.diff_attr, args.min_val,
+                            args.min_absdiff, sort_by)
+    except ScriptError as err:
+        log.error(str(err))
+        return 1
     return 0
 
 if __name__ == "__main__":
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 10/11] scripts/buildstats-diff: make logger msg format a bit more readable
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
                   ` (8 preceding siblings ...)
  2016-09-29 14:28 ` [PATCH 09/11] scripts/buildstats-diff: use exception for internal error handling Markus Lehtonen
@ 2016-09-29 14:28 ` Markus Lehtonen
  2016-09-29 14:28 ` [PATCH 11/11] scripts/buildstats-diff: implement --multi option Markus Lehtonen
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:28 UTC (permalink / raw)
  To: openembedded-core

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index 3c6cb1e..c5c48d3 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -26,7 +26,7 @@ from datetime import datetime, timedelta, tzinfo
 from operator import attrgetter
 
 # Setup logging
-logging.basicConfig(level=logging.INFO)
+logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s")
 log = logging.getLogger()
 
 
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 11/11] scripts/buildstats-diff: implement --multi option
  2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
                   ` (9 preceding siblings ...)
  2016-09-29 14:28 ` [PATCH 10/11] scripts/buildstats-diff: make logger msg format a bit more readable Markus Lehtonen
@ 2016-09-29 14:28 ` Markus Lehtonen
  10 siblings, 0 replies; 12+ messages in thread
From: Markus Lehtonen @ 2016-09-29 14:28 UTC (permalink / raw)
  To: openembedded-core

Makes it possible to average over multiple buildstats. If --multi is
specified (and the given path is a directory) the script will read all
buildstats from the given directory and use averaged values calculated
from them.

All of the buildstats must be from a "similar" build, meaning that no
differences in package versions or tasks are allowed. Otherwise, the
script will exit with an error.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/buildstats-diff | 99 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 73 insertions(+), 26 deletions(-)

diff --git a/scripts/buildstats-diff b/scripts/buildstats-diff
index c5c48d3..f918a6d 100755
--- a/scripts/buildstats-diff
+++ b/scripts/buildstats-diff
@@ -158,26 +158,15 @@ def read_buildstats_dir(bs_dir):
         epoch = match.group('epoch')
         return name, epoch, version, revision
 
-    if os.path.isfile(os.path.join(bs_dir, 'build_stats')):
-        top_dir = bs_dir
-    elif os.path.exists(bs_dir):
-        subdirs = sorted(glob.glob(bs_dir + '/*'))
-        if len(subdirs) > 1:
-            log.warning("Multiple buildstats found, using the first one")
-        top_dir = subdirs[0]
-    else:
-        raise ScriptError("No such directory: {}".format(bs_dir))
-    log.debug("Reading buildstats directory %s", top_dir)
-    subdirs = os.listdir(top_dir)
+    if not os.path.isfile(os.path.join(bs_dir, 'build_stats')):
+        raise ScriptError("{} does not look like a buildstats directory".format(bs_dir))
 
-    # Handle "old" style directory structure
-    if len(subdirs) == 1 and re.match('^20[0-9]{12}$', subdirs[0]):
-        top_dir = os.path.join(top_dir, subdirs[0])
-        subdirs = os.listdir(top_dir)
+    log.debug("Reading buildstats directory %s", bs_dir)
 
     buildstats = {}
+    subdirs = os.listdir(bs_dir)
     for dirname in subdirs:
-        recipe_dir = os.path.join(top_dir, dirname)
+        recipe_dir = os.path.join(bs_dir, dirname)
         if not os.path.isdir(recipe_dir):
             continue
         name, epoch, version, revision = split_nevr(dirname)
@@ -188,8 +177,8 @@ def read_buildstats_dir(bs_dir):
                      'revision': revision,
                      'tasks': {}}
         for task in os.listdir(recipe_dir):
-            recipe_bs['tasks'][task] = read_buildstats_file(
-                    os.path.join(recipe_dir, task))
+            recipe_bs['tasks'][task] = [read_buildstats_file(
+                    os.path.join(recipe_dir, task))]
         if name in buildstats:
             raise ScriptError("Cannot handle multiple versions of the same "
                               "package ({})".format(name))
@@ -198,6 +187,22 @@ def read_buildstats_dir(bs_dir):
     return buildstats
 
 
+def bs_append(dst, src):
+    """Append data from another buildstats"""
+    if set(dst.keys()) != set(src.keys()):
+        raise ScriptError("Refusing to join buildstats, set of packages is "
+                          "different")
+    for pkg, data in dst.items():
+        if data['nevr'] != src[pkg]['nevr']:
+            raise ScriptError("Refusing to join buildstats, package version "
+                              "differs: {} vs. {}".format(data['nevr'], src[pkg]['nevr']))
+        if set(data['tasks'].keys()) != set(src[pkg]['tasks'].keys()):
+            raise ScriptError("Refusing to join buildstats, set of tasks "
+                              "in {} differ".format(pkg))
+        for taskname, taskdata in data['tasks'].items():
+            taskdata.extend(src[pkg]['tasks'][taskname])
+
+
 def read_buildstats_json(path):
     """Read buildstats from JSON file"""
     buildstats = {}
@@ -214,20 +219,50 @@ def read_buildstats_json(path):
             recipe_bs['nevr'] = "{}-{}_{}-{}".format(recipe_bs['name'], recipe_bs['epoch'], recipe_bs['version'], recipe_bs['revision'])
 
         for task, data in recipe_bs['tasks'].copy().items():
-            recipe_bs['tasks'][task] = BSTask(data)
+            recipe_bs['tasks'][task] = [BSTask(data)]
 
         buildstats[recipe_bs['name']] = recipe_bs
 
     return buildstats
 
 
-def read_buildstats(path):
+def read_buildstats(path, multi):
     """Read buildstats"""
+    if not os.path.exists(path):
+        raise ScriptError("No such file or directory: {}".format(path))
+
     if os.path.isfile(path):
         return read_buildstats_json(path)
-    else:
+
+    if os.path.isfile(os.path.join(path, 'build_stats')):
         return read_buildstats_dir(path)
 
+    # Handle a non-buildstat directory
+    subpaths = sorted(glob.glob(path + '/*'))
+    if len(subpaths) > 1:
+        if multi:
+            log.info("Averaging over {} buildstats from {}".format(
+                     len(subpaths), path))
+        else:
+            raise ScriptError("Multiple buildstats found in '{}'. Please give "
+                              "a single buildstat directory of use the --multi "
+                              "option".format(path))
+    bs = None
+    for subpath in subpaths:
+        if os.path.isfile(subpath):
+            tmpbs = read_buildstats_json(subpath)
+        else:
+            tmpbs = read_buildstats_dir(subpath)
+        if not bs:
+            bs = tmpbs
+        else:
+            log.debug("Joining buildstats")
+            bs_append(bs, tmpbs)
+
+    if not bs:
+        raise ScriptError("No buildstats found under {}".format(path))
+    return bs
+
 
 def print_ver_diff(bs1, bs2):
     """Print package version differences"""
@@ -337,7 +372,7 @@ def print_task_diff(bs1, bs2, val_type, min_val=0, min_absdiff=0, sort_by=('absd
         total = 0.0
         for recipe_data in buildstats.values():
             for bs_task in recipe_data['tasks'].values():
-                total += getattr(bs_task, val_type)
+                total += sum([getattr(b, val_type) for b in bs_task]) / len(bs_task)
         return total
 
     tasks_diff = []
@@ -364,12 +399,16 @@ def print_task_diff(bs1, bs2, val_type, min_val=0, min_absdiff=0, sort_by=('absd
         for task in set(tasks1.keys()).union(set(tasks2.keys())):
             task_op = '  '
             if task in tasks1:
-                val1 = getattr(bs1[pkg]['tasks'][task], val_type)
+                # Average over all values
+                val1 = [getattr(b, val_type) for b in bs1[pkg]['tasks'][task]]
+                val1 = sum(val1) / len(val1)
             else:
                 task_op = '+ '
                 val1 = 0
             if task in tasks2:
-                val2 = getattr(bs2[pkg]['tasks'][task], val_type)
+                # Average over all values
+                val2 = [getattr(b, val_type) for b in bs2[pkg]['tasks'][task]]
+                val2 = sum(val2) / len(val2)
             else:
                 val2 = 0
                 task_op = '- '
@@ -470,11 +509,19 @@ Script for comparing buildstats of two separate builds."""
                         help="Comma-separated list of field sort order. "
                              "Prepend the field name with '-' for reversed sort. "
                              "Available fields are: {}".format(', '.join(taskdiff_fields)))
+    parser.add_argument('--multi', action='store_true',
+                        help="Read all buildstats from the given paths and "
+                             "average over them")
     parser.add_argument('buildstats1', metavar='BUILDSTATS1', help="'Left' buildstat")
     parser.add_argument('buildstats2', metavar='BUILDSTATS2', help="'Right' buildstat")
 
     args = parser.parse_args(argv)
 
+    # We do not nedd/want to read all buildstats if we just want to look at the
+    # package versions
+    if args.ver_diff:
+        args.multi = False
+
     # Handle defaults for the filter arguments
     if args.min_val is min_val_defaults:
         args.min_val = min_val_defaults[args.diff_attr]
@@ -500,8 +547,8 @@ def main(argv=None):
         sort_by.append(field)
 
     try:
-        bs1 = read_buildstats(args.buildstats1)
-        bs2 = read_buildstats(args.buildstats2)
+        bs1 = read_buildstats(args.buildstats1, args.multi)
+        bs2 = read_buildstats(args.buildstats2, args.multi)
 
         if args.ver_diff:
             print_ver_diff(bs1, bs2)
-- 
2.6.6



^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-09-29 14:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-29 14:27 [PATCH 00/11] Improvements to scripts/buildstats-diff Markus Lehtonen
2016-09-29 14:27 ` [PATCH 01/11] scripts/buildstats-diff: check that the given directory exists Markus Lehtonen
2016-09-29 14:27 ` [PATCH 02/11] scripts/buildstats-diff: rename --min-time and --min-timediff args Markus Lehtonen
2016-09-29 14:28 ` [PATCH 03/11] scripts/buildstats-diff: implement BSTask class Markus Lehtonen
2016-09-29 14:28 ` [PATCH 04/11] scripts/buildstats-diff: do not hardcode field widths in output Markus Lehtonen
2016-09-29 14:28 ` [PATCH 05/11] scripts/buildstats-diff: introduce --diff-attr Markus Lehtonen
2016-09-29 14:28 ` [PATCH 06/11] scripts/buildstats-diff: add read_bytes and write_bytes to --diff-attr Markus Lehtonen
2016-09-29 14:28 ` [PATCH 07/11] scripts/buildstats-diff: add read_ops and write_ops " Markus Lehtonen
2016-09-29 14:28 ` [PATCH 08/11] scripts/buildstats-diff: add walltime " Markus Lehtonen
2016-09-29 14:28 ` [PATCH 09/11] scripts/buildstats-diff: use exception for internal error handling Markus Lehtonen
2016-09-29 14:28 ` [PATCH 10/11] scripts/buildstats-diff: make logger msg format a bit more readable Markus Lehtonen
2016-09-29 14:28 ` [PATCH 11/11] scripts/buildstats-diff: implement --multi option Markus Lehtonen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.