All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] benchmark util
@ 2020-02-28  7:19 Vladimir Sementsov-Ogievskiy
  2020-02-28  7:19 ` [PATCH v3 1/4] scripts/simplebench: add simplebench.py Vladimir Sementsov-Ogievskiy
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-02-28  7:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, vsementsov, ehabkost, qemu-block, stefanha, mreitz, crosa,
	den, jsnow

Hi all!

v3:
  move all to scripts/simplebench
  add myself as a maintainer of this thing

Here is simple benchmarking utility, to generate performance
comparison tables, like the following:

----------  -------------  -------------  -------------
            backup-1       backup-2       mirror
ssd -> ssd  0.43 +- 0.00   4.48 +- 0.06   4.38 +- 0.02
ssd -> hdd  10.60 +- 0.08  10.69 +- 0.18  10.57 +- 0.05
ssd -> nbd  33.81 +- 0.37  10.67 +- 0.17  10.07 +- 0.07
----------  -------------  -------------  -------------

I'll use this benchmark in other series, hope someone
will like it.

Vladimir Sementsov-Ogievskiy (4):
  scripts/simplebench: add simplebench.py
  scripts/simplebench: add qemu/bench_block_job.py
  scripts/simplebench: add example usage of simplebench
  MAINTAINERS: add simplebench

 MAINTAINERS                            |   5 +
 scripts/simplebench/bench-example.py   |  80 ++++++++++++++++
 scripts/simplebench/bench_block_job.py | 119 +++++++++++++++++++++++
 scripts/simplebench/simplebench.py     | 128 +++++++++++++++++++++++++
 4 files changed, 332 insertions(+)
 create mode 100644 scripts/simplebench/bench-example.py
 create mode 100755 scripts/simplebench/bench_block_job.py
 create mode 100644 scripts/simplebench/simplebench.py

-- 
2.21.0



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

* [PATCH v3 1/4] scripts/simplebench: add simplebench.py
  2020-02-28  7:19 [PATCH v3 0/4] benchmark util Vladimir Sementsov-Ogievskiy
@ 2020-02-28  7:19 ` Vladimir Sementsov-Ogievskiy
  2020-02-28 13:03   ` Aleksandar Markovic
  2020-02-28  7:19 ` [PATCH v3 2/4] scripts/simplebench: add qemu/bench_block_job.py Vladimir Sementsov-Ogievskiy
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-02-28  7:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, vsementsov, ehabkost, qemu-block, stefanha, mreitz, crosa,
	den, jsnow

Add simple benchmark table creator.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 scripts/simplebench/simplebench.py | 128 +++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100644 scripts/simplebench/simplebench.py

diff --git a/scripts/simplebench/simplebench.py b/scripts/simplebench/simplebench.py
new file mode 100644
index 0000000000..59e7314ff6
--- /dev/null
+++ b/scripts/simplebench/simplebench.py
@@ -0,0 +1,128 @@
+#!/usr/bin/env python
+#
+# Simple benchmarking framework
+#
+# Copyright (c) 2019 Virtuozzo International GmbH.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+
+def bench_one(test_func, test_env, test_case, count=5, initial_run=True):
+    """Benchmark one test-case
+
+    test_func   -- benchmarking function with prototype
+                   test_func(env, case), which takes test_env and test_case
+                   arguments and returns {'seconds': int} (which is benchmark
+                   result) on success and {'error': str} on error. Returned
+                   dict may contain any other additional fields.
+    test_env    -- test environment - opaque first argument for test_func
+    test_case   -- test case - opaque second argument for test_func
+    count       -- how many times to call test_func, to calculate average
+    initial_run -- do initial run of test_func, which don't get into result
+
+    Returns dict with the following fields:
+        'runs':     list of test_func results
+        'average':  average seconds per run (exists only if at least one run
+                    succeeded)
+        'delta':    maximum delta between test_func result and the average
+                    (exists only if at least one run succeeded)
+        'n-failed': number of failed runs (exists only if at least one run
+                    failed)
+    """
+    if initial_run:
+        print('  #initial run:')
+        print('   ', test_func(test_env, test_case))
+
+    runs = []
+    for i in range(count):
+        print('  #run {}'.format(i+1))
+        res = test_func(test_env, test_case)
+        print('   ', res)
+        runs.append(res)
+
+    result = {'runs': runs}
+
+    successed = [r for r in runs if ('seconds' in r)]
+    if successed:
+        avg = sum(r['seconds'] for r in successed) / len(successed)
+        result['average'] = avg
+        result['delta'] = max(abs(r['seconds'] - avg) for r in successed)
+
+    if len(successed) < count:
+        result['n-failed'] = count - len(successed)
+
+    return result
+
+
+def ascii_one(result):
+    """Return ASCII representation of bench_one() returned dict."""
+    if 'average' in result:
+        s = '{:.2f} +- {:.2f}'.format(result['average'], result['delta'])
+        if 'n-failed' in result:
+            s += '\n({} failed)'.format(result['n-failed'])
+        return s
+    else:
+        return 'FAILED'
+
+
+def bench(test_func, test_envs, test_cases, *args, **vargs):
+    """Fill benchmark table
+
+    test_func -- benchmarking function, see bench_one for description
+    test_envs -- list of test environments, see bench_one
+    test_cases -- list of test cases, see bench_one
+    args, vargs -- additional arguments for bench_one
+
+    Returns dict with the following fields:
+        'envs':  test_envs
+        'cases': test_cases
+        'tab':   filled 2D array, where cell [i][j] is bench_one result for
+                 test_cases[i] for test_envs[j] (i.e., rows are test cases and
+                 columns are test environments)
+    """
+    tab = {}
+    results = {
+        'envs': test_envs,
+        'cases': test_cases,
+        'tab': tab
+    }
+    n = 1
+    n_tests = len(test_envs) * len(test_cases)
+    for env in test_envs:
+        for case in test_cases:
+            print('Testing {}/{}: {} :: {}'.format(n, n_tests,
+                                                   env['id'], case['id']))
+            if case['id'] not in tab:
+                tab[case['id']] = {}
+            tab[case['id']][env['id']] = bench_one(test_func, env, case,
+                                                   *args, **vargs)
+            n += 1
+
+    print('Done')
+    return results
+
+
+def ascii(results):
+    """Return ASCII representation of bench() returned dict."""
+    from tabulate import tabulate
+
+    tab = [[""] + [c['id'] for c in results['envs']]]
+    for case in results['cases']:
+        row = [case['id']]
+        for env in results['envs']:
+            row.append(ascii_one(results['tab'][case['id']][env['id']]))
+        tab.append(row)
+
+    return tabulate(tab)
-- 
2.21.0



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

