qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions
@ 2020-06-16 23:12 Ahmed Karaman
  2020-06-16 23:12 ` [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section Ahmed Karaman
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Ahmed Karaman @ 2020-06-16 23:12 UTC (permalink / raw)
  To: qemu-devel, rth, aleksandar.qemu.devel, alex.bennee, eblake,
	ldoktor, ehabkost, crosa
  Cc: Ahmed Karaman

Greetings,

As a part of the TCG Continous Benchmarking project for GSoC this
year, detailed reports discussing different performance measurement
methodologies and analysis results will be sent here on the mailing
list.

The project's first report is currently being revised and will be
posted on the mailing list in the next few days.
A section in this report will deal with measuring the top 25 executed
functions when running QEMU. It includes two Python scripts that
automatically perform this task.

This series adds these two scripts to a new performance directory
created under the scripts directory. It also adds a new
"Miscellaneous" section to the end of the MAINTAINERS file with a
"Performance Tools and Tests" subsection.

Best regards,
Ahmed Karaman

Ahmed Karaman (3):
  MAINTAINERS: Add 'Miscellaneous' section
  scripts/performance: Add callgrind_top_25.py script
  scripts/performance: Add perf_top_25.py script

 MAINTAINERS                             |  7 ++
 scripts/performance/callgrind_top_25.py | 95 +++++++++++++++++++++++++
 scripts/performance/perf_top_25.py      | 82 +++++++++++++++++++++
 3 files changed, 184 insertions(+)
 create mode 100644 scripts/performance/callgrind_top_25.py
 create mode 100644 scripts/performance/perf_top_25.py

-- 
2.17.1



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

* [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section
  2020-06-16 23:12 [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions Ahmed Karaman
@ 2020-06-16 23:12 ` Ahmed Karaman
  2020-06-17  5:35   ` Aleksandar Markovic
                     ` (2 more replies)
  2020-06-16 23:12 ` [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script Ahmed Karaman
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 22+ messages in thread
From: Ahmed Karaman @ 2020-06-16 23:12 UTC (permalink / raw)
  To: qemu-devel, rth, aleksandar.qemu.devel, alex.bennee, eblake,
	ldoktor, ehabkost, crosa
  Cc: Ahmed Karaman

This new section includes the 'Performance Tools and Tests' subsection
which will contain the the performance scripts and benchmarks written
as a part of the 'TCG Continuous Benchmarking' project.

Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 955cc8dd5c..68f668ae2a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2974,3 +2974,10 @@ M: Peter Maydell <peter.maydell@linaro.org>
 S: Maintained
 F: docs/conf.py
 F: docs/*/conf.py
+
+Miscellaneous
+-------------
+Performance Tools and Tests
+M: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
+S: Maintained
+F: scripts/performance/
\ No newline at end of file
-- 
2.17.1



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

* [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script
  2020-06-16 23:12 [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions Ahmed Karaman
  2020-06-16 23:12 ` [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section Ahmed Karaman
@ 2020-06-16 23:12 ` Ahmed Karaman
  2020-06-17  5:42   ` Aleksandar Markovic
  2020-06-17 12:16   ` Alex Bennée
  2020-06-16 23:12 ` [PATCH 3/3] scripts/performance: Add perf_top_25.py script Ahmed Karaman
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 22+ messages in thread
From: Ahmed Karaman @ 2020-06-16 23:12 UTC (permalink / raw)
  To: qemu-devel, rth, aleksandar.qemu.devel, alex.bennee, eblake,
	ldoktor, ehabkost, crosa
  Cc: Ahmed Karaman

Python script that prints the top 25 most executed functions in QEMU
using callgrind.

Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
---
 scripts/performance/callgrind_top_25.py | 95 +++++++++++++++++++++++++
 1 file changed, 95 insertions(+)
 create mode 100644 scripts/performance/callgrind_top_25.py

diff --git a/scripts/performance/callgrind_top_25.py b/scripts/performance/callgrind_top_25.py
new file mode 100644
index 0000000000..03b089a96d
--- /dev/null
+++ b/scripts/performance/callgrind_top_25.py
@@ -0,0 +1,95 @@
+#!/usr/bin/env python3
+
+#  Print the top 25 most executed functions in QEMU using callgrind.
+#  Example Usage:
+#  callgrind_top_25.py <qemu-build>/x86_64-linux-user/qemu-x86_64 executable
+#
+#  This file is a part of the project "TCG Continuous Benchmarking".
+#
+#  Copyright (C) 2020  Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
+#  Copyright (C) 2020  Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
+#
+#  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 <https://www.gnu.org/licenses/>.
+
+import os
+import sys
+
+# Ensure sufficient arguments
+if len(sys.argv) < 3:
+    print('Insufficient Script Arguments!')
+    sys.exit(1)
+
+# Get the qemu path and the executable + its arguments
+(qemu, executable) = (sys.argv[1], ' '.join(sys.argv[2:]))
+
+# Run callgrind and callgrind_annotate
+os.system('valgrind --tool=callgrind --callgrind-out-file=callgrind.data {} {} \
+            2 > / dev / null & & callgrind_annotate callgrind.data \
+            > tmp.callgrind.data'.
+          format(qemu, executable))
+
+# Line number with the total number of instructions
+number_of_instructions_line = 20
+
+# Line number with the top function
+first_func_line = 25
+
+# callgrind_annotate output
+callgrind_data = []
+
+# Open callgrind_annotate output and store it in callgrind_data
+with open('tmp.callgrind.data', 'r') as data:
+    callgrind_data = data.readlines()
+
+# Get the total number of instructions
+total_number_of_instructions = int(
+    callgrind_data[number_of_instructions_line].split(' ')[0].replace(',', ''))
+
+# Number of functions recorded by callgrind
+number_of_functions = len(callgrind_data) - first_func_line
+
+# Limit the number of top functions to 25
+number_of_top_functions = (25 if number_of_functions >
+                           25 else number_of_instructions_line)
+
+# Store the data of the top functions in top_functions[]
+top_functions = callgrind_data[first_func_line:
+                               first_func_line + number_of_top_functions]
+# Print information headers
+print('{:>4}  {:>10}  {:<25}  {}\n{}  {}  {}  {}'.format('No.',
+                                                         'Percentage',
+                                                         'Name',
+                                                         'Source File',
+                                                         '-' * 4,
+                                                         '-' * 10,
+                                                         '-' * 25,
+                                                         '-' * 30,
+                                                         ))
+
+# Print top 25 functions
+for (index, function) in enumerate(top_functions, start=1):
+    function_data = function.split()
+    # Calculate function percentage
+    percentage = (float(function_data[0].replace(
+        ',', '')) / total_number_of_instructions) * 100
+    # Get function source path and name
+    path, name = function_data[1].split(':')
+    # Print extracted data
+    print('{:>4}  {:>9.3f}%  {:<25}  {}'.format(index,
+                                                round(percentage, 3),
+                                                name,
+                                                path))
+
+# Remove intermediate files
+os.system('rm callgrind.data tmp.callgrind.data')
-- 
2.17.1



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

* [PATCH 3/3] scripts/performance: Add perf_top_25.py script
  2020-06-16 23:12 [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions Ahmed Karaman
  2020-06-16 23:12 ` [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section Ahmed Karaman
  2020-06-16 23:12 ` [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script Ahmed Karaman
@ 2020-06-16 23:12 ` Ahmed Karaman
  2020-06-17  5:56   ` Aleksandar Markovic
  2020-06-17 12:21   ` Alex Bennée
  2020-06-16 23:35 ` [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions no-reply
  2020-06-17 13:53 ` Eric Blake
  4 siblings, 2 replies; 22+ messages in thread
From: Ahmed Karaman @ 2020-06-16 23:12 UTC (permalink / raw)
  To: qemu-devel, rth, aleksandar.qemu.devel, alex.bennee, eblake,
	ldoktor, ehabkost, crosa
  Cc: Ahmed Karaman

Python script that prints the top 25 most executed functions in QEMU
using perf.

Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
---
 scripts/performance/perf_top_25.py | 82 ++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 scripts/performance/perf_top_25.py

diff --git a/scripts/performance/perf_top_25.py b/scripts/performance/perf_top_25.py
new file mode 100644
index 0000000000..eaa8cce3c3
--- /dev/null
+++ b/scripts/performance/perf_top_25.py
@@ -0,0 +1,82 @@
+#!/usr/bin/env python3
+
+#  Print the top 25 most executed functions in QEMU using perf.
+#  Example Usage:
+#  perf_top_25.py <qemu-build>/x86_64-linux-user/qemu-x86_64 executable
+#
+#  This file is a part of the project "TCG Continuous Benchmarking".
+#
+#  Copyright (C) 2020  Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
+#  Copyright (C) 2020  Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
+#
+#  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 <https://www.gnu.org/licenses/>.
+
+import os
+import sys
+
+# Ensure sufficient arguments
+if len(sys.argv) < 3:
+    print('Insufficient Script Arguments!')
+    sys.exit(1)
+
+# Get the qemu path and the executable + its arguments
+(qemu_path, executable) = (sys.argv[1], ' '.join(sys.argv[2:]))
+
+# Run perf repcord and report
+os.system('sudo perf record {} {} 2> /dev/null \
+            && sudo perf report --stdio > tmp.perf.data'
+          .format(qemu_path, executable))
+
+# Line number with the top function
+first_func_line = 11
+
+# Perf report output
+perf_data = []
+
+# Open perf report output and store it in perf_data
+with open('tmp.perf.data', 'r') as data:
+    perf_data = data.readlines()
+
+# Number of functions recorded by perf
+number_of_functions = len(perf_data) - first_func_line
+
+# Limit the number of top functions to 25
+number_of_top_functions = (25 if number_of_functions >
+                           25 else number_of_functions)
+
+# Store the data of the top functions in top_functions[]
+top_functions = perf_data[first_func_line:first_func_line
+                          + number_of_top_functions]
+
+# Print information headers
+print('{:>4}  {:>10}  {:<25}  {}\n{}  {}  {}  {}'.format('No.',
+                                                         'Percentage',
+                                                         'Name',
+                                                         'Caller',
+                                                         '-' * 4,
+                                                         '-' * 10,
+                                                         '-' * 25,
+                                                         '-' * 25,
+                                                         ))
+
+# Print top 25 functions
+for (index, function) in enumerate(top_functions, start=1):
+    function_data = function.split()
+    print('{:>4}  {:>10}  {:<25}  {}'.format(index,
+                                             function_data[0],
+                                             function_data[-1],
+                                             ' '.join(function_data[2:-2])))
+
+# Remove intermediate files
+os.system('sudo rm perf.data tmp.perf.data')
-- 
2.17.1



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

* Re: [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions
  2020-06-16 23:12 [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions Ahmed Karaman
                   ` (2 preceding siblings ...)
  2020-06-16 23:12 ` [PATCH 3/3] scripts/performance: Add perf_top_25.py script Ahmed Karaman
@ 2020-06-16 23:35 ` no-reply
  2020-06-17 13:53 ` Eric Blake
  4 siblings, 0 replies; 22+ messages in thread
From: no-reply @ 2020-06-16 23:35 UTC (permalink / raw)
  To: ahmedkhaledkaraman
  Cc: ldoktor, ehabkost, alex.bennee, qemu-devel, ahmedkhaledkaraman,
	aleksandar.qemu.devel, crosa, rth

Patchew URL: https://patchew.org/QEMU/20200616231204.8850-1-ahmedkhaledkaraman@gmail.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  GEN     docs/interop/qemu-qmp-ref.html
  GEN     docs/interop/qemu-qmp-ref.txt
  GEN     docs/interop/qemu-qmp-ref.7
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  CC      qga/commands.o
  CC      qga/guest-agent-command-state.o
  CC      qga/main.o
---
  GEN     docs/interop/qemu-ga-ref.html
  GEN     docs/interop/qemu-ga-ref.txt
  GEN     docs/interop/qemu-ga-ref.7
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-ga
  LINK    qemu-keymap
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    ivshmem-client
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    ivshmem-server
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-nbd
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-storage-daemon
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  AS      pc-bios/optionrom/multiboot.o
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  AS      pc-bios/optionrom/linuxboot.o
  CC      pc-bios/optionrom/linuxboot_dma.o
  AS      pc-bios/optionrom/kvmvapic.o
---
  BUILD   pc-bios/optionrom/kvmvapic.raw
  BUILD   pc-bios/optionrom/pvh.raw
  SIGN    pc-bios/optionrom/multiboot.bin
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  SIGN    pc-bios/optionrom/linuxboot.bin
  SIGN    pc-bios/optionrom/linuxboot_dma.bin
  SIGN    pc-bios/optionrom/kvmvapic.bin
  SIGN    pc-bios/optionrom/pvh.bin
  LINK    qemu-edid
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    fsdev/virtfs-proxy-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    scsi/qemu-pr-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-bridge-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    virtiofsd
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    vhost-user-input
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  GEN     x86_64-softmmu/hmp-commands.h
  GEN     x86_64-softmmu/hmp-commands-info.h
  GEN     x86_64-softmmu/config-devices.h
---
  CC      x86_64-softmmu/accel/tcg/tcg-all.o
  CC      x86_64-softmmu/accel/tcg/cputlb.o
  CC      x86_64-softmmu/accel/tcg/tcg-runtime.o
/tmp/qemu-test/src/fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
    absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !
/tmp/qemu-test/src/fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
        absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 !
/tmp/qemu-test/src/fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
        absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 !
/tmp/qemu-test/src/fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
    zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !
/tmp/qemu-test/src/fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
    zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !
/tmp/qemu-test/src/fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
                    ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                    !
/tmp/qemu-test/src/fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
            zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                     !
/tmp/qemu-test/src/fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
        zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 !
8 errors generated.
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: fpu/softfloat.o] Error 1
make[1]: *** Waiting for unfinished jobs....
/tmp/qemu-test/src/migration/ram.c:919:45: error: implicit conversion from 'unsigned long' to 'double' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion]
            xbzrle_counters.encoding_rate = UINT64_MAX;
                                          ~ ^~~~~~~~~~
/usr/include/stdint.h:130:23: note: expanded from macro 'UINT64_MAX'
---
18446744073709551615UL
^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: migration/ram.o] Error 1
make: *** [Makefile:527: x86_64-softmmu/all] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 669, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=b86d77e6bea74d3faa899c5f3e7fd396', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-d5nmmrdj/src/docker-src.2020-06-16-19.31.41.20160:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=b86d77e6bea74d3faa899c5f3e7fd396
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-d5nmmrdj/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    3m50.170s
user    0m7.675s


The full log is available at
http://patchew.org/logs/20200616231204.8850-1-ahmedkhaledkaraman@gmail.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section
  2020-06-16 23:12 ` [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section Ahmed Karaman
@ 2020-06-17  5:35   ` Aleksandar Markovic
  2020-06-17  5:51   ` Aleksandar Markovic
  2020-06-17 12:01   ` Alex Bennée
  2 siblings, 0 replies; 22+ messages in thread
From: Aleksandar Markovic @ 2020-06-17  5:35 UTC (permalink / raw)
  To: Ahmed Karaman; +Cc: ldoktor, ehabkost, alex.bennee, qemu-devel, crosa, rth

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

среда, 17. јун 2020., Ahmed Karaman <ahmedkhaledkaraman@gmail.com> је
написао/ла:

> This new section includes the 'Performance Tools and Tests' subsection
> which will contain the the performance scripts and benchmarks written
> as a part of the 'TCG Continuous Benchmarking' project.
>
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 955cc8dd5c..68f668ae2a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2974,3 +2974,10 @@ M: Peter Maydell <peter.maydell@linaro.org>
>  S: Maintained
>  F: docs/conf.py
>  F: docs/*/conf.py
> +
> +Miscellaneous
> +-------------
> +Performance Tools and Tests
> +M: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> +S: Maintained
> +F: scripts/performance/
> \ No newline at end of file
> --


I don't think that it makes any functional difference in this case, but it
is customary that the last line of any source file contains end-of-line
characters, so please add that in the next version of the series.

Also, I think it is more logical that this patch is the last patch in the
series - if left the way you proposed, this patch would refer to the
directory that does not exist - yet.

Otherwise, it looks good to me.

Section "Miscellaneous" makes sense to me.

Thanks,
Aleksandar



> 2.17.1
>
>

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

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

* Re: [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script
  2020-06-16 23:12 ` [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script Ahmed Karaman
@ 2020-06-17  5:42   ` Aleksandar Markovic
  2020-06-17 12:16   ` Alex Bennée
  1 sibling, 0 replies; 22+ messages in thread
From: Aleksandar Markovic @ 2020-06-17  5:42 UTC (permalink / raw)
  To: Ahmed Karaman; +Cc: ldoktor, ehabkost, alex.bennee, qemu-devel, crosa, rth

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

среда, 17. јун 2020., Ahmed Karaman <ahmedkhaledkaraman@gmail.com> је
написао/ла:

> Python script that prints the top 25 most executed functions in QEMU
> using callgrind.
>
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> ---


I think you should add an example of script usage in the commit message
(even though you mention such example in the comments of the script),
together with the script output for that example.

Thanks,
Aleksandar



>  scripts/performance/callgrind_top_25.py | 95 +++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
>  create mode 100644 scripts/performance/callgrind_top_25.py
>
> diff --git a/scripts/performance/callgrind_top_25.py
> b/scripts/performance/callgrind_top_25.py
> new file mode 100644
> index 0000000000..03b089a96d
> --- /dev/null
> +++ b/scripts/performance/callgrind_top_25.py
> @@ -0,0 +1,95 @@
> +#!/usr/bin/env python3
> +
> +#  Print the top 25 most executed functions in QEMU using callgrind.
> +#  Example Usage:
> +#  callgrind_top_25.py <qemu-build>/x86_64-linux-user/qemu-x86_64
> executable
> +#
> +#  This file is a part of the project "TCG Continuous Benchmarking".
> +#
> +#  Copyright (C) 2020  Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> +#  Copyright (C) 2020  Aleksandar Markovic <aleksandar.qemu.devel@gmail.
> com>
> +#
> +#  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 <https://www.gnu.org/licenses/>.
> +
> +import os
> +import sys
> +
> +# Ensure sufficient arguments
> +if len(sys.argv) < 3:
> +    print('Insufficient Script Arguments!')
> +    sys.exit(1)
> +
> +# Get the qemu path and the executable + its arguments
> +(qemu, executable) = (sys.argv[1], ' '.join(sys.argv[2:]))
> +
> +# Run callgrind and callgrind_annotate
> +os.system('valgrind --tool=callgrind --callgrind-out-file=callgrind.data
> {} {} \
> +            2 > / dev / null & & callgrind_annotate callgrind.data \
> +            > tmp.callgrind.data'.
> +          format(qemu, executable))
> +
> +# Line number with the total number of instructions
> +number_of_instructions_line = 20
> +
> +# Line number with the top function
> +first_func_line = 25
> +
> +# callgrind_annotate output
> +callgrind_data = []
> +
> +# Open callgrind_annotate output and store it in callgrind_data
> +with open('tmp.callgrind.data', 'r') as data:
> +    callgrind_data = data.readlines()
> +
> +# Get the total number of instructions
> +total_number_of_instructions = int(
> +    callgrind_data[number_of_instructions_line].split('
> ')[0].replace(',', ''))
> +
> +# Number of functions recorded by callgrind
> +number_of_functions = len(callgrind_data) - first_func_line
> +
> +# Limit the number of top functions to 25
> +number_of_top_functions = (25 if number_of_functions >
> +                           25 else number_of_instructions_line)
> +
> +# Store the data of the top functions in top_functions[]
> +top_functions = callgrind_data[first_func_line:
> +                               first_func_line + number_of_top_functions]
> +# Print information headers
> +print('{:>4}  {:>10}  {:<25}  {}\n{}  {}  {}  {}'.format('No.',
> +                                                         'Percentage',
> +                                                         'Name',
> +                                                         'Source File',
> +                                                         '-' * 4,
> +                                                         '-' * 10,
> +                                                         '-' * 25,
> +                                                         '-' * 30,
> +                                                         ))
> +
> +# Print top 25 functions
> +for (index, function) in enumerate(top_functions, start=1):
> +    function_data = function.split()
> +    # Calculate function percentage
> +    percentage = (float(function_data[0].replace(
> +        ',', '')) / total_number_of_instructions) * 100
> +    # Get function source path and name
> +    path, name = function_data[1].split(':')
> +    # Print extracted data
> +    print('{:>4}  {:>9.3f}%  {:<25}  {}'.format(index,
> +                                                round(percentage, 3),
> +                                                name,
> +                                                path))
> +
> +# Remove intermediate files
> +os.system('rm callgrind.data tmp.callgrind.data')
> --
> 2.17.1
>
>

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

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

* Re: [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section
  2020-06-16 23:12 ` [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section Ahmed Karaman
  2020-06-17  5:35   ` Aleksandar Markovic
@ 2020-06-17  5:51   ` Aleksandar Markovic
  2020-06-17 12:01   ` Alex Bennée
  2 siblings, 0 replies; 22+ messages in thread
From: Aleksandar Markovic @ 2020-06-17  5:51 UTC (permalink / raw)
  To: Ahmed Karaman; +Cc: ldoktor, ehabkost, alex.bennee, qemu-devel, crosa, rth

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

среда, 17. јун 2020., Ahmed Karaman <ahmedkhaledkaraman@gmail.com> је
написао/ла:

> This new section includes the 'Performance Tools and Tests' subsection
> which will contain the the performance scripts and benchmarks written
> as a part of the 'TCG Continuous Benchmarking' project.
>
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> ---


I also think that the title of the patch should be 'Add 'Performance Tools
and Tests' subsection', since this is the core purpose of the patch - new
section 'Miscellaneous' is just a mean to achieve this.

Thanks,
Aleksandar




>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 955cc8dd5c..68f668ae2a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2974,3 +2974,10 @@ M: Peter Maydell <peter.maydell@linaro.org>
>  S: Maintained
>  F: docs/conf.py
>  F: docs/*/conf.py
> +
> +Miscellaneous
> +-------------
> +Performance Tools and Tests
> +M: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> +S: Maintained
> +F: scripts/performance/
> \ No newline at end of file
> --
> 2.17.1
>
>

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

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

* Re: [PATCH 3/3] scripts/performance: Add perf_top_25.py script
  2020-06-16 23:12 ` [PATCH 3/3] scripts/performance: Add perf_top_25.py script Ahmed Karaman
@ 2020-06-17  5:56   ` Aleksandar Markovic
  2020-06-17 12:21   ` Alex Bennée
  1 sibling, 0 replies; 22+ messages in thread
From: Aleksandar Markovic @ 2020-06-17  5:56 UTC (permalink / raw)
  To: Ahmed Karaman; +Cc: ldoktor, ehabkost, alex.bennee, qemu-devel, crosa, rth

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

среда, 17. јун 2020., Ahmed Karaman <ahmedkhaledkaraman@gmail.com> је
написао/ла:

> Python script that prints the top 25 most executed functions in QEMU
> using perf.
>
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> ---


The same comment as I made for patch 2, apply here too.

Thanks,
Aleksandar



>  scripts/performance/perf_top_25.py | 82 ++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 scripts/performance/perf_top_25.py
>
> diff --git a/scripts/performance/perf_top_25.py
> b/scripts/performance/perf_top_25.py
> new file mode 100644
> index 0000000000..eaa8cce3c3
> --- /dev/null
> +++ b/scripts/performance/perf_top_25.py
> @@ -0,0 +1,82 @@
> +#!/usr/bin/env python3
> +
> +#  Print the top 25 most executed functions in QEMU using perf.
> +#  Example Usage:
> +#  perf_top_25.py <qemu-build>/x86_64-linux-user/qemu-x86_64 executable
> +#
> +#  This file is a part of the project "TCG Continuous Benchmarking".
> +#
> +#  Copyright (C) 2020  Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> +#  Copyright (C) 2020  Aleksandar Markovic <aleksandar.qemu.devel@gmail.
> com>
> +#
> +#  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 <https://www.gnu.org/licenses/>.
> +
> +import os
> +import sys
> +
> +# Ensure sufficient arguments
> +if len(sys.argv) < 3:
> +    print('Insufficient Script Arguments!')
> +    sys.exit(1)
> +
> +# Get the qemu path and the executable + its arguments
> +(qemu_path, executable) = (sys.argv[1], ' '.join(sys.argv[2:]))
> +
> +# Run perf repcord and report
> +os.system('sudo perf record {} {} 2> /dev/null \
> +            && sudo perf report --stdio > tmp.perf.data'
> +          .format(qemu_path, executable))
> +
> +# Line number with the top function
> +first_func_line = 11
> +
> +# Perf report output
> +perf_data = []
> +
> +# Open perf report output and store it in perf_data
> +with open('tmp.perf.data', 'r') as data:
> +    perf_data = data.readlines()
> +
> +# Number of functions recorded by perf
> +number_of_functions = len(perf_data) - first_func_line
> +
> +# Limit the number of top functions to 25
> +number_of_top_functions = (25 if number_of_functions >
> +                           25 else number_of_functions)
> +
> +# Store the data of the top functions in top_functions[]
> +top_functions = perf_data[first_func_line:first_func_line
> +                          + number_of_top_functions]
> +
> +# Print information headers
> +print('{:>4}  {:>10}  {:<25}  {}\n{}  {}  {}  {}'.format('No.',
> +                                                         'Percentage',
> +                                                         'Name',
> +                                                         'Caller',
> +                                                         '-' * 4,
> +                                                         '-' * 10,
> +                                                         '-' * 25,
> +                                                         '-' * 25,
> +                                                         ))
> +
> +# Print top 25 functions
> +for (index, function) in enumerate(top_functions, start=1):
> +    function_data = function.split()
> +    print('{:>4}  {:>10}  {:<25}  {}'.format(index,
> +                                             function_data[0],
> +                                             function_data[-1],
> +                                             '
> '.join(function_data[2:-2])))
> +
> +# Remove intermediate files
> +os.system('sudo rm perf.data tmp.perf.data')
> --
> 2.17.1
>
>

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

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

* Re: [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section
  2020-06-16 23:12 ` [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section Ahmed Karaman
  2020-06-17  5:35   ` Aleksandar Markovic
  2020-06-17  5:51   ` Aleksandar Markovic
@ 2020-06-17 12:01   ` Alex Bennée
  2 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2020-06-17 12:01 UTC (permalink / raw)
  To: Ahmed Karaman
  Cc: ldoktor, ehabkost, qemu-devel, aleksandar.qemu.devel, crosa, rth


Ahmed Karaman <ahmedkhaledkaraman@gmail.com> writes:

> This new section includes the 'Performance Tools and Tests' subsection
> which will contain the the performance scripts and benchmarks written
> as a part of the 'TCG Continuous Benchmarking' project.
>
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 955cc8dd5c..68f668ae2a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2974,3 +2974,10 @@ M: Peter Maydell <peter.maydell@linaro.org>
>  S: Maintained
>  F: docs/conf.py
>  F: docs/*/conf.py
> +
> +Miscellaneous
> +-------------
> +Performance Tools and Tests
> +M: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> +S: Maintained
> +F: scripts/performance/
> \ No newline at end of file

Personally I think this fits under "Build and test automation" but
whatever:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée


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

* Re: [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script
  2020-06-16 23:12 ` [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script Ahmed Karaman
  2020-06-17  5:42   ` Aleksandar Markovic
@ 2020-06-17 12:16   ` Alex Bennée
  2020-06-17 16:08     ` Ahmed Karaman
  1 sibling, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2020-06-17 12:16 UTC (permalink / raw)
  To: Ahmed Karaman
  Cc: ldoktor, ehabkost, qemu-devel, aleksandar.qemu.devel, crosa, rth


Ahmed Karaman <ahmedkhaledkaraman@gmail.com> writes:

> Python script that prints the top 25 most executed functions in QEMU
> using callgrind.
>
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> ---
>  scripts/performance/callgrind_top_25.py | 95 +++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
>  create mode 100644 scripts/performance/callgrind_top_25.py
>
> diff --git a/scripts/performance/callgrind_top_25.py b/scripts/performance/callgrind_top_25.py
> new file mode 100644

You will want the script to be +x if the user is to execute it.

> index 0000000000..03b089a96d
> --- /dev/null
> +++ b/scripts/performance/callgrind_top_25.py
> @@ -0,0 +1,95 @@
> +#!/usr/bin/env python3
> +
> +#  Print the top 25 most executed functions in QEMU using callgrind.
> +#  Example Usage:
> +#  callgrind_top_25.py <qemu-build>/x86_64-linux-user/qemu-x86_64
> executable

Why limit to 25, make the name generic and maybe just default to 25
unless the user specifies a different option.

> +#
> +#  This file is a part of the project "TCG Continuous Benchmarking".
> +#
> +#  Copyright (C) 2020  Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> +#  Copyright (C) 2020  Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> +#
> +#  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 <https://www.gnu.org/licenses/>.
> +
> +import os
> +import sys
> +
> +# Ensure sufficient arguments
> +if len(sys.argv) < 3:
> +    print('Insufficient Script Arguments!')
> +    sys.exit(1)
> +
> +# Get the qemu path and the executable + its arguments
> +(qemu, executable) = (sys.argv[1], ' '.join(sys.argv[2:]))

I would recommend using:

  from argparse import ArgumentParser

from the start as adding options with hand parsing will be a pain. I
would suggest a specific option for the qemu binary and then using a
positional argument that can be read after -- so you don't confuse
options.

> +
> +# Run callgrind and callgrind_annotate
> +os.system('valgrind --tool=callgrind --callgrind-out-file=callgrind.data {} {} \
> +            2 > / dev / null & & callgrind_annotate callgrind.data \
> +            > tmp.callgrind.data'.
> +          format(qemu, executable))

Direct os.system calls are discouraged, you tend to get weird effects
like:

  ../../scripts/performance/callgrind_top_25.py ./aarch64-linux-user/qemu-aarch64 ./tests/tcg/aarch64-linux-user/fcvt
  sh: 1: Syntax error: "&" unexpected
  Traceback (most recent call last):
    File "../../scripts/performance/callgrind_top_25.py", line 52, in <module>
      with open('tmp.callgrind.data', 'r') as data:
  FileNotFoundError: [Errno 2] No such file or directory: 'tmp.callgrind.data'

I would:

  - check for valgrind in path and fail gracefully if not found
  - use os.subprocess API for launching (with or without the shell)

> +
> +# Line number with the total number of instructions
> +number_of_instructions_line = 20
> +
> +# Line number with the top function
> +first_func_line = 25

for example

    parser.add_argument('-n', dest="top", type=int, default=25,
                        help="Hottest n functions")

> +
> +# callgrind_annotate output
> +callgrind_data = []
> +
> +# Open callgrind_annotate output and store it in callgrind_data
> +with open('tmp.callgrind.data', 'r') as data:
> +    callgrind_data = data.readlines()
> +
> +# Get the total number of instructions
> +total_number_of_instructions = int(
> +    callgrind_data[number_of_instructions_line].split('
> ')[0].replace(',', ''))

There is no harm in having your steps split out a little.

> +
> +# Number of functions recorded by callgrind
> +number_of_functions = len(callgrind_data) - first_func_line
> +
> +# Limit the number of top functions to 25
> +number_of_top_functions = (25 if number_of_functions >
> +                           25 else number_of_instructions_line)
> +
> +# Store the data of the top functions in top_functions[]
> +top_functions = callgrind_data[first_func_line:
> +                               first_func_line + number_of_top_functions]
> +# Print information headers
> +print('{:>4}  {:>10}  {:<25}  {}\n{}  {}  {}  {}'.format('No.',
> +                                                         'Percentage',
> +                                                         'Name',
> +                                                         'Source File',
> +                                                         '-' * 4,
> +                                                         '-' * 10,
> +                                                         '-' * 25,
> +                                                         '-' * 30,
> +                                                         ))
> +
> +# Print top 25 functions
> +for (index, function) in enumerate(top_functions, start=1):
> +    function_data = function.split()
> +    # Calculate function percentage
> +    percentage = (float(function_data[0].replace(
> +        ',', '')) / total_number_of_instructions) * 100
> +    # Get function source path and name
> +    path, name = function_data[1].split(':')
> +    # Print extracted data
> +    print('{:>4}  {:>9.3f}%  {:<25}  {}'.format(index,
> +                                                round(percentage, 3),
> +                                                name,
> +                                                path))
> +
> +# Remove intermediate files
> +os.system('rm callgrind.data tmp.callgrind.data')

os.unlink()

-- 
Alex Bennée


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

* Re: [PATCH 3/3] scripts/performance: Add perf_top_25.py script
  2020-06-16 23:12 ` [PATCH 3/3] scripts/performance: Add perf_top_25.py script Ahmed Karaman
  2020-06-17  5:56   ` Aleksandar Markovic
@ 2020-06-17 12:21   ` Alex Bennée
  2020-06-17 16:15     ` Ahmed Karaman
  1 sibling, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2020-06-17 12:21 UTC (permalink / raw)
  To: Ahmed Karaman
  Cc: ldoktor, ehabkost, qemu-devel, aleksandar.qemu.devel, crosa, rth


Ahmed Karaman <ahmedkhaledkaraman@gmail.com> writes:

> Python script that prints the top 25 most executed functions in QEMU
> using perf.
>
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> ---
>  scripts/performance/perf_top_25.py | 82 ++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 scripts/performance/perf_top_25.py
>
> diff --git a/scripts/performance/perf_top_25.py b/scripts/performance/perf_top_25.py
> new file mode 100644
> index 0000000000..eaa8cce3c3
> --- /dev/null
> +++ b/scripts/performance/perf_top_25.py
> @@ -0,0 +1,82 @@
> +#!/usr/bin/env python3
> +
> +#  Print the top 25 most executed functions in QEMU using perf.
> +#  Example Usage:
> +#  perf_top_25.py <qemu-build>/x86_64-linux-user/qemu-x86_64 executable
> +#
> +#  This file is a part of the project "TCG Continuous Benchmarking".
> +#
> +#  Copyright (C) 2020  Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> +#  Copyright (C) 2020  Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> +#
> +#  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 <https://www.gnu.org/licenses/>.
> +
> +import os
> +import sys
> +
> +# Ensure sufficient arguments
> +if len(sys.argv) < 3:
> +    print('Insufficient Script Arguments!')
> +    sys.exit(1)
> +
> +# Get the qemu path and the executable + its arguments
> +(qemu_path, executable) = (sys.argv[1], ' '.join(sys.argv[2:]))

Same comments as other script.

> +
> +# Run perf repcord and report
> +os.system('sudo perf record {} {} 2> /dev/null \
> +            && sudo perf report --stdio > tmp.perf.data'
> +          .format(qemu_path, executable))

Why sudo?

Also redirecting just stderr? why?

I think you could separate the steps (as well as use the subprocess
api).

> +
> +# Line number with the top function
> +first_func_line = 11
> +
> +# Perf report output
> +perf_data = []
> +
> +# Open perf report output and store it in perf_data
> +with open('tmp.perf.data', 'r') as data:
> +    perf_data = data.readlines()
> +
> +# Number of functions recorded by perf
> +number_of_functions = len(perf_data) - first_func_line
> +
> +# Limit the number of top functions to 25
> +number_of_top_functions = (25 if number_of_functions >
> +                           25 else number_of_functions)
> +
> +# Store the data of the top functions in top_functions[]
> +top_functions = perf_data[first_func_line:first_func_line
> +                          + number_of_top_functions]
> +
> +# Print information headers
> +print('{:>4}  {:>10}  {:<25}  {}\n{}  {}  {}  {}'.format('No.',
> +                                                         'Percentage',
> +                                                         'Name',
> +                                                         'Caller',
> +                                                         '-' * 4,
> +                                                         '-' * 10,
> +                                                         '-' * 25,
> +                                                         '-' * 25,
> +                                                         ))
> +
> +# Print top 25 functions
> +for (index, function) in enumerate(top_functions, start=1):
> +    function_data = function.split()
> +    print('{:>4}  {:>10}  {:<25}  {}'.format(index,
> +                                             function_data[0],
> +                                             function_data[-1],
> +                                             ' '.join(function_data[2:-2])))
> +
> +# Remove intermediate files
> +os.system('sudo rm perf.data tmp.perf.data')

Again os.unlink()

-- 
Alex Bennée


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

* Re: [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions
  2020-06-16 23:12 [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions Ahmed Karaman
                   ` (3 preceding siblings ...)
  2020-06-16 23:35 ` [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions no-reply
@ 2020-06-17 13:53 ` Eric Blake
  2020-06-17 15:34   ` Alex Bennée
  2020-06-17 16:16   ` Ahmed Karaman
  4 siblings, 2 replies; 22+ messages in thread
From: Eric Blake @ 2020-06-17 13:53 UTC (permalink / raw)
  To: Ahmed Karaman, qemu-devel, rth, aleksandar.qemu.devel,
	alex.bennee, ldoktor, ehabkost, crosa

On 6/16/20 6:12 PM, Ahmed Karaman wrote:
> Greetings,
> 
> As a part of the TCG Continous Benchmarking project for GSoC this
> year, detailed reports discussing different performance measurement
> methodologies and analysis results will be sent here on the mailing
> list.
> 
> The project's first report is currently being revised and will be
> posted on the mailing list in the next few days.
> A section in this report will deal with measuring the top 25 executed
> functions when running QEMU. It includes two Python scripts that
> automatically perform this task.
> 
> This series adds these two scripts to a new performance directory
> created under the scripts directory. It also adds a new
> "Miscellaneous" section to the end of the MAINTAINERS file with a
> "Performance Tools and Tests" subsection.
> 
> Best regards,
> Ahmed Karaman
> 
> Ahmed Karaman (3):
>    MAINTAINERS: Add 'Miscellaneous' section
>    scripts/performance: Add callgrind_top_25.py script
>    scripts/performance: Add perf_top_25.py script
> 
>   MAINTAINERS                             |  7 ++
>   scripts/performance/callgrind_top_25.py | 95 +++++++++++++++++++++++++
>   scripts/performance/perf_top_25.py      | 82 +++++++++++++++++++++
>   3 files changed, 184 insertions(+)
>   create mode 100644 scripts/performance/callgrind_top_25.py
>   create mode 100644 scripts/performance/perf_top_25.py

Are the new scripts supposed to have executable permissions, or are they 
always invoked as 'python path/to/script.py' where the executable bit is 
less important?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions
  2020-06-17 13:53 ` Eric Blake
@ 2020-06-17 15:34   ` Alex Bennée
  2020-06-17 16:16     ` Aleksandar Markovic
  2020-06-17 16:16   ` Ahmed Karaman
  1 sibling, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2020-06-17 15:34 UTC (permalink / raw)
  To: Eric Blake
  Cc: ldoktor, ehabkost, qemu-devel, Ahmed Karaman,
	aleksandar.qemu.devel, crosa, rth


Eric Blake <eblake@redhat.com> writes:

> On 6/16/20 6:12 PM, Ahmed Karaman wrote:
>> Greetings,
>> 
>> As a part of the TCG Continous Benchmarking project for GSoC this
>> year, detailed reports discussing different performance measurement
>> methodologies and analysis results will be sent here on the mailing
>> list.
>> 
>> The project's first report is currently being revised and will be
>> posted on the mailing list in the next few days.
>> A section in this report will deal with measuring the top 25 executed
>> functions when running QEMU. It includes two Python scripts that
>> automatically perform this task.
>> 
>> This series adds these two scripts to a new performance directory
>> created under the scripts directory. It also adds a new
>> "Miscellaneous" section to the end of the MAINTAINERS file with a
>> "Performance Tools and Tests" subsection.
>> 
>> Best regards,
>> Ahmed Karaman
>> 
>> Ahmed Karaman (3):
>>    MAINTAINERS: Add 'Miscellaneous' section
>>    scripts/performance: Add callgrind_top_25.py script
>>    scripts/performance: Add perf_top_25.py script
>> 
>>   MAINTAINERS                             |  7 ++
>>   scripts/performance/callgrind_top_25.py | 95 +++++++++++++++++++++++++
>>   scripts/performance/perf_top_25.py      | 82 +++++++++++++++++++++
>>   3 files changed, 184 insertions(+)
>>   create mode 100644 scripts/performance/callgrind_top_25.py
>>   create mode 100644 scripts/performance/perf_top_25.py
>
> Are the new scripts supposed to have executable permissions, or are they 
> always invoked as 'python path/to/script.py' where the executable bit is 
> less important?

I would assume +x for directly invocable scripts - certainly we have a
lot of those in the scripts directory. 

-- 
Alex Bennée


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

* Re: [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script
  2020-06-17 12:16   ` Alex Bennée
@ 2020-06-17 16:08     ` Ahmed Karaman
  0 siblings, 0 replies; 22+ messages in thread
From: Ahmed Karaman @ 2020-06-17 16:08 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Lukáš Doktor, ehabkost, QEMU Developers,
	Aleksandar Markovic, crosa, Richard Henderson

Thanks Mr. Alex for your suggestions. I will send a v2 of this series
with the updates.

On Wed, Jun 17, 2020 at 2:16 PM Alex Bennée <alex.bennee@linaro.org> wrote:

> You will want the script to be +x if the user is to execute it.

Thanks for the reminder. Forgot to do this before sending the patch.

> > +#  Print the top 25 most executed functions in QEMU using callgrind.
> > +#  Example Usage:
> > +#  callgrind_top_25.py <qemu-build>/x86_64-linux-user/qemu-x86_64
> > executable
>
> Why limit to 25, make the name generic and maybe just default to 25
> unless the user specifies a different option.

Very valid suggestion. Thanks!

>
> I would recommend using:
>
>   from argparse import ArgumentParser
>
> from the start as adding options with hand parsing will be a pain. I
> would suggest a specific option for the qemu binary and then using a
> positional argument that can be read after -- so you don't confuse
> options.
>

Great, what do you think of the format below:
topN_callgrind.py -n20 -- /path/to/qemu executable -executable - arguments

> Direct os.system calls are discouraged, you tend to get weird effects
> like:
>
>   ../../scripts/performance/callgrind_top_25.py ./aarch64-linux-user/qemu-aarch64 ./tests/tcg/aarch64-linux-user/fcvt
>   sh: 1: Syntax error: "&" unexpected
>   Traceback (most recent call last):
>     File "../../scripts/performance/callgrind_top_25.py", line 52, in <module>
>       with open('tmp.callgrind.data', 'r') as data:
>   FileNotFoundError: [Errno 2] No such file or directory: 'tmp.callgrind.data'
>
> I would:
>
>   - check for valgrind in path and fail gracefully if not found
>   - use os.subprocess API for launching (with or without the shell)
>

This weird error was because of the space between "&&" and "2>/dev/null"
These were inserted by the autopep8 python formatter before committing.
When this is fixed, everything works fine, but I believe your
suggestion of using the os.subprocess is valid so I will implement it
and also check for valgrind as you've said.

> > +
> > +# Line number with the total number of instructions
> > +number_of_instructions_line = 20
> > +
> > +# Line number with the top function
> > +first_func_line = 25
>
> for example
>
>     parser.add_argument('-n', dest="top", type=int, default=25,
>                         help="Hottest n functions")

Will also use:
    parser.add_argument('command',  type=str, nargs='+',
                help="QEMU invocation to report the top functions for")
To parse all remaining arguments after "--".

> > +# Get the total number of instructions
> > +total_number_of_instructions = int(
> > +    callgrind_data[number_of_instructions_line].split('
> > ')[0].replace(',', ''))
>
> There is no harm in having your steps split out a little.

Noted!

> > +# Remove intermediate files
> > +os.system('rm callgrind.data tmp.callgrind.data')
>
> os.unlink()
>

Noted!


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

* Re: [PATCH 3/3] scripts/performance: Add perf_top_25.py script
  2020-06-17 12:21   ` Alex Bennée
@ 2020-06-17 16:15     ` Ahmed Karaman
  2020-06-17 17:35       ` Alex Bennée
  0 siblings, 1 reply; 22+ messages in thread
From: Ahmed Karaman @ 2020-06-17 16:15 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Lukáš Doktor, ehabkost, QEMU Developers,
	Aleksandar Markovic, crosa, Richard Henderson

On Wed, Jun 17, 2020 at 2:21 PM Alex Bennée <alex.bennee@linaro.org> wrote:

> > +
> > +# Run perf repcord and report
> > +os.system('sudo perf record {} {} 2> /dev/null \
> > +            && sudo perf report --stdio > tmp.perf.data'
> > +          .format(qemu_path, executable))
>
> Why sudo?

This is the default requirement by perf. You can modify the
kernel.perf_event_paranoid setting to run without root privileges.

> Also redirecting just stderr? why?

Perf, as well as Valgrind, print their output on stderr not stdout.

> I think you could separate the steps (as well as use the subprocess
> api).

Noted!

> Again os.unlink()

Noted!


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

* Re: [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions
  2020-06-17 13:53 ` Eric Blake
  2020-06-17 15:34   ` Alex Bennée
@ 2020-06-17 16:16   ` Ahmed Karaman
  1 sibling, 0 replies; 22+ messages in thread
From: Ahmed Karaman @ 2020-06-17 16:16 UTC (permalink / raw)
  To: Eric Blake
  Cc: Lukáš Doktor, ehabkost, QEMU Developers,
	Aleksandar Markovic, crosa, Alex Bennée, Richard Henderson

On Wed, Jun 17, 2020 at 3:53 PM Eric Blake <eblake@redhat.com> wrote:
> Are the new scripts supposed to have executable permissions, or are they
> always invoked as 'python path/to/script.py' where the executable bit is
> less important?
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>
The execution permission will be added to the scripts in v2 of this
series. There will be no need to invoke them with Python when this is
done.


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

* Re: [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions
  2020-06-17 15:34   ` Alex Bennée
@ 2020-06-17 16:16     ` Aleksandar Markovic
  2020-06-17 17:42       ` Alex Bennée
  0 siblings, 1 reply; 22+ messages in thread
From: Aleksandar Markovic @ 2020-06-17 16:16 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Lukáš Doktor, Eduardo Habkost, QEMU Developers,
	Ahmed Karaman, Cleber Rosa, Richard Henderson

сре, 17. јун 2020. у 17:34 Alex Bennée <alex.bennee@linaro.org> је написао/ла:
>
>
> Eric Blake <eblake@redhat.com> writes:
>
> > On 6/16/20 6:12 PM, Ahmed Karaman wrote:
> >> Greetings,
> >>
> >> As a part of the TCG Continous Benchmarking project for GSoC this
> >> year, detailed reports discussing different performance measurement
> >> methodologies and analysis results will be sent here on the mailing
> >> list.
> >>
> >> The project's first report is currently being revised and will be
> >> posted on the mailing list in the next few days.
> >> A section in this report will deal with measuring the top 25 executed
> >> functions when running QEMU. It includes two Python scripts that
> >> automatically perform this task.
> >>
> >> This series adds these two scripts to a new performance directory
> >> created under the scripts directory. It also adds a new
> >> "Miscellaneous" section to the end of the MAINTAINERS file with a
> >> "Performance Tools and Tests" subsection.
> >>
> >> Best regards,
> >> Ahmed Karaman
> >>
> >> Ahmed Karaman (3):
> >>    MAINTAINERS: Add 'Miscellaneous' section
> >>    scripts/performance: Add callgrind_top_25.py script
> >>    scripts/performance: Add perf_top_25.py script
> >>
> >>   MAINTAINERS                             |  7 ++
> >>   scripts/performance/callgrind_top_25.py | 95 +++++++++++++++++++++++++
> >>   scripts/performance/perf_top_25.py      | 82 +++++++++++++++++++++
> >>   3 files changed, 184 insertions(+)
> >>   create mode 100644 scripts/performance/callgrind_top_25.py
> >>   create mode 100644 scripts/performance/perf_top_25.py
> >
> > Are the new scripts supposed to have executable permissions, or are they
> > always invoked as 'python path/to/script.py' where the executable bit is
> > less important?
>
> I would assume +x for directly invocable scripts - certainly we have a
> lot of those in the scripts directory.
>

There is no reason IMHO for these scripts not having +x and Ahmed
should correct this in v2, and I think that was his original
intention.

However, I was a little surprized though when I found out this in our
scripts directory:

$ find . -name \*.py -type f  -print | xargs ls -l
-rwxr-xr-x 1 rtrk rtrk  9103 May 10 11:21 ./analyse-9p-simpletrace.py
-rwxr-xr-x 1 rtrk rtrk  3544 May 10 11:21 ./analyse-locks-simpletrace.py
-rwxr-xr-x 1 rtrk rtrk 20647 Jun  2 10:22 ./analyze-migration.py
-rwxr-xr-x 1 rtrk rtrk 38358 Jun  2 10:22 ./decodetree.py
-rw-r--r-- 1 rtrk rtrk 20723 May 10 11:21 ./dump-guest-memory.py
-rwxr-xr-x 1 rtrk rtrk 23599 May 10 11:21 ./minikconf.py
-rw-r--r-- 1 rtrk rtrk  2751 Jun  2 10:22 ./modules/module_block.py
-rw-r--r-- 1 rtrk rtrk  7842 May 10 11:21 ./qapi/commands.py
-rw-r--r-- 1 rtrk rtrk  5673 May 10 11:21 ./qapi/common.py
-rw-r--r-- 1 rtrk rtrk  9742 May 10 11:21 ./qapi/doc.py
-rw-r--r-- 1 rtrk rtrk  1107 May 10 11:21 ./qapi/error.py
-rw-r--r-- 1 rtrk rtrk  6148 May 10 11:21 ./qapi/events.py
-rw-r--r-- 1 rtrk rtrk 12297 May 10 11:21 ./qapi/expr.py
-rwxr-xr-x 1 rtrk rtrk  2066 May 10 11:21 ./qapi-gen.py
-rw-r--r-- 1 rtrk rtrk  8280 May 10 11:21 ./qapi/gen.py
-rw-r--r-- 1 rtrk rtrk     0 May 10 11:21 ./qapi/__init__.py
-rw-r--r-- 1 rtrk rtrk  8533 May 10 11:21 ./qapi/introspect.py
-rw-r--r-- 1 rtrk rtrk 21696 May 10 11:21 ./qapi/parser.py
-rw-r--r-- 1 rtrk rtrk 41301 May 10 11:21 ./qapi/schema.py
-rw-r--r-- 1 rtrk rtrk  1789 May 10 11:21 ./qapi/source.py
-rw-r--r-- 1 rtrk rtrk  8724 May 10 11:21 ./qapi/types.py
-rw-r--r-- 1 rtrk rtrk  9980 May 10 11:21 ./qapi/visit.py
-rw-r--r-- 1 rtrk rtrk  1843 Jun  2 10:22 ./qemugdb/aio.py
-rw-r--r-- 1 rtrk rtrk  3514 Jun  2 10:22 ./qemugdb/coroutine.py
-rw-r--r-- 1 rtrk rtrk   897 Jun  2 10:22 ./qemugdb/__init__.py
-rw-r--r-- 1 rtrk rtrk  2741 Jun  2 10:22 ./qemugdb/mtree.py
-rw-r--r-- 1 rtrk rtrk  1213 Jun  2 10:22 ./qemu-gdb.py
-rw-r--r-- 1 rtrk rtrk  1469 Jun  2 10:22 ./qemugdb/tcg.py
-rw-r--r-- 1 rtrk rtrk  1850 Jun  2 10:22 ./qemugdb/timers.py
-rwxr-xr-x 1 rtrk rtrk  3791 May 10 11:21 ./render_block_graph.py
-rwxr-xr-x 1 rtrk rtrk 12118 May 10 11:21 ./replay-dump.py
-rwxr-xr-x 1 rtrk rtrk  1272 May 10 11:21 ./signrom.py
-rwxr-xr-x 1 rtrk rtrk  3963 May 10 11:21 ./simplebench/bench_block_job.py
-rw-r--r-- 1 rtrk rtrk  2447 May 10 11:21 ./simplebench/bench-example.py
-rw-r--r-- 1 rtrk rtrk  4615 May 10 11:21 ./simplebench/simplebench.py
-rwxr-xr-x 1 rtrk rtrk  8572 May 10 11:21 ./simpletrace.py
-rw-r--r-- 1 rtrk rtrk  1443 May 10 11:21 ./tracetool/backend/dtrace.py
-rw-r--r-- 1 rtrk rtrk  1471 May 10 11:21 ./tracetool/backend/ftrace.py
-rw-r--r-- 1 rtrk rtrk  4098 May 10 11:21 ./tracetool/backend/__init__.py
-rw-r--r-- 1 rtrk rtrk  1499 May 10 11:21 ./tracetool/backend/log.py
-rw-r--r-- 1 rtrk rtrk  3116 May 10 11:21 ./tracetool/backend/simple.py
-rw-r--r-- 1 rtrk rtrk  1175 May 10 11:21 ./tracetool/backend/syslog.py
-rw-r--r-- 1 rtrk rtrk  1190 May 10 11:21 ./tracetool/backend/ust.py
-rw-r--r-- 1 rtrk rtrk  2094 May 10 11:21 ./tracetool/format/c.py
-rw-r--r-- 1 rtrk rtrk  1744 May 10 11:21 ./tracetool/format/d.py
-rw-r--r-- 1 rtrk rtrk  2912 May 10 11:21 ./tracetool/format/h.py
-rw-r--r-- 1 rtrk rtrk  2402 May 10 11:21 ./tracetool/format/__init__.py
-rw-r--r-- 1 rtrk rtrk  3725 May 10 11:21 ./tracetool/format/log_stap.py
-rw-r--r-- 1 rtrk rtrk  2467 May 10 11:21 ./tracetool/format/simpletrace_stap.py
-rw-r--r-- 1 rtrk rtrk  1653 May 10 11:21 ./tracetool/format/stap.py
-rw-r--r-- 1 rtrk rtrk  2388 May 10 11:21 ./tracetool/format/tcg_helper_c.py
-rw-r--r-- 1 rtrk rtrk  1343 May 10 11:21 ./tracetool/format/tcg_helper_h.py
-rw-r--r-- 1 rtrk rtrk  2145 May 10 11:21
./tracetool/format/tcg_helper_wrapper_h.py
-rw-r--r-- 1 rtrk rtrk  2749 May 10 11:21 ./tracetool/format/tcg_h.py
-rw-r--r-- 1 rtrk rtrk   968 May 10 11:21 ./tracetool/format/ust_events_c.py
-rw-r--r-- 1 rtrk rtrk  3678 May 10 11:21 ./tracetool/format/ust_events_h.py
-rw-r--r-- 1 rtrk rtrk 14489 May 10 11:21 ./tracetool/__init__.py
-rwxr-xr-x 1 rtrk rtrk  4525 May 10 11:21 ./tracetool.py
-rw-r--r-- 1 rtrk rtrk  4301 May 10 11:21 ./tracetool/transform.py
-rw-r--r-- 1 rtrk rtrk  2067 May 10 11:21 ./tracetool/vcpu.py
-rwxr-xr-x 1 rtrk rtrk 16117 May 10 11:21 ./vmstate-static-checker.py

$ find . -name \*.pl -type f -print | xargs ls -l
-rwxr-xr-x 1 rtrk rtrk 84694 May 10 11:21 ./checkpatch.pl
-rwxr-xr-x 1 rtrk rtrk  6559 May 10 11:21 ./clean-header-guards.pl
-rwxr-xr-x 1 rtrk rtrk  1396 May 10 11:21 ./cleanup-trace-events.pl
-rwxr-xr-x 1 rtrk rtrk  2725 May 10 11:21 ./disas-objdump.pl
-rwxr-xr-x 1 rtrk rtrk 54645 May 10 11:21 ./get_maintainer.pl
-rwxr-xr-x 1 rtrk rtrk  4289 May 10 11:21 ./hxtool-conv.pl
-rw-r--r-- 1 rtrk rtrk   310 May 10 11:21 ./shaderinclude.pl
-rwxr-xr-x 1 rtrk rtrk  9933 May 10 11:21 ./tap-driver.pl
-rwxr-xr-x 1 rtrk rtrk  3011 May 10 11:21 ./tap-merge.pl
-rwxr-xr-x 1 rtrk rtrk 13657 May 10 11:21 ./texi2pod.pl

Are all these permissions all right?

Thanks,
Aleksandar







> --
> Alex Bennée


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

* Re: [PATCH 3/3] scripts/performance: Add perf_top_25.py script
  2020-06-17 16:15     ` Ahmed Karaman
@ 2020-06-17 17:35       ` Alex Bennée
  2020-06-17 18:21         ` Ahmed Karaman
  0 siblings, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2020-06-17 17:35 UTC (permalink / raw)
  To: Ahmed Karaman
  Cc: Lukáš Doktor, ehabkost, QEMU Developers,
	Aleksandar Markovic, crosa, Richard Henderson


Ahmed Karaman <ahmedkhaledkaraman@gmail.com> writes:

> On Wed, Jun 17, 2020 at 2:21 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> > +
>> > +# Run perf repcord and report
>> > +os.system('sudo perf record {} {} 2> /dev/null \
>> > +            && sudo perf report --stdio > tmp.perf.data'
>> > +          .format(qemu_path, executable))
>>
>> Why sudo?
>
> This is the default requirement by perf. You can modify the
> kernel.perf_event_paranoid setting to run without root privileges.

Right - which I do as a developer. It would be rude to sudo things if
you don't need to because then you end up running your potentially
un-trusted application with root privileges.

Could we either probe for the requirement or require an explicit sudo
flag which we can prompt for if it fails?

>
>> Also redirecting just stderr? why?
>
> Perf, as well as Valgrind, print their output on stderr not stdout.

Right so I think a bit of splitting apart and use of subprocess can make
this cleaner and not involve quite so much being done with shell
redirection in one invocation.

>
>> I think you could separate the steps (as well as use the subprocess
>> api).
>
> Noted!
>
>> Again os.unlink()
>
> Noted!


-- 
Alex Bennée


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

* Re: [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions
  2020-06-17 16:16     ` Aleksandar Markovic
@ 2020-06-17 17:42       ` Alex Bennée
  0 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2020-06-17 17:42 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: Lukáš Doktor, Eduardo Habkost, QEMU Developers,
	Ahmed Karaman, Cleber Rosa, Richard Henderson


Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> writes:

> сре, 17. јун 2020. у 17:34 Alex Bennée <alex.bennee@linaro.org> је написао/ла:
>>
>>
>> Eric Blake <eblake@redhat.com> writes:
>>
>> > On 6/16/20 6:12 PM, Ahmed Karaman wrote:
>> >> Greetings,
>> >>
>> >> As a part of the TCG Continous Benchmarking project for GSoC this
>> >> year, detailed reports discussing different performance measurement
>> >> methodologies and analysis results will be sent here on the mailing
>> >> list.
>> >>
>> >> The project's first report is currently being revised and will be
>> >> posted on the mailing list in the next few days.
>> >> A section in this report will deal with measuring the top 25 executed
>> >> functions when running QEMU. It includes two Python scripts that
>> >> automatically perform this task.
>> >>
>> >> This series adds these two scripts to a new performance directory
>> >> created under the scripts directory. It also adds a new
>> >> "Miscellaneous" section to the end of the MAINTAINERS file with a
>> >> "Performance Tools and Tests" subsection.
>> >>
>> >> Best regards,
>> >> Ahmed Karaman
>> >>
>> >> Ahmed Karaman (3):
>> >>    MAINTAINERS: Add 'Miscellaneous' section
>> >>    scripts/performance: Add callgrind_top_25.py script
>> >>    scripts/performance: Add perf_top_25.py script
>> >>
>> >>   MAINTAINERS                             |  7 ++
>> >>   scripts/performance/callgrind_top_25.py | 95 +++++++++++++++++++++++++
>> >>   scripts/performance/perf_top_25.py      | 82 +++++++++++++++++++++
>> >>   3 files changed, 184 insertions(+)
>> >>   create mode 100644 scripts/performance/callgrind_top_25.py
>> >>   create mode 100644 scripts/performance/perf_top_25.py
>> >
>> > Are the new scripts supposed to have executable permissions, or are they
>> > always invoked as 'python path/to/script.py' where the executable bit is
>> > less important?
>>
>> I would assume +x for directly invocable scripts - certainly we have a
>> lot of those in the scripts directory.
>>
>
> There is no reason IMHO for these scripts not having +x and Ahmed
> should correct this in v2, and I think that was his original
> intention.
>
> However, I was a little surprized though when I found out this in our
> scripts directory:
>
> $ find . -name \*.py -type f  -print | xargs ls -l
<snip>
> -rw-r--r-- 1 rtrk rtrk 20723 May 10 11:21 ./dump-guest-memory.py
> -rw-r--r-- 1 rtrk rtrk  2751 Jun  2 10:22 ./modules/module_block.py
> -rw-r--r-- 1 rtrk rtrk  7842 May 10 11:21 ./qapi/commands.py
> -rw-r--r-- 1 rtrk rtrk  5673 May 10 11:21 ./qapi/common.py
> -rw-r--r-- 1 rtrk rtrk  9742 May 10 11:21 ./qapi/doc.py
> -rw-r--r-- 1 rtrk rtrk  1107 May 10 11:21 ./qapi/error.py
> -rw-r--r-- 1 rtrk rtrk  6148 May 10 11:21 ./qapi/events.py
> -rw-r--r-- 1 rtrk rtrk 12297 May 10 11:21 ./qapi/expr.py
> -rw-r--r-- 1 rtrk rtrk  8280 May 10 11:21 ./qapi/gen.py
> -rw-r--r-- 1 rtrk rtrk     0 May 10 11:21 ./qapi/__init__.py
> -rw-r--r-- 1 rtrk rtrk  8533 May 10 11:21 ./qapi/introspect.py
> -rw-r--r-- 1 rtrk rtrk 21696 May 10 11:21 ./qapi/parser.py
> -rw-r--r-- 1 rtrk rtrk 41301 May 10 11:21 ./qapi/schema.py
> -rw-r--r-- 1 rtrk rtrk  1789 May 10 11:21 ./qapi/source.py
> -rw-r--r-- 1 rtrk rtrk  8724 May 10 11:21 ./qapi/types.py
> -rw-r--r-- 1 rtrk rtrk  9980 May 10 11:21 ./qapi/visit.py
> -rw-r--r-- 1 rtrk rtrk  1843 Jun  2 10:22 ./qemugdb/aio.py
> -rw-r--r-- 1 rtrk rtrk  3514 Jun  2 10:22 ./qemugdb/coroutine.py
> -rw-r--r-- 1 rtrk rtrk   897 Jun  2 10:22 ./qemugdb/__init__.py
> -rw-r--r-- 1 rtrk rtrk  2741 Jun  2 10:22 ./qemugdb/mtree.py
> -rw-r--r-- 1 rtrk rtrk  1213 Jun  2 10:22 ./qemu-gdb.py
> -rw-r--r-- 1 rtrk rtrk  1469 Jun  2 10:22 ./qemugdb/tcg.py
> -rw-r--r-- 1 rtrk rtrk  1850 Jun  2 10:22 ./qemugdb/timers.py
> -rw-r--r-- 1 rtrk rtrk  2447 May 10 11:21 ./simplebench/bench-example.py
> -rw-r--r-- 1 rtrk rtrk  4615 May 10 11:21 ./simplebench/simplebench.py
> -rw-r--r-- 1 rtrk rtrk  1443 May 10 11:21 ./tracetool/backend/dtrace.py
> -rw-r--r-- 1 rtrk rtrk  1471 May 10 11:21 ./tracetool/backend/ftrace.py
> -rw-r--r-- 1 rtrk rtrk  4098 May 10 11:21 ./tracetool/backend/__init__.py
> -rw-r--r-- 1 rtrk rtrk  1499 May 10 11:21 ./tracetool/backend/log.py
> -rw-r--r-- 1 rtrk rtrk  3116 May 10 11:21 ./tracetool/backend/simple.py
> -rw-r--r-- 1 rtrk rtrk  1175 May 10 11:21 ./tracetool/backend/syslog.py
> -rw-r--r-- 1 rtrk rtrk  1190 May 10 11:21 ./tracetool/backend/ust.py
> -rw-r--r-- 1 rtrk rtrk  2094 May 10 11:21 ./tracetool/format/c.py
> -rw-r--r-- 1 rtrk rtrk  1744 May 10 11:21 ./tracetool/format/d.py
> -rw-r--r-- 1 rtrk rtrk  2912 May 10 11:21 ./tracetool/format/h.py
> -rw-r--r-- 1 rtrk rtrk  2402 May 10 11:21 ./tracetool/format/__init__.py
> -rw-r--r-- 1 rtrk rtrk  3725 May 10 11:21 ./tracetool/format/log_stap.py
> -rw-r--r-- 1 rtrk rtrk  2467 May 10 11:21 ./tracetool/format/simpletrace_stap.py
> -rw-r--r-- 1 rtrk rtrk  1653 May 10 11:21 ./tracetool/format/stap.py
> -rw-r--r-- 1 rtrk rtrk  2388 May 10 11:21 ./tracetool/format/tcg_helper_c.py
> -rw-r--r-- 1 rtrk rtrk  1343 May 10 11:21 ./tracetool/format/tcg_helper_h.py
> -rw-r--r-- 1 rtrk rtrk  2145 May 10 11:21
> ./tracetool/format/tcg_helper_wrapper_h.py
> -rw-r--r-- 1 rtrk rtrk  2749 May 10 11:21 ./tracetool/format/tcg_h.py
> -rw-r--r-- 1 rtrk rtrk   968 May 10 11:21 ./tracetool/format/ust_events_c.py
> -rw-r--r-- 1 rtrk rtrk  3678 May 10 11:21 ./tracetool/format/ust_events_h.py
> -rw-r--r-- 1 rtrk rtrk 14489 May 10 11:21 ./tracetool/__init__.py
> -rw-r--r-- 1 rtrk rtrk  4301 May 10 11:21 ./tracetool/transform.py
> -rw-r--r-- 1 rtrk rtrk  2067 May 10 11:21 ./tracetool/vcpu.py

So I think these are all modules or loaded indirectly (in the case of
the gdb helpers). There was a big clean-up recently removing the
#!/bin/env python headers from a chunk of these.

<snip>
> -rw-r--r-- 1 rtrk rtrk   310 May 10 11:21 ./shaderinclude.pl

Arguably this could be +x but it seems to be there for the benefit of
the make system which explicitly calls perl anyway.

>
> Are all these permissions all right?

I think so.

-- 
Alex Bennée


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

* Re: [PATCH 3/3] scripts/performance: Add perf_top_25.py script
  2020-06-17 17:35       ` Alex Bennée
@ 2020-06-17 18:21         ` Ahmed Karaman
  2020-06-18 15:07           ` Aleksandar Markovic
  0 siblings, 1 reply; 22+ messages in thread
From: Ahmed Karaman @ 2020-06-17 18:21 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Lukáš Doktor, ehabkost, QEMU Developers,
	Aleksandar Markovic, crosa, Richard Henderson

On Wed, Jun 17, 2020 at 7:35 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Right - which I do as a developer. It would be rude to sudo things if
> you don't need to because then you end up running your potentially
> un-trusted application with root privileges.
>
> Could we either probe for the requirement or require an explicit sudo
> flag which we can prompt for if it fails?
>
To make sure I got it right. You mean I should specify in the script
comment on the top that the user should modify the
kernel.perf_event_paranoid setting in order to run the script, otherwise,
they should add a --sudo flag when running the Python script to invoke
perf as sudo?

> >
> >> Also redirecting just stderr? why?
> >
> > Perf, as well as Valgrind, print their output on stderr not stdout.
>
> Right so I think a bit of splitting apart and use of subprocess can make
> this cleaner and not involve quite so much being done with shell
> redirection in one invocation.
>

Noted!


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

* Re: [PATCH 3/3] scripts/performance: Add perf_top_25.py script
  2020-06-17 18:21         ` Ahmed Karaman
@ 2020-06-18 15:07           ` Aleksandar Markovic
  0 siblings, 0 replies; 22+ messages in thread
From: Aleksandar Markovic @ 2020-06-18 15:07 UTC (permalink / raw)
  To: Ahmed Karaman
  Cc: Lukáš Doktor, ehabkost, Alex Bennée,
	QEMU Developers, crosa, Richard Henderson

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

среда, 17. јун 2020., Ahmed Karaman <ahmedkhaledkaraman@gmail.com> је
написао/ла:

> On Wed, Jun 17, 2020 at 7:35 PM Alex Bennée <alex.bennee@linaro.org>
> wrote:
> >
> > Right - which I do as a developer. It would be rude to sudo things if
> > you don't need to because then you end up running your potentially
> > un-trusted application with root privileges.
> >
> > Could we either probe for the requirement or require an explicit sudo
> > flag which we can prompt for if it fails?
> >
> To make sure I got it right. You mean I should specify in the script
> comment on the top that the user should modify the
> kernel.perf_event_paranoid setting in order to run the script, otherwise,
> they should add a --sudo flag when running the Python script to invoke
> perf as sudo?
>
>
I think Alex meant that everything related to super user access should be
examined by the script. You could:

A. Establish if the user that execute the script is already super user. If
yes, you can proceed with command lines containing "perf. (see hints how to
do this here:
https://stackoverflow.com/questions/2806897/what-is-the-best-way-for-checking-if-the-user-of-a-script-has-root-like-privileg
)

B. Establish if "perf" can be executed successfully with current user
privelages. If yes, you can also proceed with command lines containing
"perf" (a primitive but simple way for establishing this is to run "perf
stat ls /" and see what happens, success or error)

C. If neither A nor B are satisfied, you cold emit error message
instructing the user what he needs to/could do.

Just my 2 c. Not sure if this is compatible with Alex' thoughts.

Aleksandar


> > >
> > >> Also redirecting just stderr? why?
> > >
> > > Perf, as well as Valgrind, print their output on stderr not stdout.
> >
> > Right so I think a bit of splitting apart and use of subprocess can make
> > this cleaner and not involve quite so much being done with shell
> > redirection in one invocation.
> >
>
> Noted!
>

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

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

end of thread, other threads:[~2020-06-18 15:10 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-16 23:12 [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions Ahmed Karaman
2020-06-16 23:12 ` [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section Ahmed Karaman
2020-06-17  5:35   ` Aleksandar Markovic
2020-06-17  5:51   ` Aleksandar Markovic
2020-06-17 12:01   ` Alex Bennée
2020-06-16 23:12 ` [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script Ahmed Karaman
2020-06-17  5:42   ` Aleksandar Markovic
2020-06-17 12:16   ` Alex Bennée
2020-06-17 16:08     ` Ahmed Karaman
2020-06-16 23:12 ` [PATCH 3/3] scripts/performance: Add perf_top_25.py script Ahmed Karaman
2020-06-17  5:56   ` Aleksandar Markovic
2020-06-17 12:21   ` Alex Bennée
2020-06-17 16:15     ` Ahmed Karaman
2020-06-17 17:35       ` Alex Bennée
2020-06-17 18:21         ` Ahmed Karaman
2020-06-18 15:07           ` Aleksandar Markovic
2020-06-16 23:35 ` [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions no-reply
2020-06-17 13:53 ` Eric Blake
2020-06-17 15:34   ` Alex Bennée
2020-06-17 16:16     ` Aleksandar Markovic
2020-06-17 17:42       ` Alex Bennée
2020-06-17 16:16   ` Ahmed Karaman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).