* [PATCH v3 2/4] scripts/simplebench: add qemu/bench_block_job.py
  2020-02-28  7:19 [PATCH v3 0/4] benchmark util Vladimir Sementsov-Ogievskiy
  2020-02-28  7:19 ` [PATCH v3 1/4] scripts/simplebench: add simplebench.py Vladimir Sementsov-Ogievskiy
@ 2020-02-28  7:19 ` Vladimir Sementsov-Ogievskiy
  2020-02-28 13:04   ` Aleksandar Markovic
  2020-02-28  7:19 ` [PATCH v3 3/4] scripts/simplebench: add example usage of simplebench Vladimir Sementsov-Ogievskiy
  2020-02-28  7:19 ` [PATCH v3 4/4] MAINTAINERS: add simplebench Vladimir Sementsov-Ogievskiy
  3 siblings, 1 reply; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-02-28  7:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, vsementsov, ehabkost, qemu-block, stefanha, mreitz, crosa,
	den, jsnow

Add block-job benchmarking helper functions.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 scripts/simplebench/bench_block_job.py | 119 +++++++++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100755 scripts/simplebench/bench_block_job.py

diff --git a/scripts/simplebench/bench_block_job.py b/scripts/simplebench/bench_block_job.py
new file mode 100755
index 0000000000..9808d696cf
--- /dev/null
+++ b/scripts/simplebench/bench_block_job.py
@@ -0,0 +1,119 @@
+#!/usr/bin/env python
+#
+# Benchmark block jobs
+#
+# Copyright (c) 2019 Virtuozzo International GmbH.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+
+import sys
+import os
+import socket
+import json
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
+from qemu.machine import QEMUMachine
+from qemu.qmp import QMPConnectError
+
+
+def bench_block_job(cmd, cmd_args, qemu_args):
+    """Benchmark block-job
+
+    cmd       -- qmp command to run block-job (like blockdev-backup)
+    cmd_args  -- dict of qmp command arguments
+    qemu_args -- list of Qemu command line arguments, including path to Qemu
+                 binary
+
+    Returns {'seconds': int} on success and {'error': str} on failure, dict may
+    contain addional 'vm-log' field. Return value is compatible with
+    simplebench lib.
+    """
+
+    vm = QEMUMachine(qemu_args[0], args=qemu_args[1:])
+
+    try:
+        vm.launch()
+    except OSError as e:
+        return {'error': 'popen failed: ' + str(e)}
+    except (QMPConnectError, socket.timeout):
+        return {'error': 'qemu failed: ' + str(vm.get_log())}
+
+    try:
+        res = vm.qmp(cmd, **cmd_args)
+        if res != {'return': {}}:
+            vm.shutdown()
+            return {'error': '"{}" command failed: {}'.format(cmd, str(res))}
+
+        e = vm.event_wait('JOB_STATUS_CHANGE')
+        assert e['data']['status'] == 'created'
+        start_ms = e['timestamp']['seconds'] * 1000000 + \
+            e['timestamp']['microseconds']
+
+        e = vm.events_wait((('BLOCK_JOB_READY', None),
+                            ('BLOCK_JOB_COMPLETED', None),
+                            ('BLOCK_JOB_FAILED', None)), timeout=True)
+        if e['event'] not in ('BLOCK_JOB_READY', 'BLOCK_JOB_COMPLETED'):
+            vm.shutdown()
+            return {'error': 'block-job failed: ' + str(e),
+                    'vm-log': vm.get_log()}
+        end_ms = e['timestamp']['seconds'] * 1000000 + \
+            e['timestamp']['microseconds']
+    finally:
+        vm.shutdown()
+
+    return {'seconds': (end_ms - start_ms) / 1000000.0}
+
+
+# Bench backup or mirror
+def bench_block_copy(qemu_binary, cmd, source, target):
+    """Helper to run bench_block_job() for mirror or backup"""
+    assert cmd in ('blockdev-backup', 'blockdev-mirror')
+
+    source['node-name'] = 'source'
+    target['node-name'] = 'target'
+
+    return bench_block_job(cmd,
+                           {'job-id': 'job0', 'device': 'source',
+                            'target': 'target', 'sync': 'full'},
+                           [qemu_binary,
+                            '-blockdev', json.dumps(source),
+                            '-blockdev', json.dumps(target)])
+
+
+def drv_file(filename):
+    return {'driver': 'file', 'filename': filename,
+            'cache': {'direct': True}, 'aio': 'native'}
+
+
+def drv_nbd(host, port):
+    return {'driver': 'nbd',
+            'server': {'type': 'inet', 'host': host, 'port': port}}
+
+
+if __name__ == '__main__':
+    import sys
+
+    if len(sys.argv) < 4:
+        print('USAGE: {} <qmp block-job command name> '
+              '<json string of arguments for the command> '
+              '<qemu binary path and arguments>'.format(sys.argv[0]))
+        exit(1)
+
+    res = bench_block_job(sys.argv[1], json.loads(sys.argv[2]), sys.argv[3:])
+    if 'seconds' in res:
+        print('{:.2f}'.format(res['seconds']))
+    else:
+        print(res)
-- 
2.21.0



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

* [PATCH v3 3/4] scripts/simplebench: add example usage of simplebench
  2020-02-28  7:19 [PATCH v3 0/4] benchmark util Vladimir Sementsov-Ogievskiy
  2020-02-28  7:19 ` [PATCH v3 1/4] scripts/simplebench: add simplebench.py Vladimir Sementsov-Ogievskiy
  2020-02-28  7:19 ` [PATCH v3 2/4] scripts/simplebench: add qemu/bench_block_job.py Vladimir Sementsov-Ogievskiy
@ 2020-02-28  7:19 ` Vladimir Sementsov-Ogievskiy
  2020-02-28 13:27   ` Aleksandar Markovic
  2020-02-28  7:19 ` [PATCH v3 4/4] MAINTAINERS: add simplebench Vladimir Sementsov-Ogievskiy
  3 siblings, 1 reply; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-02-28  7:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, vsementsov, ehabkost, qemu-block, stefanha, mreitz, crosa,
	den, jsnow

This example may be used as a template for custom benchmark.
It illustrates three things to prepare:
 - define bench_func
 - define test environments (columns)
 - define test cases (rows)
And final call of simplebench API.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 scripts/simplebench/bench-example.py | 80 ++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 scripts/simplebench/bench-example.py

diff --git a/scripts/simplebench/bench-example.py b/scripts/simplebench/bench-example.py
new file mode 100644
index 0000000000..c642a5b891
--- /dev/null
+++ b/scripts/simplebench/bench-example.py
@@ -0,0 +1,80 @@
+#!/usr/bin/env python3
+#
+# Benchmark example
+#
+# Copyright (c) 2019 Virtuozzo International GmbH.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import simplebench
+from bench_block_job import bench_block_copy, drv_file, drv_nbd
+
+
+def bench_func(env, case):
+    """ Handle one "cell" of benchmarking table. """
+    return bench_block_copy(env['qemu_binary'], env['cmd'],
+                            case['source'], case['target'])
+
+
+# You may set the following five variables to correct values, to turn this
+# example to real benchmark.
+ssd_source = '/path-to-raw-source-image-at-ssd'
+ssd_target = '/path-to-raw-target-image-at-ssd'
+hdd_target = '/path-to-raw-source-image-at-hdd'
+nbd_ip = 'nbd-ip-addr'
+nbd_port = 'nbd-port-number'
+
+# Test-cases are "rows" in benchmark resulting table, 'id' is a caption for
+# the row, other fields are handled by bench_func.
+test_cases = [
+    {
+        'id': 'ssd -> ssd',
+        'source': drv_file(ssd_source),
+        'target': drv_file(ssd_target)
+    },
+    {
+        'id': 'ssd -> hdd',
+        'source': drv_file(ssd_source),
+        'target': drv_file(hdd_target)
+    },
+    {
+        'id': 'ssd -> nbd',
+        'source': drv_file(ssd_source),
+        'target': drv_nbd(nbd_ip, nbd_port)
+    },
+]
+
+# Test-envs are "columns" in benchmark resulting table, 'id is a caption for
+# the column, other fields are handled by bench_func.
+test_envs = [
+    {
+        'id': 'backup-1',
+        'cmd': 'blockdev-backup',
+        'qemu_binary': '/path-to-qemu-binary-1'
+    },
+    {
+        'id': 'backup-2',
+        'cmd': 'blockdev-backup',
+        'qemu_binary': '/path-to-qemu-binary-2'
+    },
+    {
+        'id': 'mirror',
+        'cmd': 'blockdev-mirror',
+        'qemu_binary': '/path-to-qemu-binary-1'
+    }
+]
+
+result = simplebench.bench(bench_func, test_envs, test_cases, count=3)
+print(simplebench.ascii(result))
-- 
2.21.0



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

* [PATCH v3 4/4] MAINTAINERS: add simplebench
  2020-02-28  7:19 [PATCH v3 0/4] benchmark util Vladimir Sementsov-Ogievskiy
                   ` (2 preceding siblings ...)
  2020-02-28  7:19 ` [PATCH v3 3/4] scripts/simplebench: add example usage of simplebench Vladimir Sementsov-Ogievskiy
@ 2020-02-28  7:19 ` Vladimir Sementsov-Ogievskiy
  2020-02-28 13:27   ` Aleksandar Markovic
  3 siblings, 1 reply; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-02-28  7:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, vsementsov, ehabkost, qemu-block, stefanha, mreitz, crosa,
	den, jsnow

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5e5e3e52d6..16d069adc5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2038,6 +2038,11 @@ F: python/qemu/*py
 F: scripts/*.py
 F: tests/*.py
 
+Benchmark util
+M: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+S: Maintained
+F: scripts/simplebench/
+
 QAPI
 M: Markus Armbruster <armbru@redhat.com>
 M: Michael Roth <mdroth@linux.vnet.ibm.com>
-- 
2.21.0



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

* Re: [PATCH v3 1/4] scripts/simplebench: add simplebench.py
  2020-02-28  7:19 ` [PATCH v3 1/4] scripts/simplebench: add simplebench.py Vladimir Sementsov-Ogievskiy
@ 2020-02-28 13:03   ` Aleksandar Markovic
  2020-02-28 13:48     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 13+ messages in thread
From: Aleksandar Markovic @ 2020-02-28 13:03 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Kevin Wolf, Eduardo Habkost, open list:bochs, Stefan Hajnoczi,
	QEMU Developers, Max Reitz, Cleber Rosa, Denis V. Lunev,
	John Snow

On Fri, Feb 28, 2020 at 8:19 AM Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
>
> Add simple benchmark table creator.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  scripts/simplebench/simplebench.py | 128 +++++++++++++++++++++++++++++
>  1 file changed, 128 insertions(+)
>  create mode 100644 scripts/simplebench/simplebench.py
>
> diff --git a/scripts/simplebench/simplebench.py b/scripts/simplebench/simplebench.py
> new file mode 100644
> index 0000000000..59e7314ff6
> --- /dev/null
> +++ b/scripts/simplebench/simplebench.py
> @@ -0,0 +1,128 @@
> +#!/usr/bin/env python
> +#
> +# Simple benchmarking framework
> +#
> +# Copyright (c) 2019 Virtuozzo International GmbH.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +
> +def bench_one(test_func, test_env, test_case, count=5, initial_run=True):
> +    """Benchmark one test-case
> +
> +    test_func   -- benchmarking function with prototype
> +                   test_func(env, case), which takes test_env and test_case
> +                   arguments and returns {'seconds': int} (which is benchmark
> +                   result) on success and {'error': str} on error. Returned
> +                   dict may contain any other additional fields.
> +    test_env    -- test environment - opaque first argument for test_func
> +    test_case   -- test case - opaque second argument for test_func
> +    count       -- how many times to call test_func, to calculate average
> +    initial_run -- do initial run of test_func, which don't get into result
> +
> +    Returns dict with the following fields:
> +        'runs':     list of test_func results
> +        'average':  average seconds per run (exists only if at least one run
> +                    succeeded)
> +        'delta':    maximum delta between test_func result and the average
> +                    (exists only if at least one run succeeded)
> +        'n-failed': number of failed runs (exists only if at least one run
> +                    failed)
> +    """
> +    if initial_run:
> +        print('  #initial run:')
> +        print('   ', test_func(test_env, test_case))
> +
> +    runs = []
> +    for i in range(count):
> +        print('  #run {}'.format(i+1))
> +        res = test_func(test_env, test_case)
> +        print('   ', res)
> +        runs.append(res)
> +
> +    result = {'runs': runs}
> +
> +    successed = [r for r in runs if ('seconds' in r)]
> +    if successed:
> +        avg = sum(r['seconds'] for r in successed) / len(successed)
> +        result['average'] = avg
> +        result['delta'] = max(abs(r['seconds'] - avg) for r in successed)
> +
> +    if len(successed) < count:
> +        result['n-failed'] = count - len(successed)
> +
> +    return result
> +
> +
> +def ascii_one(result):
> +    """Return ASCII representation of bench_one() returned dict."""
> +    if 'average' in result:
> +        s = '{:.2f} +- {:.2f}'.format(result['average'], result['delta'])
> +        if 'n-failed' in result:
> +            s += '\n({} failed)'.format(result['n-failed'])
> +        return s
> +    else:
> +        return 'FAILED'

I think it would be visually clearer if "+-" was printed without any
space between it and the following number, using something
like this:

s = ' {:.2f} +-{:.2f}'.format(result['average'], result['delta'])

The resulting table would look like:

----------  -------------  -------------  -------------
            backup-1       backup-2       mirror
ssd -> ssd   0.43 +-0.00    4.48 +-0.06    4.38 +-0.02
ssd -> hdd   10.60 +-0.08   10.69 +-0.18   10.57 +-0.05
ssd -> nbd   33.81 +-0.37   10.67 +-0.17   10.07 +-0.07
----------  -------------  -------------  -------------

But, this is just cosmetics.

With or without the suggestion above:

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

> +
> +
> +def bench(test_func, test_envs, test_cases, *args, **vargs):
> +    """Fill benchmark table
> +
> +    test_func -- benchmarking function, see bench_one for description
> +    test_envs -- list of test environments, see bench_one
> +    test_cases -- list of test cases, see bench_one
> +    args, vargs -- additional arguments for bench_one
> +
> +    Returns dict with the following fields:
> +        'envs':  test_envs
> +        'cases': test_cases
> +        'tab':   filled 2D array, where cell [i][j] is bench_one result for
> +                 test_cases[i] for test_envs[j] (i.e., rows are test cases and
> +                 columns are test environments)
> +    """
> +    tab = {}
> +    results = {
> +        'envs': test_envs,
> +        'cases': test_cases,
> +        'tab': tab
> +    }
> +    n = 1
> +    n_tests = len(test_envs) * len(test_cases)
> +    for env in test_envs:
> +        for case in test_cases:
> +            print('Testing {}/{}: {} :: {}'.format(n, n_tests,
> +                                                   env['id'], case['id']))
> +            if case['id'] not in tab:
> +                tab[case['id']] = {}
> +            tab[case['id']][env['id']] = bench_one(test_func, env, case,
> +                                                   *args, **vargs)
> +            n += 1
> +
> +    print('Done')
> +    return results
> +
> +
> +def ascii(results):
> +    """Return ASCII representation of bench() returned dict."""
> +    from tabulate import tabulate
> +
> +    tab = [[""] + [c['id'] for c in results['envs']]]
> +    for case in results['cases']:
> +        row = [case['id']]
> +        for env in results['envs']:
> +            row.append(ascii_one(results['tab'][case['id']][env['id']]))
> +        tab.append(row)
> +
> +    return tabulate(tab)
> --
> 2.21.0
>
>


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

* Re: [PATCH v3 2/4] scripts/simplebench: add qemu/bench_block_job.py
  2020-02-28  7:19 ` [PATCH v3 2/4] scripts/simplebench: add qemu/bench_block_job.py Vladimir Sementsov-Ogievskiy
@ 2020-02-28 13:04   ` Aleksandar Markovic
  0 siblings, 0 replies; 13+ messages in thread
From: Aleksandar Markovic @ 2020-02-28 13:04 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Kevin Wolf, Eduardo Habkost, open list:bochs, Stefan Hajnoczi,
	QEMU Developers, Max Reitz, Cleber Rosa, Denis V. Lunev,
	John Snow

On Fri, Feb 28, 2020 at 8:19 AM Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
>
> Add block-job benchmarking helper functions.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

>  scripts/simplebench/bench_block_job.py | 119 +++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100755 scripts/simplebench/bench_block_job.py
>
> diff --git a/scripts/simplebench/bench_block_job.py b/scripts/simplebench/bench_block_job.py
> new file mode 100755
> index 0000000000..9808d696cf
> --- /dev/null
> +++ b/scripts/simplebench/bench_block_job.py
> @@ -0,0 +1,119 @@
> +#!/usr/bin/env python
> +#
> +# Benchmark block jobs
> +#
> +# Copyright (c) 2019 Virtuozzo International GmbH.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +
> +import sys
> +import os
> +import socket
> +import json
> +
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> +from qemu.machine import QEMUMachine
> +from qemu.qmp import QMPConnectError
> +
> +
> +def bench_block_job(cmd, cmd_args, qemu_args):
> +    """Benchmark block-job
> +
> +    cmd       -- qmp command to run block-job (like blockdev-backup)
> +    cmd_args  -- dict of qmp command arguments
> +    qemu_args -- list of Qemu command line arguments, including path to Qemu
> +                 binary
> +
> +    Returns {'seconds': int} on success and {'error': str} on failure, dict may
> +    contain addional 'vm-log' field. Return value is compatible with
> +    simplebench lib.
> +    """
> +
> +    vm = QEMUMachine(qemu_args[0], args=qemu_args[1:])
> +
> +    try:
> +        vm.launch()
> +    except OSError as e:
> +        return {'error': 'popen failed: ' + str(e)}
> +    except (QMPConnectError, socket.timeout):
> +        return {'error': 'qemu failed: ' + str(vm.get_log())}
> +
> +    try:
> +        res = vm.qmp(cmd, **cmd_args)
> +        if res != {'return': {}}:
> +            vm.shutdown()
> +            return {'error': '"{}" command failed: {}'.format(cmd, str(res))}
> +
> +        e = vm.event_wait('JOB_STATUS_CHANGE')
> +        assert e['data']['status'] == 'created'
> +        start_ms = e['timestamp']['seconds'] * 1000000 + \
> +            e['timestamp']['microseconds']
> +
> +        e = vm.events_wait((('BLOCK_JOB_READY', None),
> +                            ('BLOCK_JOB_COMPLETED', None),
> +                            ('BLOCK_JOB_FAILED', None)), timeout=True)
> +        if e['event'] not in ('BLOCK_JOB_READY', 'BLOCK_JOB_COMPLETED'):
> +            vm.shutdown()
> +            return {'error': 'block-job failed: ' + str(e),
> +                    'vm-log': vm.get_log()}
> +        end_ms = e['timestamp']['seconds'] * 1000000 + \
> +            e['timestamp']['microseconds']
> +    finally:
> +        vm.shutdown()
> +
> +    return {'seconds': (end_ms - start_ms) / 1000000.0}
> +
> +
> +# Bench backup or mirror
> +def bench_block_copy(qemu_binary, cmd, source, target):
> +    """Helper to run bench_block_job() for mirror or backup"""
> +    assert cmd in ('blockdev-backup', 'blockdev-mirror')
> +
> +    source['node-name'] = 'source'
> +    target['node-name'] = 'target'
> +
> +    return bench_block_job(cmd,
> +                           {'job-id': 'job0', 'device': 'source',
> +                            'target': 'target', 'sync': 'full'},
> +                           [qemu_binary,
> +                            '-blockdev', json.dumps(source),
> +                            '-blockdev', json.dumps(target)])
> +
> +
> +def drv_file(filename):
> +    return {'driver': 'file', 'filename': filename,
> +            'cache': {'direct': True}, 'aio': 'native'}
> +
> +
> +def drv_nbd(host, port):
> +    return {'driver': 'nbd',
> +            'server': {'type': 'inet', 'host': host, 'port': port}}
> +
> +
> +if __name__ == '__main__':
> +    import sys
> +
> +    if len(sys.argv) < 4:
> +        print('USAGE: {} <qmp block-job command name> '
> +              '<json string of arguments for the command> '
> +              '<qemu binary path and arguments>'.format(sys.argv[0]))
> +        exit(1)
> +
> +    res = bench_block_job(sys.argv[1], json.loads(sys.argv[2]), sys.argv[3:])
> +    if 'seconds' in res:
> +        print('{:.2f}'.format(res['seconds']))
> +    else:
> +        print(res)
> --
> 2.21.0
>
>


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

* Re: [PATCH v3 3/4] scripts/simplebench: add example usage of simplebench
  2020-02-28  7:19 ` [PATCH v3 3/4] scripts/simplebench: add example usage of simplebench Vladimir Sementsov-Ogievskiy
@ 2020-02-28 13:27   ` Aleksandar Markovic
  0 siblings, 0 replies; 13+ messages in thread
From: Aleksandar Markovic @ 2020-02-28 13:27 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Kevin Wolf, Eduardo Habkost, open list:bochs, Stefan Hajnoczi,
	QEMU Developers, Max Reitz, Cleber Rosa, Denis V. Lunev,
	John Snow

On Fri, Feb 28, 2020 at 8:21 AM Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
>
> This example may be used as a template for custom benchmark.
> It illustrates three things to prepare:
>  - define bench_func
>  - define test environments (columns)
>  - define test cases (rows)
> And final call of simplebench API.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---

In future, it would be perhaps useful to add option to output results
in the format of csv (comma separated values) (or similar) - it would
simplify import to other data presentation tools. In any case:

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

>  scripts/simplebench/bench-example.py | 80 ++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 scripts/simplebench/bench-example.py
>
> diff --git a/scripts/simplebench/bench-example.py b/scripts/simplebench/bench-example.py
> new file mode 100644
> index 0000000000..c642a5b891
> --- /dev/null
> +++ b/scripts/simplebench/bench-example.py
> @@ -0,0 +1,80 @@
> +#!/usr/bin/env python3
> +#
> +# Benchmark example
> +#
> +# Copyright (c) 2019 Virtuozzo International GmbH.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import simplebench
> +from bench_block_job import bench_block_copy, drv_file, drv_nbd
> +
> +
> +def bench_func(env, case):
> +    """ Handle one "cell" of benchmarking table. """
> +    return bench_block_copy(env['qemu_binary'], env['cmd'],
> +                            case['source'], case['target'])
> +
> +
> +# You may set the following five variables to correct values, to turn this
> +# example to real benchmark.
> +ssd_source = '/path-to-raw-source-image-at-ssd'
> +ssd_target = '/path-to-raw-target-image-at-ssd'
> +hdd_target = '/path-to-raw-source-image-at-hdd'
> +nbd_ip = 'nbd-ip-addr'
> +nbd_port = 'nbd-port-number'
> +
> +# Test-cases are "rows" in benchmark resulting table, 'id' is a caption for
> +# the row, other fields are handled by bench_func.
> +test_cases = [
> +    {
> +        'id': 'ssd -> ssd',
> +        'source': drv_file(ssd_source),
> +        'target': drv_file(ssd_target)
> +    },
> +    {
> +        'id': 'ssd -> hdd',
> +        'source': drv_file(ssd_source),
> +        'target': drv_file(hdd_target)
> +    },
> +    {
> +        'id': 'ssd -> nbd',
> +        'source': drv_file(ssd_source),
> +        'target': drv_nbd(nbd_ip, nbd_port)
> +    },
> +]
> +
> +# Test-envs are "columns" in benchmark resulting table, 'id is a caption for
> +# the column, other fields are handled by bench_func.
> +test_envs = [
> +    {
> +        'id': 'backup-1',
> +        'cmd': 'blockdev-backup',
> +        'qemu_binary': '/path-to-qemu-binary-1'
> +    },
> +    {
> +        'id': 'backup-2',
> +        'cmd': 'blockdev-backup',
> +        'qemu_binary': '/path-to-qemu-binary-2'
> +    },
> +    {
> +        'id': 'mirror',
> +        'cmd': 'blockdev-mirror',
> +        'qemu_binary': '/path-to-qemu-binary-1'
> +    }
> +]
> +
> +result = simplebench.bench(bench_func, test_envs, test_cases, count=3)
> +print(simplebench.ascii(result))
> --
> 2.21.0
>
>


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

* Re: [PATCH v3 4/4] MAINTAINERS: add simplebench
  2020-02-28  7:19 ` [PATCH v3 4/4] MAINTAINERS: add simplebench Vladimir Sementsov-Ogievskiy
@ 2020-02-28 13:27   ` Aleksandar Markovic
  0 siblings, 0 replies; 13+ messages in thread
From: Aleksandar Markovic @ 2020-02-28 13:27 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Kevin Wolf, Eduardo Habkost, open list:bochs, Stefan Hajnoczi,
	QEMU Developers, Max Reitz, Cleber Rosa, Denis V. Lunev,
	John Snow

On Fri, Feb 28, 2020 at 8:19 AM Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

>  MAINTAINERS | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5e5e3e52d6..16d069adc5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2038,6 +2038,11 @@ F: python/qemu/*py
>  F: scripts/*.py
>  F: tests/*.py
>
> +Benchmark util
> +M: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> +S: Maintained
> +F: scripts/simplebench/
> +
>  QAPI
>  M: Markus Armbruster <armbru@redhat.com>
>  M: Michael Roth <mdroth@linux.vnet.ibm.com>
> --
> 2.21.0
>
>


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

* Re: [PATCH v3 1/4] scripts/simplebench: add simplebench.py
  2020-02-28 13:03   ` Aleksandar Markovic
@ 2020-02-28 13:48     ` Vladimir Sementsov-Ogievskiy
  2020-03-02 21:05       ` Aleksandar Markovic
  0 siblings, 1 reply; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-02-28 13:48 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: Kevin Wolf, Eduardo Habkost, open list:bochs, Stefan Hajnoczi,
	QEMU Developers, Max Reitz, Cleber Rosa, Denis V. Lunev,
	John Snow

28.02.2020 16:03, Aleksandar Markovic wrote:
> On Fri, Feb 28, 2020 at 8:19 AM Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com> wrote:
>>
>> Add simple benchmark table creator.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   scripts/simplebench/simplebench.py | 128 +++++++++++++++++++++++++++++
>>   1 file changed, 128 insertions(+)
>>   create mode 100644 scripts/simplebench/simplebench.py
>>
>> diff --git a/scripts/simplebench/simplebench.py b/scripts/simplebench/simplebench.py
>> new file mode 100644
>> index 0000000000..59e7314ff6
>> --- /dev/null
>> +++ b/scripts/simplebench/simplebench.py
>> @@ -0,0 +1,128 @@
>> +#!/usr/bin/env python
>> +#
>> +# Simple benchmarking framework
>> +#
>> +# Copyright (c) 2019 Virtuozzo International GmbH.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +
>> +
>> +def bench_one(test_func, test_env, test_case, count=5, initial_run=True):
>> +    """Benchmark one test-case
>> +
>> +    test_func   -- benchmarking function with prototype
>> +                   test_func(env, case), which takes test_env and test_case
>> +                   arguments and returns {'seconds': int} (which is benchmark
>> +                   result) on success and {'error': str} on error. Returned
>> +                   dict may contain any other additional fields.
>> +    test_env    -- test environment - opaque first argument for test_func
>> +    test_case   -- test case - opaque second argument for test_func
>> +    count       -- how many times to call test_func, to calculate average
>> +    initial_run -- do initial run of test_func, which don't get into result
>> +
>> +    Returns dict with the following fields:
>> +        'runs':     list of test_func results
>> +        'average':  average seconds per run (exists only if at least one run
>> +                    succeeded)
>> +        'delta':    maximum delta between test_func result and the average
>> +                    (exists only if at least one run succeeded)
>> +        'n-failed': number of failed runs (exists only if at least one run
>> +                    failed)
>> +    """
>> +    if initial_run:
>> +        print('  #initial run:')
>> +        print('   ', test_func(test_env, test_case))
>> +
>> +    runs = []
>> +    for i in range(count):
>> +        print('  #run {}'.format(i+1))
>> +        res = test_func(test_env, test_case)
>> +        print('   ', res)
>> +        runs.append(res)
>> +
>> +    result = {'runs': runs}
>> +
>> +    successed = [r for r in runs if ('seconds' in r)]
>> +    if successed:
>> +        avg = sum(r['seconds'] for r in successed) / len(successed)
>> +        result['average'] = avg
>> +        result['delta'] = max(abs(r['seconds'] - avg) for r in successed)
>> +
>> +    if len(successed) < count:
>> +        result['n-failed'] = count - len(successed)
>> +
>> +    return result
>> +
>> +
>> +def ascii_one(result):
>> +    """Return ASCII representation of bench_one() returned dict."""
>> +    if 'average' in result:
>> +        s = '{:.2f} +- {:.2f}'.format(result['average'], result['delta'])
>> +        if 'n-failed' in result:
>> +            s += '\n({} failed)'.format(result['n-failed'])
>> +        return s
>> +    else:
>> +        return 'FAILED'
> 
> I think it would be visually clearer if "+-" was printed without any
> space between it and the following number, using something
> like this:
> 
> s = ' {:.2f} +-{:.2f}'.format(result['average'], result['delta'])
> 
> The resulting table would look like:
> 
> ----------  -------------  -------------  -------------
>              backup-1       backup-2       mirror
> ssd -> ssd   0.43 +-0.00    4.48 +-0.06    4.38 +-0.02
> ssd -> hdd   10.60 +-0.08   10.69 +-0.18   10.57 +-0.05
> ssd -> nbd   33.81 +-0.37   10.67 +-0.17   10.07 +-0.07
> ----------  -------------  -------------  -------------
> 
> But, this is just cosmetics.
> 
> With or without the suggestion above:
> 
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

Thanks for reviewing!

Agree with this change, but I don't think it worth to resend the series for this one space)
Hope it may be applied with pull request..

> 
>> +
>> +
>> +def bench(test_func, test_envs, test_cases, *args, **vargs):
>> +    """Fill benchmark table
>> +
>> +    test_func -- benchmarking function, see bench_one for description
>> +    test_envs -- list of test environments, see bench_one
>> +    test_cases -- list of test cases, see bench_one
>> +    args, vargs -- additional arguments for bench_one
>> +
>> +    Returns dict with the following fields:
>> +        'envs':  test_envs
>> +        'cases': test_cases
>> +        'tab':   filled 2D array, where cell [i][j] is bench_one result for
>> +                 test_cases[i] for test_envs[j] (i.e., rows are test cases and
>> +                 columns are test environments)
>> +    """
>> +    tab = {}
>> +    results = {
>> +        'envs': test_envs,
>> +        'cases': test_cases,
>> +        'tab': tab
>> +    }
>> +    n = 1
>> +    n_tests = len(test_envs) * len(test_cases)
>> +    for env in test_envs:
>> +        for case in test_cases:
>> +            print('Testing {}/{}: {} :: {}'.format(n, n_tests,
>> +                                                   env['id'], case['id']))
>> +            if case['id'] not in tab:
>> +                tab[case['id']] = {}
>> +            tab[case['id']][env['id']] = bench_one(test_func, env, case,
>> +                                                   *args, **vargs)
>> +            n += 1
>> +
>> +    print('Done')
>> +    return results
>> +
>> +
>> +def ascii(results):
>> +    """Return ASCII representation of bench() returned dict."""
>> +    from tabulate import tabulate
>> +
>> +    tab = [[""] + [c['id'] for c in results['envs']]]
>> +    for case in results['cases']:
>> +        row = [case['id']]
>> +        for env in results['envs']:
>> +            row.append(ascii_one(results['tab'][case['id']][env['id']]))
>> +        tab.append(row)
>> +
>> +    return tabulate(tab)
>> --
>> 2.21.0
>>
>>


-- 
Best regards,
Vladimir


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

* Re: [PATCH v3 1/4] scripts/simplebench: add simplebench.py
  2020-02-28 13:48     ` Vladimir Sementsov-Ogievskiy
@ 2020-03-02 21:05       ` Aleksandar Markovic
  2020-03-17 14:40         ` Aleksandar Markovic
  0 siblings, 1 reply; 13+ messages in thread
From: Aleksandar Markovic @ 2020-03-02 21:05 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Kevin Wolf, Eduardo Habkost, open list:bochs, Stefan Hajnoczi,
	QEMU Developers, Max Reitz, Cleber Rosa, Denis V. Lunev,
	John Snow

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

> >> +
> >> +
> >> +def ascii_one(result):
> >> +    """Return ASCII representation of bench_one() returned dict."""
> >> +    if 'average' in result:
> >> +        s = '{:.2f} +- {:.2f}'.format(result['average'],
result['delta'])
> >> +        if 'n-failed' in result:
> >> +            s += '\n({} failed)'.format(result['n-failed'])
> >> +        return s
> >> +    else:
> >> +        return 'FAILED'
> >
> > I think it would be visually clearer if "+-" was printed without any
> > space between it and the following number, using something
> > like this:
> >
> > s = ' {:.2f} +-{:.2f}'.format(result['average'], result['delta'])
> >
> > The resulting table would look like:
> >
> > ----------  -------------  -------------  -------------
> >              backup-1       backup-2       mirror
> > ssd -> ssd   0.43 +-0.00    4.48 +-0.06    4.38 +-0.02
> > ssd -> hdd   10.60 +-0.08   10.69 +-0.18   10.57 +-0.05
> > ssd -> nbd   33.81 +-0.37   10.67 +-0.17   10.07 +-0.07
> > ----------  -------------  -------------  -------------
> >
> > But, this is just cosmetics.
> >
> > With or without the suggestion above:
> >
> > Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Thanks for reviewing!
>
> Agree with this change, but I don't think it worth to resend the series
for this one space)
> Hope it may be applied with pull request..
>

I am an occasional Python programmer, and I felt comfortable
reviewing your series, but I am not a maintainer of this directory,
and I believe Eduardo or Cleber or other more active Python
contributors would be better choice for selecting this series in
their pull request.

So, I can't send this series to Peter - Cleber, Eduardo, please
see to it.

Yours,
Aleksandar

> >
> >> +
> >> +
> >> +def bench(test_func, test_envs, test_cases, *args, **vargs):
> >> +    """Fill benchmark table
> >> +
> >> +    test_func -- benchmarking function, see bench_one for description
> >> +    test_envs -- list of test environments, see bench_one
> >> +    test_cases -- list of test cases, see bench_one
> >> +    args, vargs -- additional arguments for bench_one
> >> +
> >> +    Returns dict with the following fields:
> >> +        'envs':  test_envs
> >> +        'cases': test_cases
> >> +        'tab':   filled 2D array, where cell [i][j] is bench_one
result for
> >> +                 test_cases[i] for test_envs[j] (i.e., rows are test
cases and
> >> +                 columns are test environments)
> >> +    """
> >> +    tab = {}
> >> +    results = {
> >> +        'envs': test_envs,
> >> +        'cases': test_cases,
> >> +        'tab': tab
> >> +    }
> >> +    n = 1
> >> +    n_tests = len(test_envs) * len(test_cases)
> >> +    for env in test_envs:
> >> +        for case in test_cases:
> >> +            print('Testing {}/{}: {} :: {}'.format(n, n_tests,
> >> +                                                   env['id'],
case['id']))
> >> +            if case['id'] not in tab:
> >> +                tab[case['id']] = {}
> >> +            tab[case['id']][env['id']] = bench_one(test_func, env,
case,
> >> +                                                   *args, **vargs)
> >> +            n += 1
> >> +
> >> +    print('Done')
> >> +    return results
> >> +
> >> +
> >> +def ascii(results):
> >> +    """Return ASCII representation of bench() returned dict."""
> >> +    from tabulate import tabulate
> >> +
> >> +    tab = [[""] + [c['id'] for c in results['envs']]]
> >> +    for case in results['cases']:
> >> +        row = [case['id']]
> >> +        for env in results['envs']:
> >> +
 row.append(ascii_one(results['tab'][case['id']][env['id']]))
> >> +        tab.append(row)
> >> +
> >> +    return tabulate(tab)
> >> --
> >> 2.21.0
> >>
> >>
>
>
> --
> Best regards,
> Vladimir

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

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

* Re: [PATCH v3 1/4] scripts/simplebench: add simplebench.py
  2020-03-02 21:05       ` Aleksandar Markovic
@ 2020-03-17 14:40         ` Aleksandar Markovic
  2020-03-17 15:01           ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 13+ messages in thread
From: Aleksandar Markovic @ 2020-03-17 14:40 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Kevin Wolf, Eduardo Habkost, open list:bochs, Stefan Hajnoczi,
	QEMU Developers, Max Reitz, Cleber Rosa, Denis V. Lunev,
	John Snow

On Mon, Mar 2, 2020 at 10:05 PM Aleksandar Markovic
<aleksandar.m.mail@gmail.com> wrote:
>
>
>
>
> > >> +
> > >> +
> > >> +def ascii_one(result):
> > >> +    """Return ASCII representation of bench_one() returned dict."""
> > >> +    if 'average' in result:
> > >> +        s = '{:.2f} +- {:.2f}'.format(result['average'], result['delta'])
> > >> +        if 'n-failed' in result:
> > >> +            s += '\n({} failed)'.format(result['n-failed'])
> > >> +        return s
> > >> +    else:
> > >> +        return 'FAILED'
> > >
> > > I think it would be visually clearer if "+-" was printed without any
> > > space between it and the following number, using something
> > > like this:
> > >
> > > s = ' {:.2f} +-{:.2f}'.format(result['average'], result['delta'])
> > >
> > > The resulting table would look like:
> > >
> > > ----------  -------------  -------------  -------------
> > >              backup-1       backup-2       mirror
> > > ssd -> ssd   0.43 +-0.00    4.48 +-0.06    4.38 +-0.02
> > > ssd -> hdd   10.60 +-0.08   10.69 +-0.18   10.57 +-0.05
> > > ssd -> nbd   33.81 +-0.37   10.67 +-0.17   10.07 +-0.07
> > > ----------  -------------  -------------  -------------
> > >
> > > But, this is just cosmetics.
> > >
> > > With or without the suggestion above:
> > >
> > > Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> >
> > Thanks for reviewing!
> >
> > Agree with this change, but I don't think it worth to resend the series for this one space)
> > Hope it may be applied with pull request..
> >
>
> I am an occasional Python programmer, and I felt comfortable
> reviewing your series, but I am not a maintainer of this directory,
> and I believe Eduardo or Cleber or other more active Python
> contributors would be better choice for selecting this series in
> their pull request.
>
> So, I can't send this series to Peter - Cleber, Eduardo, please
> see to it.
>

Eduardo, can you perhaps consider this series for selecting
into your pull request?

Thanks,
Aleksandar

> Yours,
> Aleksandar
>
> > >
> > >> +
> > >> +
> > >> +def bench(test_func, test_envs, test_cases, *args, **vargs):
> > >> +    """Fill benchmark table
> > >> +
> > >> +    test_func -- benchmarking function, see bench_one for description
> > >> +    test_envs -- list of test environments, see bench_one
> > >> +    test_cases -- list of test cases, see bench_one
> > >> +    args, vargs -- additional arguments for bench_one
> > >> +
> > >> +    Returns dict with the following fields:
> > >> +        'envs':  test_envs
> > >> +        'cases': test_cases
> > >> +        'tab':   filled 2D array, where cell [i][j] is bench_one result for
> > >> +                 test_cases[i] for test_envs[j] (i.e., rows are test cases and
> > >> +                 columns are test environments)
> > >> +    """
> > >> +    tab = {}
> > >> +    results = {
> > >> +        'envs': test_envs,
> > >> +        'cases': test_cases,
> > >> +        'tab': tab
> > >> +    }
> > >> +    n = 1
> > >> +    n_tests = len(test_envs) * len(test_cases)
> > >> +    for env in test_envs:
> > >> +        for case in test_cases:
> > >> +            print('Testing {}/{}: {} :: {}'.format(n, n_tests,
> > >> +                                                   env['id'], case['id']))
> > >> +            if case['id'] not in tab:
> > >> +                tab[case['id']] = {}
> > >> +            tab[case['id']][env['id']] = bench_one(test_func, env, case,
> > >> +                                                   *args, **vargs)
> > >> +            n += 1
> > >> +
> > >> +    print('Done')
> > >> +    return results
> > >> +
> > >> +
> > >> +def ascii(results):
> > >> +    """Return ASCII representation of bench() returned dict."""
> > >> +    from tabulate import tabulate
> > >> +
> > >> +    tab = [[""] + [c['id'] for c in results['envs']]]
> > >> +    for case in results['cases']:
> > >> +        row = [case['id']]
> > >> +        for env in results['envs']:
> > >> +            row.append(ascii_one(results['tab'][case['id']][env['id']]))
> > >> +        tab.append(row)
> > >> +
> > >> +    return tabulate(tab)
> > >> --
> > >> 2.21.0
> > >>
> > >>
> >
> >
> > --
> > Best regards,
> > Vladimir


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

* Re: [PATCH v3 1/4] scripts/simplebench: add simplebench.py
  2020-03-17 14:40         ` Aleksandar Markovic
@ 2020-03-17 15:01           ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 13+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-03-17 15:01 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: Kevin Wolf, Eduardo Habkost, open list:bochs, Stefan Hajnoczi,
	QEMU Developers, Max Reitz, Cleber Rosa, Denis V. Lunev,
	John Snow

17.03.2020 17:40, Aleksandar Markovic wrote:
> On Mon, Mar 2, 2020 at 10:05 PM Aleksandar Markovic
> <aleksandar.m.mail@gmail.com> wrote:
>>
>>
>>
>>
>>>>> +
>>>>> +
>>>>> +def ascii_one(result):
>>>>> +    """Return ASCII representation of bench_one() returned dict."""
>>>>> +    if 'average' in result:
>>>>> +        s = '{:.2f} +- {:.2f}'.format(result['average'], result['delta'])
>>>>> +        if 'n-failed' in result:
>>>>> +            s += '\n({} failed)'.format(result['n-failed'])
>>>>> +        return s
>>>>> +    else:
>>>>> +        return 'FAILED'
>>>>
>>>> I think it would be visually clearer if "+-" was printed without any
>>>> space between it and the following number, using something
>>>> like this:
>>>>
>>>> s = ' {:.2f} +-{:.2f}'.format(result['average'], result['delta'])
>>>>
>>>> The resulting table would look like:
>>>>
>>>> ----------  -------------  -------------  -------------
>>>>               backup-1       backup-2       mirror
>>>> ssd -> ssd   0.43 +-0.00    4.48 +-0.06    4.38 +-0.02
>>>> ssd -> hdd   10.60 +-0.08   10.69 +-0.18   10.57 +-0.05
>>>> ssd -> nbd   33.81 +-0.37   10.67 +-0.17   10.07 +-0.07
>>>> ----------  -------------  -------------  -------------
>>>>
>>>> But, this is just cosmetics.
>>>>
>>>> With or without the suggestion above:
>>>>
>>>> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
>>>
>>> Thanks for reviewing!
>>>
>>> Agree with this change, but I don't think it worth to resend the series for this one space)
>>> Hope it may be applied with pull request..
>>>
>>
>> I am an occasional Python programmer, and I felt comfortable
>> reviewing your series, but I am not a maintainer of this directory,
>> and I believe Eduardo or Cleber or other more active Python
>> contributors would be better choice for selecting this series in
>> their pull request.
>>
>> So, I can't send this series to Peter - Cleber, Eduardo, please
>> see to it.
>>
> 
> Eduardo, can you perhaps consider this series for selecting
> into your pull request?

I saw, he has taken the patches into python-next in:
https://github.com/ehabkost/qemu/commits/python-next

Hope, pull request will come)


> 
> Thanks,
> Aleksandar
> 
>> Yours,
>> Aleksandar
>>
>>>>
>>>>> +
>>>>> +
>>>>> +def bench(test_func, test_envs, test_cases, *args, **vargs):
>>>>> +    """Fill benchmark table
>>>>> +
>>>>> +    test_func -- benchmarking function, see bench_one for description
>>>>> +    test_envs -- list of test environments, see bench_one
>>>>> +    test_cases -- list of test cases, see bench_one
>>>>> +    args, vargs -- additional arguments for bench_one
>>>>> +
>>>>> +    Returns dict with the following fields:
>>>>> +        'envs':  test_envs
>>>>> +        'cases': test_cases
>>>>> +        'tab':   filled 2D array, where cell [i][j] is bench_one result for
>>>>> +                 test_cases[i] for test_envs[j] (i.e., rows are test cases and
>>>>> +                 columns are test environments)
>>>>> +    """
>>>>> +    tab = {}
>>>>> +    results = {
>>>>> +        'envs': test_envs,
>>>>> +        'cases': test_cases,
>>>>> +        'tab': tab
>>>>> +    }
>>>>> +    n = 1
>>>>> +    n_tests = len(test_envs) * len(test_cases)
>>>>> +    for env in test_envs:
>>>>> +        for case in test_cases:
>>>>> +            print('Testing {}/{}: {} :: {}'.format(n, n_tests,
>>>>> +                                                   env['id'], case['id']))
>>>>> +            if case['id'] not in tab:
>>>>> +                tab[case['id']] = {}
>>>>> +            tab[case['id']][env['id']] = bench_one(test_func, env, case,
>>>>> +                                                   *args, **vargs)
>>>>> +            n += 1
>>>>> +
>>>>> +    print('Done')
>>>>> +    return results
>>>>> +
>>>>> +
>>>>> +def ascii(results):
>>>>> +    """Return ASCII representation of bench() returned dict."""
>>>>> +    from tabulate import tabulate
>>>>> +
>>>>> +    tab = [[""] + [c['id'] for c in results['envs']]]
>>>>> +    for case in results['cases']:
>>>>> +        row = [case['id']]
>>>>> +        for env in results['envs']:
>>>>> +            row.append(ascii_one(results['tab'][case['id']][env['id']]))
>>>>> +        tab.append(row)
>>>>> +
>>>>> +    return tabulate(tab)
>>>>> --
>>>>> 2.21.0
>>>>>
>>>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Vladimir


-- 
Best regards,
Vladimir


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

end of thread, other threads:[~2020-03-17 15:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28  7:19 [PATCH v3 0/4] benchmark util Vladimir Sementsov-Ogievskiy
2020-02-28  7:19 ` [PATCH v3 1/4] scripts/simplebench: add simplebench.py Vladimir Sementsov-Ogievskiy
2020-02-28 13:03   ` Aleksandar Markovic
2020-02-28 13:48     ` Vladimir Sementsov-Ogievskiy
2020-03-02 21:05       ` Aleksandar Markovic
2020-03-17 14:40         ` Aleksandar Markovic
2020-03-17 15:01           ` Vladimir Sementsov-Ogievskiy
2020-02-28  7:19 ` [PATCH v3 2/4] scripts/simplebench: add qemu/bench_block_job.py Vladimir Sementsov-Ogievskiy
2020-02-28 13:04   ` Aleksandar Markovic
2020-02-28  7:19 ` [PATCH v3 3/4] scripts/simplebench: add example usage of simplebench Vladimir Sementsov-Ogievskiy
2020-02-28 13:27   ` Aleksandar Markovic
2020-02-28  7:19 ` [PATCH v3 4/4] MAINTAINERS: add simplebench Vladimir Sementsov-Ogievskiy
2020-02-28 13:27   ` Aleksandar Markovic

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.