| Message ID | 20200828104102.4490-2-ahmedkhaledkaraman@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/9] scripts/performance: Refactor topN_perf.py | expand |
On Friday, August 28, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com> wrote: > - Apply pylint and flake8 formatting rules to the script. > - Use 'tempfile' instead of '/tmp' for creating temporary files. > > Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> Reply Reply all Forward View Gmail in: *Mobile* | Older version <https://mail.google.com/mail/mu/mp/966/#> | Desktop <https://mail.google.com/mail/mu/mp/966/#> © 2020 Google > Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com> > --- > scripts/performance/topN_perf.py | 174 +++++++++++++++---------------- > 1 file changed, 87 insertions(+), 87 deletions(-) > > diff --git a/scripts/performance/topN_perf.py b/scripts/performance/topN_ > perf.py > index 07be195fc8..56b100da87 100755 > --- a/scripts/performance/topN_perf.py > +++ b/scripts/performance/topN_perf.py > @@ -1,72 +1,77 @@ > #!/usr/bin/env python3 > > -# Print the top N most executed functions in QEMU using perf. > -# Syntax: > -# topN_perf.py [-h] [-n] <number of displayed top functions> -- \ > -# <qemu executable> [<qemu executable options>] \ > -# <target executable> [<target execurable options>] > -# > -# [-h] - Print the script arguments help message. > -# [-n] - Specify the number of top functions to print. > -# - If this flag is not specified, the tool defaults to 25. > -# > -# Example of usage: > -# topN_perf.py -n 20 -- qemu-arm coulomb_double-arm > -# > -# 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/>. > +""" > +Print the top N most executed functions in QEMU using perf. > + > +Syntax: > +topN_perf.py [-h] [-n <number of displayed top functions>] -- \ > + <qemu executable> [<qemu executable options>] \ > + <target executable> [<target execurable options>] > + > +[-h] - Print the script arguments help message. > +[-n] - Specify the number of top functions to print. > + - If this flag is not specified, the tool defaults to 25. > + > +Example of usage: > +topN_perf.py -n 20 -- qemu-arm coulomb_double-arm > + > +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 argparse > import os > import subprocess > import sys > +import tempfile > > > # Parse the command line arguments > -parser = argparse.ArgumentParser( > - usage='topN_perf.py [-h] [-n] <number of displayed top functions > > -- ' > +PARSER = argparse.ArgumentParser( > + usage='topN_perf.py [-h] [-n <number of displayed top functions>] -- ' > '<qemu executable> [<qemu executable options>] ' > '<target executable> [<target executable options>]') > > -parser.add_argument('-n', dest='top', type=int, default=25, > +PARSER.add_argument('-n', dest='top', type=int, default=25, > help='Specify the number of top functions to print.') > > -parser.add_argument('command', type=str, nargs='+', > help=argparse.SUPPRESS) > +PARSER.add_argument('command', type=str, nargs='+', > help=argparse.SUPPRESS) > > -args = parser.parse_args() > +ARGS = PARSER.parse_args() > > # Extract the needed variables from the args > -command = args.command > -top = args.top > +COMMAND = ARGS.command > +TOP = ARGS.top > > # Insure that perf is installed > -check_perf_presence = subprocess.run(["which", "perf"], > - stdout=subprocess.DEVNULL) > -if check_perf_presence.returncode: > +CHECK_PERF_PRESENCE = subprocess.run(["which", "perf"], > + stdout=subprocess.DEVNULL, > + check=False) > +if CHECK_PERF_PRESENCE.returncode: > sys.exit("Please install perf before running the script!") > > # Insure user has previllage to run perf > -check_perf_executability = subprocess.run(["perf", "stat", "ls", "/"], > +CHECK_PERF_EXECUTABILITY = subprocess.run(["perf", "stat", "ls", "/"], > stdout=subprocess.DEVNULL, > - stderr=subprocess.DEVNULL) > -if check_perf_executability.returncode: > - sys.exit( > -""" > + stderr=subprocess.DEVNULL, > + check=False) > +if CHECK_PERF_EXECUTABILITY.returncode: > + sys.exit(""" > Error: > You may not have permission to collect stats. > > @@ -85,43 +90,42 @@ To make this setting permanent, edit /etc/sysctl.conf > too, e.g.: > kernel.perf_event_paranoid = -1 > > * Alternatively, you can run this script under sudo privileges. > -""" > -) > - > -# Run perf record > -perf_record = subprocess.run((["perf", "record", > "--output=/tmp/perf.data"] + > - command), > - stdout=subprocess.DEVNULL, > - stderr=subprocess.PIPE) > -if perf_record.returncode: > - os.unlink('/tmp/perf.data') > - sys.exit(perf_record.stderr.decode("utf-8")) > - > -# Save perf report output to /tmp/perf_report.out > -with open("/tmp/perf_report.out", "w") as output: > - perf_report = subprocess.run( > - ["perf", "report", "--input=/tmp/perf.data", "--stdio"], > - stdout=output, > - stderr=subprocess.PIPE) > - if perf_report.returncode: > - os.unlink('/tmp/perf.data') > - output.close() > - os.unlink('/tmp/perf_report.out') > - sys.exit(perf_report.stderr.decode("utf-8")) > - > -# Read the reported data to functions[] > -functions = [] > -with open("/tmp/perf_report.out", "r") as data: > - # Only read lines that are not comments (comments start with #) > - # Only read lines that are not empty > - functions = [line for line in data.readlines() if line and line[0] > - != '#' and line[0] != "\n"] > - > -# Limit the number of top functions to "top" > -number_of_top_functions = top if len(functions) > top else len(functions) > - > -# Store the data of the top functions in top_functions[] > -top_functions = functions[:number_of_top_functions] > +""") > + > +# Run perf and save all intermediate files in a temporary directory > +with tempfile.TemporaryDirectory() as tmpdir: > + RECORD_PATH = os.path.join(tmpdir, "record.data") > + REPORT_PATH = os.path.join(tmpdir, "report.txt") > + > + PERF_RECORD = subprocess.run((["perf", "record", > "--output="+RECORD_PATH] + > + COMMAND), > + stdout=subprocess.DEVNULL, > + stderr=subprocess.PIPE, > + check=False) > + if PERF_RECORD.returncode: > + sys.exit(PERF_RECORD.stderr.decode("utf-8")) > + > + with open(REPORT_PATH, "w") as output: > + PERF_REPORT = subprocess.run( > + ["perf", "report", "--input="+RECORD_PATH, "--stdio"], > + stdout=output, > + stderr=subprocess.PIPE, > + check=False) > + if PERF_REPORT.returncode: > + sys.exit(PERF_REPORT.stderr.decode("utf-8")) > + > + # Save the reported data to FUNCTIONS[] > + with open(REPORT_PATH, "r") as data: > + # Only read lines that are not comments (comments start with #) > + # Only read lines that are not empty > + FUNCTIONS = [line for line in data.readlines() if line and > + line[0] != '#' and line[0] != "\n"] > + > +# Limit the number of top functions to "TOP" > +NO_TOP_FUNCTIONS = TOP if len(FUNCTIONS) > TOP else len(FUNCTIONS) > + > +# Store the data of the top functions in TOP_FUNCTIONS[] > +TOP_FUNCTIONS = FUNCTIONS[:NO_TOP_FUNCTIONS] > > # Print table header > print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.', > @@ -134,7 +138,7 @@ print('{:>4} {:>10} {:<30} {}\n{} {} {} > {}'.format('No.', > '-' * 25)) > > # Print top N functions > -for (index, function) in enumerate(top_functions, start=1): > +for (index, function) in enumerate(TOP_FUNCTIONS, start=1): > function_data = function.split() > function_percentage = function_data[0] > function_name = function_data[-1] > @@ -143,7 +147,3 @@ for (index, function) in enumerate(top_functions, > start=1): > function_percentage, > function_name, > function_invoker)) > - > -# Remove intermediate files > -os.unlink('/tmp/perf.data') > -os.unlink('/tmp/perf_report.out') > -- > 2.17.1 > > <br><br>On Friday, August 28, 2020, Ahmed Karaman <<a href="mailto:ahmedkhaledkaraman@gmail.com">ahmedkhaledkaraman@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">- Apply pylint and flake8 formatting rules to the script.<br> - Use 'tempfile' instead of '/tmp' for creating temporary files.<br> <br></blockquote><div><br></div><div><div id="cv_" class=" Xg " style="width:320px;height:auto"><div class="Zg" style="width:320px;overflow-y:auto"><div class="Rg"><div class="Li"><div class="yj"><div class="Ph"><div id="cvcmsg_1744e012f27314c1" class="yh" style="border-top-left-radius:0px;border-top-right-radius:0px;margin-bottom:11px"><div class="Vh" id="cvcfullmsg_1744e012f27314c1"><div id="cvcmsgbod_1744e012f27314c1" class="aj"><div class="Ni"><div class="ni pi " dir="ltr"><div>Reviewed-by: Aleksandar Markovic <<a href="mailto:aleksandar.qemu.devel@gmail.com" target="_blank">aleksandar.qemu.devel@gmail.c<wbr>om</a>><br></div><div style="clear:both"></div></div><div style="clear:both"></div><div><div class="M j T b hc Aj S" tabindex="0"><div class="V j hf"></div></div></div><div style="clear:both"></div></div></div></div></div></div><div class="Pj"></div><div id="cvcfb" class="Cj"><div class="Dj" style="width:300px"><div class="Ej"><div class="M j T b hc Fj Mi S" tabindex="0"><div class="V j vd Hj"></div><div class="V j Y Lj">Reply</div></div></div><div class="Gj" style="background-size:initial"></div><div id="cvcfra" class="Ej"><div class="M j T b hc Fj Mi S" tabindex="0"><div class="V j ud Hj"></div><div class="V j Y Lj">Reply all</div></div></div><div id="cvcfrab" class="Gj" style="background-size:initial"></div><div class="Ej"><div class="M j T b hc Fj Mi S" tabindex="0"><div class="V j sd Hj"></div><div class="V j Y Lj">Forward</div></div></div></div></div></div></div><div class="Og"><div class="dh"><div class="Nh"><div class="Nh">View Gmail in: <b>Mobile</b> | <a href="https://mail.google.com/mail/mu/mp/966/#">Older version</a> | <a href="https://mail.google.com/mail/mu/mp/966/#">Desktop</a></div><div class="Nh Oh"><span dir="ltr">© 2020 Google</span></div></div></div></div></div></div><div class="ah" style="width:320px"></div></div><div class="Ls" style="display:block"><div class="As"><div class="Bs"><div class="Gs " style="font-size:13px"><div class="Is Hs"></div></div></div></div></div></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Signed-off-by: Ahmed Karaman <<a href="mailto:ahmedkhaledkaraman@gmail.com">ahmedkhaledkaraman@gmail.com</a>><br> ---<br> scripts/performance/topN_perf.<wbr>py | 174 +++++++++++++++---------------<wbr>-<br> 1 file changed, 87 insertions(+), 87 deletions(-)<br> <br> diff --git a/scripts/performance/topN_<wbr>perf.py b/scripts/performance/topN_<wbr>perf.py<br> index 07be195fc8..56b100da87 100755<br> --- a/scripts/performance/topN_<wbr>perf.py<br> +++ b/scripts/performance/topN_<wbr>perf.py<br> @@ -1,72 +1,77 @@<br> #!/usr/bin/env python3<br> <br> -# Print the top N most executed functions in QEMU using perf.<br> -# Syntax:<br> -# topN_perf.py [-h] [-n] <number of displayed top functions> -- \<br> -# <qemu executable> [<qemu executable options>] \<br> -# <target executable> [<target execurable options>]<br> -#<br> -# [-h] - Print the script arguments help message.<br> -# [-n] - Specify the number of top functions to print.<br> -# - If this flag is not specified, the tool defaults to 25.<br> -#<br> -# Example of usage:<br> -# topN_perf.py -n 20 -- qemu-arm coulomb_double-arm<br> -#<br> -# This file is a part of the project "TCG Continuous Benchmarking".<br> -#<br> -# Copyright (C) 2020 Ahmed Karaman <<a href="mailto:ahmedkhaledkaraman@gmail.com">ahmedkhaledkaraman@gmail.com</a>><br> -# Copyright (C) 2020 Aleksandar Markovic <<a href="mailto:aleksandar.qemu.devel@gmail.com">aleksandar.qemu.devel@gmail.<wbr>com</a>><br> -#<br> -# This program is free software: you can redistribute it and/or modify<br> -# it under the terms of the GNU General Public License as published by<br> -# the Free Software Foundation, either version 2 of the License, or<br> -# (at your option) any later version.<br> -#<br> -# This program is distributed in the hope that it will be useful,<br> -# but WITHOUT ANY WARRANTY; without even the implied warranty of<br> -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the<br> -# GNU General Public License for more details.<br> -#<br> -# You should have received a copy of the GNU General Public License<br> -# along with this program. If not, see <<a href="https://www.gnu.org/licenses/" target="_blank">https://www.gnu.org/licenses/</a><wbr>>.<br> +"""<br> +Print the top N most executed functions in QEMU using perf.<br> +<br> +Syntax:<br> +topN_perf.py [-h] [-n <number of displayed top functions>] -- \<br> + <qemu executable> [<qemu executable options>] \<br> + <target executable> [<target execurable options>]<br> +<br> +[-h] - Print the script arguments help message.<br> +[-n] - Specify the number of top functions to print.<br> + - If this flag is not specified, the tool defaults to 25.<br> +<br> +Example of usage:<br> +topN_perf.py -n 20 -- qemu-arm coulomb_double-arm<br> +<br> +This file is a part of the project "TCG Continuous Benchmarking".<br> +<br> +Copyright (C) 2020 Ahmed Karaman <<a href="mailto:ahmedkhaledkaraman@gmail.com">ahmedkhaledkaraman@gmail.com</a>><br> +Copyright (C) 2020 Aleksandar Markovic <<a href="mailto:aleksandar.qemu.devel@gmail.com">aleksandar.qemu.devel@gmail.<wbr>com</a>><br> +<br> +This program is free software: you can redistribute it and/or modify<br> +it under the terms of the GNU General Public License as published by<br> +the Free Software Foundation, either version 2 of the License, or<br> +(at your option) any later version.<br> +<br> +This program is distributed in the hope that it will be useful,<br> +but WITHOUT ANY WARRANTY; without even the implied warranty of<br> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the<br> +GNU General Public License for more details.<br> +<br> +You should have received a copy of the GNU General Public License<br> +along with this program. If not, see <<a href="https://www.gnu.org/licenses/" target="_blank">https://www.gnu.org/licenses/</a><wbr>>.<br> +"""<br> <br> import argparse<br> import os<br> import subprocess<br> import sys<br> +import tempfile<br> <br> <br> # Parse the command line arguments<br> -parser = argparse.ArgumentParser(<br> - usage='topN_perf.py [-h] [-n] <number of displayed top functions > -- '<br> +PARSER = argparse.ArgumentParser(<br> + usage='topN_perf.py [-h] [-n <number of displayed top functions>] -- '<br> '<qemu executable> [<qemu executable options>] '<br> '<target executable> [<target executable options>]')<br> <br> -parser.add_argument('-n', dest='top', type=int, default=25,<br> +PARSER.add_argument('-n', dest='top', type=int, default=25,<br> help='Specify the number of top functions to print.')<br> <br> -parser.add_argument('command'<wbr>, type=str, nargs='+', help=argparse.SUPPRESS)<br> +PARSER.add_argument('command'<wbr>, type=str, nargs='+', help=argparse.SUPPRESS)<br> <br> -args = parser.parse_args()<br> +ARGS = PARSER.parse_args()<br> <br> # Extract the needed variables from the args<br> -command = args.command<br> -top = args.top<br> +COMMAND = ARGS.command<br> +TOP = ARGS.top<br> <br> # Insure that perf is installed<br> -check_perf_presence = subprocess.run(["which", "perf"],<br> - stdout=subprocess.DEVNULL)<br> -if check_perf_presence.<wbr>returncode:<br> +CHECK_PERF_PRESENCE = subprocess.run(["which", "perf"],<br> + stdout=subprocess.DEVNULL,<br> + check=False)<br> +if CHECK_PERF_PRESENCE.<wbr>returncode:<br> sys.exit("Please install perf before running the script!")<br> <br> # Insure user has previllage to run perf<br> -check_perf_executability = subprocess.run(["perf", "stat", "ls", "/"],<br> +CHECK_PERF_EXECUTABILITY = subprocess.run(["perf", "stat", "ls", "/"],<br> stdout=subprocess.DEVNULL,<br> - stderr=subprocess.DEVNULL)<br> -if check_perf_executability.<wbr>returncode:<br> - sys.exit(<br> -"""<br> + stderr=subprocess.DEVNULL,<br> + check=False)<br> +if CHECK_PERF_EXECUTABILITY.<wbr>returncode:<br> + sys.exit("""<br> Error:<br> You may not have permission to collect stats.<br> <br> @@ -85,43 +90,42 @@ To make this setting permanent, edit /etc/sysctl.conf too, e.g.:<br> kernel.perf_event_paranoid = -1<br> <br> * Alternatively, you can run this script under sudo privileges.<br> -"""<br> -)<br> -<br> -# Run perf record<br> -perf_record = subprocess.run((["perf", "record", "--output=/tmp/perf.data"] +<br> - command),<br> - stdout=subprocess.DEVNULL,<br> - stderr=subprocess.PIPE)<br> -if perf_record.returncode:<br> - os.unlink('/tmp/perf.data')<br> - sys.exit(perf_record.stderr.<wbr>decode("utf-8"))<br> -<br> -# Save perf report output to /tmp/perf_report.out<br> -with open("/tmp/perf_report.out", "w") as output:<br> - perf_report = subprocess.run(<br> - ["perf", "report", "--input=/tmp/perf.data", "--stdio"],<br> - stdout=output,<br> - stderr=subprocess.PIPE)<br> - if perf_report.returncode:<br> - os.unlink('/tmp/perf.data')<br> - output.close()<br> - os.unlink('/tmp/perf_report.<wbr>out')<br> - sys.exit(perf_report.stderr.<wbr>decode("utf-8"))<br> -<br> -# Read the reported data to functions[]<br> -functions = []<br> -with open("/tmp/perf_report.out", "r") as data:<br> - # Only read lines that are not comments (comments start with #)<br> - # Only read lines that are not empty<br> - functions = [line for line in data.readlines() if line and line[0]<br> - != '#' and line[0] != "\n"]<br> -<br> -# Limit the number of top functions to "top"<br> -number_of_top_functions = top if len(functions) > top else len(functions)<br> -<br> -# Store the data of the top functions in top_functions[]<br> -top_functions = functions[:number_of_top_<wbr>functions]<br> +""")<br> +<br> +# Run perf and save all intermediate files in a temporary directory<br> +with tempfile.TemporaryDirectory() as tmpdir:<br> + RECORD_PATH = os.path.join(tmpdir, "record.data")<br> + REPORT_PATH = os.path.join(tmpdir, "report.txt")<br> +<br> + PERF_RECORD = subprocess.run((["perf", "record", "--output="+RECORD_PATH] +<br> + COMMAND),<br> + stdout=subprocess.DEVNULL,<br> + stderr=subprocess.PIPE,<br> + check=False)<br> + if PERF_RECORD.returncode:<br> + sys.exit(PERF_RECORD.stderr.<wbr>decode("utf-8"))<br> +<br> + with open(REPORT_PATH, "w") as output:<br> + PERF_REPORT = subprocess.run(<br> + ["perf", "report", "--input="+RECORD_PATH, "--stdio"],<br> + stdout=output,<br> + stderr=subprocess.PIPE,<br> + check=False)<br> + if PERF_REPORT.returncode:<br> + sys.exit(PERF_REPORT.stderr.<wbr>decode("utf-8"))<br> +<br> + # Save the reported data to FUNCTIONS[]<br> + with open(REPORT_PATH, "r") as data:<br> + # Only read lines that are not comments (comments start with #)<br> + # Only read lines that are not empty<br> + FUNCTIONS = [line for line in data.readlines() if line and<br> + line[0] != '#' and line[0] != "\n"]<br> +<br> +# Limit the number of top functions to "TOP"<br> +NO_TOP_FUNCTIONS = TOP if len(FUNCTIONS) > TOP else len(FUNCTIONS)<br> +<br> +# Store the data of the top functions in TOP_FUNCTIONS[]<br> +TOP_FUNCTIONS = FUNCTIONS[:NO_TOP_FUNCTIONS]<br> <br> # Print table header<br> print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.',<br> @@ -134,7 +138,7 @@ print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.',<br> '-' * 25))<br> <br> # Print top N functions<br> -for (index, function) in enumerate(top_functions, start=1):<br> +for (index, function) in enumerate(TOP_FUNCTIONS, start=1):<br> function_data = function.split()<br> function_percentage = function_data[0]<br> function_name = function_data[-1]<br> @@ -143,7 +147,3 @@ for (index, function) in enumerate(top_functions, start=1):<br> function_percentage,<br> function_name,<br> function_invoker))<br> -<br> -# Remove intermediate files<br> -os.unlink('/tmp/perf.data')<br> -os.unlink('/tmp/perf_report.<wbr>out')<br> -- <br> 2.17.1<br> <br> </blockquote>
On Friday, August 28, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com> wrote: > - Apply pylint and flake8 formatting rules to the script. > - Use 'tempfile' instead of '/tmp' for creating temporary files. > > Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com> > --- Hello, folks. This series seems forgotten. Can some of you perhaps take a look, review, and possibly integrate some patches in a pull request? Ahmed invested quite of time to improve the functionality and quality of scripts, and they are truly useful for developers interested in performance measurement. Thanks, Aleksandar > scripts/performance/topN_perf.py | 174 +++++++++++++++---------------- > 1 file changed, 87 insertions(+), 87 deletions(-) > > diff --git a/scripts/performance/topN_perf.py b/scripts/performance/topN_ > perf.py > index 07be195fc8..56b100da87 100755 > --- a/scripts/performance/topN_perf.py > +++ b/scripts/performance/topN_perf.py > @@ -1,72 +1,77 @@ > #!/usr/bin/env python3 > > -# Print the top N most executed functions in QEMU using perf. > -# Syntax: > -# topN_perf.py [-h] [-n] <number of displayed top functions> -- \ > -# <qemu executable> [<qemu executable options>] \ > -# <target executable> [<target execurable options>] > -# > -# [-h] - Print the script arguments help message. > -# [-n] - Specify the number of top functions to print. > -# - If this flag is not specified, the tool defaults to 25. > -# > -# Example of usage: > -# topN_perf.py -n 20 -- qemu-arm coulomb_double-arm > -# > -# 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/>. > +""" > +Print the top N most executed functions in QEMU using perf. > + > +Syntax: > +topN_perf.py [-h] [-n <number of displayed top functions>] -- \ > + <qemu executable> [<qemu executable options>] \ > + <target executable> [<target execurable options>] > + > +[-h] - Print the script arguments help message. > +[-n] - Specify the number of top functions to print. > + - If this flag is not specified, the tool defaults to 25. > + > +Example of usage: > +topN_perf.py -n 20 -- qemu-arm coulomb_double-arm > + > +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 argparse > import os > import subprocess > import sys > +import tempfile > > > # Parse the command line arguments > -parser = argparse.ArgumentParser( > - usage='topN_perf.py [-h] [-n] <number of displayed top functions > > -- ' > +PARSER = argparse.ArgumentParser( > + usage='topN_perf.py [-h] [-n <number of displayed top functions>] -- ' > '<qemu executable> [<qemu executable options>] ' > '<target executable> [<target executable options>]') > > -parser.add_argument('-n', dest='top', type=int, default=25, > +PARSER.add_argument('-n', dest='top', type=int, default=25, > help='Specify the number of top functions to print.') > > -parser.add_argument('command', type=str, nargs='+', > help=argparse.SUPPRESS) > +PARSER.add_argument('command', type=str, nargs='+', > help=argparse.SUPPRESS) > > -args = parser.parse_args() > +ARGS = PARSER.parse_args() > > # Extract the needed variables from the args > -command = args.command > -top = args.top > +COMMAND = ARGS.command > +TOP = ARGS.top > > # Insure that perf is installed > -check_perf_presence = subprocess.run(["which", "perf"], > - stdout=subprocess.DEVNULL) > -if check_perf_presence.returncode: > +CHECK_PERF_PRESENCE = subprocess.run(["which", "perf"], > + stdout=subprocess.DEVNULL, > + check=False) > +if CHECK_PERF_PRESENCE.returncode: > sys.exit("Please install perf before running the script!") > > # Insure user has previllage to run perf > -check_perf_executability = subprocess.run(["perf", "stat", "ls", "/"], > +CHECK_PERF_EXECUTABILITY = subprocess.run(["perf", "stat", "ls", "/"], > stdout=subprocess.DEVNULL, > - stderr=subprocess.DEVNULL) > -if check_perf_executability.returncode: > - sys.exit( > -""" > + stderr=subprocess.DEVNULL, > + check=False) > +if CHECK_PERF_EXECUTABILITY.returncode: > + sys.exit(""" > Error: > You may not have permission to collect stats. > > @@ -85,43 +90,42 @@ To make this setting permanent, edit /etc/sysctl.conf > too, e.g.: > kernel.perf_event_paranoid = -1 > > * Alternatively, you can run this script under sudo privileges. > -""" > -) > - > -# Run perf record > -perf_record = subprocess.run((["perf", "record", > "--output=/tmp/perf.data"] + > - command), > - stdout=subprocess.DEVNULL, > - stderr=subprocess.PIPE) > -if perf_record.returncode: > - os.unlink('/tmp/perf.data') > - sys.exit(perf_record.stderr.decode("utf-8")) > - > -# Save perf report output to /tmp/perf_report.out > -with open("/tmp/perf_report.out", "w") as output: > - perf_report = subprocess.run( > - ["perf", "report", "--input=/tmp/perf.data", "--stdio"], > - stdout=output, > - stderr=subprocess.PIPE) > - if perf_report.returncode: > - os.unlink('/tmp/perf.data') > - output.close() > - os.unlink('/tmp/perf_report.out') > - sys.exit(perf_report.stderr.decode("utf-8")) > - > -# Read the reported data to functions[] > -functions = [] > -with open("/tmp/perf_report.out", "r") as data: > - # Only read lines that are not comments (comments start with #) > - # Only read lines that are not empty > - functions = [line for line in data.readlines() if line and line[0] > - != '#' and line[0] != "\n"] > - > -# Limit the number of top functions to "top" > -number_of_top_functions = top if len(functions) > top else len(functions) > - > -# Store the data of the top functions in top_functions[] > -top_functions = functions[:number_of_top_functions] > +""") > + > +# Run perf and save all intermediate files in a temporary directory > +with tempfile.TemporaryDirectory() as tmpdir: > + RECORD_PATH = os.path.join(tmpdir, "record.data") > + REPORT_PATH = os.path.join(tmpdir, "report.txt") > + > + PERF_RECORD = subprocess.run((["perf", "record", > "--output="+RECORD_PATH] + > + COMMAND), > + stdout=subprocess.DEVNULL, > + stderr=subprocess.PIPE, > + check=False) > + if PERF_RECORD.returncode: > + sys.exit(PERF_RECORD.stderr.decode("utf-8")) > + > + with open(REPORT_PATH, "w") as output: > + PERF_REPORT = subprocess.run( > + ["perf", "report", "--input="+RECORD_PATH, "--stdio"], > + stdout=output, > + stderr=subprocess.PIPE, > + check=False) > + if PERF_REPORT.returncode: > + sys.exit(PERF_REPORT.stderr.decode("utf-8")) > + > + # Save the reported data to FUNCTIONS[] > + with open(REPORT_PATH, "r") as data: > + # Only read lines that are not comments (comments start with #) > + # Only read lines that are not empty > + FUNCTIONS = [line for line in data.readlines() if line and > + line[0] != '#' and line[0] != "\n"] > + > +# Limit the number of top functions to "TOP" > +NO_TOP_FUNCTIONS = TOP if len(FUNCTIONS) > TOP else len(FUNCTIONS) > + > +# Store the data of the top functions in TOP_FUNCTIONS[] > +TOP_FUNCTIONS = FUNCTIONS[:NO_TOP_FUNCTIONS] > > # Print table header > print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.', > @@ -134,7 +138,7 @@ print('{:>4} {:>10} {:<30} {}\n{} {} {} > {}'.format('No.', > '-' * 25)) > > # Print top N functions > -for (index, function) in enumerate(top_functions, start=1): > +for (index, function) in enumerate(TOP_FUNCTIONS, start=1): > function_data = function.split() > function_percentage = function_data[0] > function_name = function_data[-1] > @@ -143,7 +147,3 @@ for (index, function) in enumerate(top_functions, > start=1): > function_percentage, > function_name, > function_invoker)) > - > -# Remove intermediate files > -os.unlink('/tmp/perf.data') > -os.unlink('/tmp/perf_report.out') > -- > 2.17.1 > > > <br><br>On Friday, August 28, 2020, Ahmed Karaman <<a href="mailto:ahmedkhaledkaraman@gmail.com">ahmedkhaledkaraman@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">- Apply pylint and flake8 formatting rules to the script.<br> - Use 'tempfile' instead of '/tmp' for creating temporary files.<br> <br> Signed-off-by: Ahmed Karaman <<a href="mailto:ahmedkhaledkaraman@gmail.com">ahmedkhaledkaraman@gmail.com</a>><br> ---</blockquote><div><br></div><div>Hello, folks.</div><div><br></div><div>This series seems forgotten. Can some of you perhaps take a look, review, and possibly integrate some patches in a pull request?</div><div><br></div><div>Ahmed invested quite of time to improve the functionality and quality of scripts, and they are truly useful for developers interested in performance measurement.</div><div><br></div><div>Thanks,</div><div>Aleksandar</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> scripts/performance/topN_perf.<wbr>py | 174 +++++++++++++++---------------<wbr>-<br> 1 file changed, 87 insertions(+), 87 deletions(-)<br> <br> diff --git a/scripts/performance/topN_<wbr>perf.py b/scripts/performance/topN_<wbr>perf.py<br> index 07be195fc8..56b100da87 100755<br> --- a/scripts/performance/topN_<wbr>perf.py<br> +++ b/scripts/performance/topN_<wbr>perf.py<br> @@ -1,72 +1,77 @@<br> #!/usr/bin/env python3<br> <br> -# Print the top N most executed functions in QEMU using perf.<br> -# Syntax:<br> -# topN_perf.py [-h] [-n] <number of displayed top functions> -- \<br> -# <qemu executable> [<qemu executable options>] \<br> -# <target executable> [<target execurable options>]<br> -#<br> -# [-h] - Print the script arguments help message.<br> -# [-n] - Specify the number of top functions to print.<br> -# - If this flag is not specified, the tool defaults to 25.<br> -#<br> -# Example of usage:<br> -# topN_perf.py -n 20 -- qemu-arm coulomb_double-arm<br> -#<br> -# This file is a part of the project "TCG Continuous Benchmarking".<br> -#<br> -# Copyright (C) 2020 Ahmed Karaman <<a href="mailto:ahmedkhaledkaraman@gmail.com">ahmedkhaledkaraman@gmail.com</a>><br> -# Copyright (C) 2020 Aleksandar Markovic <<a href="mailto:aleksandar.qemu.devel@gmail.com">aleksandar.qemu.devel@gmail.<wbr>com</a>><br> -#<br> -# This program is free software: you can redistribute it and/or modify<br> -# it under the terms of the GNU General Public License as published by<br> -# the Free Software Foundation, either version 2 of the License, or<br> -# (at your option) any later version.<br> -#<br> -# This program is distributed in the hope that it will be useful,<br> -# but WITHOUT ANY WARRANTY; without even the implied warranty of<br> -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the<br> -# GNU General Public License for more details.<br> -#<br> -# You should have received a copy of the GNU General Public License<br> -# along with this program. If not, see <<a href="https://www.gnu.org/licenses/" target="_blank">https://www.gnu.org/licenses/</a><wbr>>.<br> +"""<br> +Print the top N most executed functions in QEMU using perf.<br> +<br> +Syntax:<br> +topN_perf.py [-h] [-n <number of displayed top functions>] -- \<br> + <qemu executable> [<qemu executable options>] \<br> + <target executable> [<target execurable options>]<br> +<br> +[-h] - Print the script arguments help message.<br> +[-n] - Specify the number of top functions to print.<br> + - If this flag is not specified, the tool defaults to 25.<br> +<br> +Example of usage:<br> +topN_perf.py -n 20 -- qemu-arm coulomb_double-arm<br> +<br> +This file is a part of the project "TCG Continuous Benchmarking".<br> +<br> +Copyright (C) 2020 Ahmed Karaman <<a href="mailto:ahmedkhaledkaraman@gmail.com">ahmedkhaledkaraman@gmail.com</a>><br> +Copyright (C) 2020 Aleksandar Markovic <<a href="mailto:aleksandar.qemu.devel@gmail.com">aleksandar.qemu.devel@gmail.<wbr>com</a>><br> +<br> +This program is free software: you can redistribute it and/or modify<br> +it under the terms of the GNU General Public License as published by<br> +the Free Software Foundation, either version 2 of the License, or<br> +(at your option) any later version.<br> +<br> +This program is distributed in the hope that it will be useful,<br> +but WITHOUT ANY WARRANTY; without even the implied warranty of<br> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the<br> +GNU General Public License for more details.<br> +<br> +You should have received a copy of the GNU General Public License<br> +along with this program. If not, see <<a href="https://www.gnu.org/licenses/" target="_blank">https://www.gnu.org/licenses/</a><wbr>>.<br> +"""<br> <br> import argparse<br> import os<br> import subprocess<br> import sys<br> +import tempfile<br> <br> <br> # Parse the command line arguments<br> -parser = argparse.ArgumentParser(<br> - usage='topN_perf.py [-h] [-n] <number of displayed top functions > -- '<br> +PARSER = argparse.ArgumentParser(<br> + usage='topN_perf.py [-h] [-n <number of displayed top functions>] -- '<br> '<qemu executable> [<qemu executable options>] '<br> '<target executable> [<target executable options>]')<br> <br> -parser.add_argument('-n', dest='top', type=int, default=25,<br> +PARSER.add_argument('-n', dest='top', type=int, default=25,<br> help='Specify the number of top functions to print.')<br> <br> -parser.add_argument('command'<wbr>, type=str, nargs='+', help=argparse.SUPPRESS)<br> +PARSER.add_argument('command'<wbr>, type=str, nargs='+', help=argparse.SUPPRESS)<br> <br> -args = parser.parse_args()<br> +ARGS = PARSER.parse_args()<br> <br> # Extract the needed variables from the args<br> -command = args.command<br> -top = args.top<br> +COMMAND = ARGS.command<br> +TOP = ARGS.top<br> <br> # Insure that perf is installed<br> -check_perf_presence = subprocess.run(["which", "perf"],<br> - stdout=subprocess.DEVNULL)<br> -if check_perf_presence.<wbr>returncode:<br> +CHECK_PERF_PRESENCE = subprocess.run(["which", "perf"],<br> + stdout=subprocess.DEVNULL,<br> + check=False)<br> +if CHECK_PERF_PRESENCE.<wbr>returncode:<br> sys.exit("Please install perf before running the script!")<br> <br> # Insure user has previllage to run perf<br> -check_perf_executability = subprocess.run(["perf", "stat", "ls", "/"],<br> +CHECK_PERF_EXECUTABILITY = subprocess.run(["perf", "stat", "ls", "/"],<br> stdout=subprocess.DEVNULL,<br> - stderr=subprocess.DEVNULL)<br> -if check_perf_executability.<wbr>returncode:<br> - sys.exit(<br> -"""<br> + stderr=subprocess.DEVNULL,<br> + check=False)<br> +if CHECK_PERF_EXECUTABILITY.<wbr>returncode:<br> + sys.exit("""<br> Error:<br> You may not have permission to collect stats.<br> <br> @@ -85,43 +90,42 @@ To make this setting permanent, edit /etc/sysctl.conf too, e.g.:<br> kernel.perf_event_paranoid = -1<br> <br> * Alternatively, you can run this script under sudo privileges.<br> -"""<br> -)<br> -<br> -# Run perf record<br> -perf_record = subprocess.run((["perf", "record", "--output=/tmp/perf.data"] +<br> - command),<br> - stdout=subprocess.DEVNULL,<br> - stderr=subprocess.PIPE)<br> -if perf_record.returncode:<br> - os.unlink('/tmp/perf.data')<br> - sys.exit(perf_record.stderr.<wbr>decode("utf-8"))<br> -<br> -# Save perf report output to /tmp/perf_report.out<br> -with open("/tmp/perf_report.out", "w") as output:<br> - perf_report = subprocess.run(<br> - ["perf", "report", "--input=/tmp/perf.data", "--stdio"],<br> - stdout=output,<br> - stderr=subprocess.PIPE)<br> - if perf_report.returncode:<br> - os.unlink('/tmp/perf.data')<br> - output.close()<br> - os.unlink('/tmp/perf_report.<wbr>out')<br> - sys.exit(perf_report.stderr.<wbr>decode("utf-8"))<br> -<br> -# Read the reported data to functions[]<br> -functions = []<br> -with open("/tmp/perf_report.out", "r") as data:<br> - # Only read lines that are not comments (comments start with #)<br> - # Only read lines that are not empty<br> - functions = [line for line in data.readlines() if line and line[0]<br> - != '#' and line[0] != "\n"]<br> -<br> -# Limit the number of top functions to "top"<br> -number_of_top_functions = top if len(functions) > top else len(functions)<br> -<br> -# Store the data of the top functions in top_functions[]<br> -top_functions = functions[:number_of_top_<wbr>functions]<br> +""")<br> +<br> +# Run perf and save all intermediate files in a temporary directory<br> +with tempfile.TemporaryDirectory() as tmpdir:<br> + RECORD_PATH = os.path.join(tmpdir, "record.data")<br> + REPORT_PATH = os.path.join(tmpdir, "report.txt")<br> +<br> + PERF_RECORD = subprocess.run((["perf", "record", "--output="+RECORD_PATH] +<br> + COMMAND),<br> + stdout=subprocess.DEVNULL,<br> + stderr=subprocess.PIPE,<br> + check=False)<br> + if PERF_RECORD.returncode:<br> + sys.exit(PERF_RECORD.stderr.<wbr>decode("utf-8"))<br> +<br> + with open(REPORT_PATH, "w") as output:<br> + PERF_REPORT = subprocess.run(<br> + ["perf", "report", "--input="+RECORD_PATH, "--stdio"],<br> + stdout=output,<br> + stderr=subprocess.PIPE,<br> + check=False)<br> + if PERF_REPORT.returncode:<br> + sys.exit(PERF_REPORT.stderr.<wbr>decode("utf-8"))<br> +<br> + # Save the reported data to FUNCTIONS[]<br> + with open(REPORT_PATH, "r") as data:<br> + # Only read lines that are not comments (comments start with #)<br> + # Only read lines that are not empty<br> + FUNCTIONS = [line for line in data.readlines() if line and<br> + line[0] != '#' and line[0] != "\n"]<br> +<br> +# Limit the number of top functions to "TOP"<br> +NO_TOP_FUNCTIONS = TOP if len(FUNCTIONS) > TOP else len(FUNCTIONS)<br> +<br> +# Store the data of the top functions in TOP_FUNCTIONS[]<br> +TOP_FUNCTIONS = FUNCTIONS[:NO_TOP_FUNCTIONS]<br> <br> # Print table header<br> print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.',<br> @@ -134,7 +138,7 @@ print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.',<br> '-' * 25))<br> <br> # Print top N functions<br> -for (index, function) in enumerate(top_functions, start=1):<br> +for (index, function) in enumerate(TOP_FUNCTIONS, start=1):<br> function_data = function.split()<br> function_percentage = function_data[0]<br> function_name = function_data[-1]<br> @@ -143,7 +147,3 @@ for (index, function) in enumerate(top_functions, start=1):<br> function_percentage,<br> function_name,<br> function_invoker))<br> -<br> -# Remove intermediate files<br> -os.unlink('/tmp/perf.data')<br> -os.unlink('/tmp/perf_report.<wbr>out')<br> -- <br> 2.17.1<br> <br> <br> </blockquote>
Hi Aleksandar, (extending the Cc list to broader audience) On 9/18/20 10:33 PM, Aleksandar Markovic wrote: > > On Friday, August 28, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com > <mailto:ahmedkhaledkaraman@gmail.com>> wrote: > > - Apply pylint and flake8 formatting rules to the script. > - Use 'tempfile' instead of '/tmp' for creating temporary files. > > --- > > Hello, folks. > > This series seems forgotten. Can some of you perhaps take a look, > review, and possibly integrate some patches in a pull request? > > Ahmed invested quite of time to improve the functionality and quality of > scripts, and they are truly useful for developers interested in > performance measurement. The Python patches are indeed being merged slowly, but are not forgotten :) Eduardo sent just a pull request *yesterday* for patches he had queued *before* QEMU 5.1, that is more than 1 year ago! https://www.mail-archive.com/qemu-devel@nongnu.org/msg742228.html I hope he will be able to process the other Python patches sent the last 12 months. He raised the problem few month ago saying he was overwhelmed and was looking for help from the community. Cleber helped a pair of times, I helped once, but then nobody popped up volunteering. I agree this is a community problem, nobody wants to become the de-facto Python maintainer. Current maintainers are already busy solving problem with low-level languages such C. As a project, QEMU is evolving, using more and more Python, switched to Meson, we might have Rust code too. Learning that for current maintainers takes time. I guess we lack new contributors/maintainers with interest in Python & QEMU. This is my simple/rough analysis, as John had the same problem 2/3 months ago, his patches were on the list unreviewed for various weeks. Same problem with Avocado patches, Lukas sent a series a bit before Ahmed and it is still unreviewed: https://www.mail-archive.com/qemu-devel@nongnu.org/msg737272.html Alex Bennée recently said: "review time is the currency of qemu-devel ;-)" We might be failing at explaining new contributors the importance of peer-review, as this helps a lot maintainers. It is described in the wiki but maybe new contributors don't read it, we don't remember them to: https://wiki.qemu.org/Contribute/SubmitAPatch#Participating_in_Code_Review and: https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor My 2 cents... Regards, Phil. > > Thanks, > Aleksandar
On 9/19/20 7:17 AM, Philippe Mathieu-Daudé wrote: > Hi Aleksandar, > > (extending the Cc list to broader audience) > > On 9/18/20 10:33 PM, Aleksandar Markovic wrote: >> >> On Friday, August 28, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com >> <mailto:ahmedkhaledkaraman@gmail.com>> wrote: >> >> - Apply pylint and flake8 formatting rules to the script. >> - Use 'tempfile' instead of '/tmp' for creating temporary files. >> >> --- >> >> Hello, folks. >> >> This series seems forgotten. Can some of you perhaps take a look, >> review, and possibly integrate some patches in a pull request? >> >> Ahmed invested quite of time to improve the functionality and quality of >> scripts, and they are truly useful for developers interested in >> performance measurement. > Hi, I will add it to my queue. I intended to take over Python maintenance but I have been busy refactoring the QAPI python code and haven't been reading my mail as regularly. > The Python patches are indeed being merged slowly, but are not > forgotten :) Eduardo sent just a pull request *yesterday* for > patches he had queued *before* QEMU 5.1, that is more than 1 year > ago! > https://www.mail-archive.com/qemu-devel@nongnu.org/msg742228.html > > I hope he will be able to process the other Python patches sent > the last 12 months. He raised the problem few month ago saying he > was overwhelmed and was looking for help from the community. > Cleber helped a pair of times, I helped once, but then nobody > popped up volunteering. > > I agree this is a community problem, nobody wants to become the > de-facto Python maintainer. Current maintainers are already busy > solving problem with low-level languages such C. > As a project, QEMU is evolving, using more and more Python, switched > to Meson, we might have Rust code too. Learning that for current > maintainers takes time. I guess we lack new contributors/maintainers > with interest in Python & QEMU. > I'm volunteering, since I am doing so much work in Python. I could use a dedicated reviewer to help me, however. I prefer a maintainer policy where all patches get at least ONE review by someone other than the primary author. In the case that I am writing so much Python, I still need a co-maintainer to help review *my* patches. > This is my simple/rough analysis, as John had the same problem > 2/3 months ago, his patches were on the list unreviewed for various > weeks. Same problem with Avocado patches, Lukas sent a series a bit > before Ahmed and it is still unreviewed: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg737272.html > > Alex Bennée recently said: > > "review time is the currency of qemu-devel ;-)" > > We might be failing at explaining new contributors the importance > of peer-review, as this helps a lot maintainers. It is described > in the wiki but maybe new contributors don't read it, we don't > remember them to: > https://wiki.qemu.org/Contribute/SubmitAPatch#Participating_in_Code_Review > and: > https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor > > My 2 cents... > > Regards, > > Phil. > >> >> Thanks, >> Aleksandar >
On Sat, Sep 19, 2020 at 01:17:06PM +0200, Philippe Mathieu-Daudé wrote: > Hi Aleksandar, > > (extending the Cc list to broader audience) > > On 9/18/20 10:33 PM, Aleksandar Markovic wrote: > > > > On Friday, August 28, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com > > <mailto:ahmedkhaledkaraman@gmail.com>> wrote: > > > > - Apply pylint and flake8 formatting rules to the script. > > - Use 'tempfile' instead of '/tmp' for creating temporary files. > > > > --- > > > > Hello, folks. > > > > This series seems forgotten. Can some of you perhaps take a look, > > review, and possibly integrate some patches in a pull request? > > > > Ahmed invested quite of time to improve the functionality and quality of > > scripts, and they are truly useful for developers interested in > > performance measurement. > > The Python patches are indeed being merged slowly, but are not > forgotten :) Eduardo sent just a pull request *yesterday* for > patches he had queued *before* QEMU 5.1, that is more than 1 year > ago! > https://www.mail-archive.com/qemu-devel@nongnu.org/msg742228.html > > I hope he will be able to process the other Python patches sent > the last 12 months. He raised the problem few month ago saying he > was overwhelmed and was looking for help from the community. > Cleber helped a pair of times, I helped once, but then nobody > popped up volunteering. The patches were from July this year. But, yeah, I'm not being able to keep up with the maintainer role for Python code, and plan to change my M: line to R: in the "Python scripts" section. > > I agree this is a community problem, nobody wants to become the > de-facto Python maintainer. Current maintainers are already busy > solving problem with low-level languages such C. > As a project, QEMU is evolving, using more and more Python, switched > to Meson, we might have Rust code too. Learning that for current > maintainers takes time. I guess we lack new contributors/maintainers > with interest in Python & QEMU. I believe the solution here is to not have a single owner for all Python code, but owners for specific areas. We already have 36 F: entries in MAINTAINERS for files inside `scripts/`, and each of those maintainers can send pull requests containing Python code in the areas they maintain. Including Ahmed, who is already listed as the maintainer of `scripts/performance/`. > > This is my simple/rough analysis, as John had the same problem > 2/3 months ago, his patches were on the list unreviewed for various > weeks. Same problem with Avocado patches, Lukas sent a series a bit > before Ahmed and it is still unreviewed: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg737272.html > > Alex Bennée recently said: > > "review time is the currency of qemu-devel ;-)" > > We might be failing at explaining new contributors the importance > of peer-review, as this helps a lot maintainers. It is described > in the wiki but maybe new contributors don't read it, we don't > remember them to: > https://wiki.qemu.org/Contribute/SubmitAPatch#Participating_in_Code_Review > and: > https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor > > My 2 cents... > > Regards, > > Phil. > > > > > Thanks, > > Aleksandar > -- Eduardo
On Sat, Sep 19, 2020 at 01:17:06PM +0200, Philippe Mathieu-Daudé wrote: > Hi Aleksandar, > > (extending the Cc list to broader audience) > > On 9/18/20 10:33 PM, Aleksandar Markovic wrote: > > > > On Friday, August 28, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com > > <mailto:ahmedkhaledkaraman@gmail.com>> wrote: > > > > - Apply pylint and flake8 formatting rules to the script. > > - Use 'tempfile' instead of '/tmp' for creating temporary files. > > > > --- > > > > Hello, folks. > > > > This series seems forgotten. Can some of you perhaps take a look, > > review, and possibly integrate some patches in a pull request? > > > > Ahmed invested quite of time to improve the functionality and quality of > > scripts, and they are truly useful for developers interested in > > performance measurement. > > The Python patches are indeed being merged slowly, but are not > forgotten :) Eduardo sent just a pull request *yesterday* for > patches he had queued *before* QEMU 5.1, that is more than 1 year > ago! > https://www.mail-archive.com/qemu-devel@nongnu.org/msg742228.html > > I hope he will be able to process the other Python patches sent > the last 12 months. He raised the problem few month ago saying he > was overwhelmed and was looking for help from the community. > Cleber helped a pair of times, I helped once, but then nobody > popped up volunteering. > Yes, Python patches are not forgotten... they have been haunting me every day and night for the last few months. I have a *lot* of blame to take here, so my sincere apologies. One word of hope is that my resources (which were exhausted during the last months towards the Avocado 82.0 LTS release, released last week) are now freed. And *a lot* of the Avocado features were eyeing QEMU, so it's just natural that I will (because I want and have to) get back to being more active and responsive. > I agree this is a community problem, nobody wants to become the > de-facto Python maintainer. Current maintainers are already busy > solving problem with low-level languages such C. > As a project, QEMU is evolving, using more and more Python, switched > to Meson, we might have Rust code too. Learning that for current > maintainers takes time. I guess we lack new contributors/maintainers > with interest in Python & QEMU. > > This is my simple/rough analysis, as John had the same problem > 2/3 months ago, his patches were on the list unreviewed for various > weeks. Same problem with Avocado patches, Lukas sent a series a bit > before Ahmed and it is still unreviewed: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg737272.html > ACK. This is a good real example of my previous abstract explanation above. So, besides me getting back with more resources to review and maintain the Python pathes, I have also requested Willian Rampazzo (thanks!) to keep an eye on those patches and help with reviews. > Alex Bennée recently said: > > "review time is the currency of qemu-devel ;-)" > > We might be failing at explaining new contributors the importance > of peer-review, as this helps a lot maintainers. It is described > in the wiki but maybe new contributors don't read it, we don't > remember them to: > https://wiki.qemu.org/Contribute/SubmitAPatch#Participating_in_Code_Review > and: > https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor > > My 2 cents... > > Regards, > > Phil. > So while more help is always welcome, I do sincerely hope with myself and Willian back into the loop, the Python patches won't lag behind anymore. And thanks for raising the awareness of the issue Phil! - Cleber. > > > > Thanks, > > Aleksandar >
I realize this review comes well after you are no longer being paid to work on this, so I am offering my time to help polish your patches if you would like. On 8/28/20 6:40 AM, Ahmed Karaman wrote: > - Apply pylint and flake8 formatting rules to the script. > - Use 'tempfile' instead of '/tmp' for creating temporary files. > I had meant to maybe consider using some helper functions so that you didn't need to rename the globals, for instance: > Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com> > --- > scripts/performance/topN_perf.py | 174 +++++++++++++++---------------- > 1 file changed, 87 insertions(+), 87 deletions(-) > > diff --git a/scripts/performance/topN_perf.py b/scripts/performance/topN_perf.py > index 07be195fc8..56b100da87 100755 > --- a/scripts/performance/topN_perf.py > +++ b/scripts/performance/topN_perf.py > @@ -1,72 +1,77 @@ > #!/usr/bin/env python3 > > -# Print the top N most executed functions in QEMU using perf. > -# Syntax: > -# topN_perf.py [-h] [-n] <number of displayed top functions> -- \ > -# <qemu executable> [<qemu executable options>] \ > -# <target executable> [<target execurable options>] > -# > -# [-h] - Print the script arguments help message. > -# [-n] - Specify the number of top functions to print. > -# - If this flag is not specified, the tool defaults to 25. > -# > -# Example of usage: > -# topN_perf.py -n 20 -- qemu-arm coulomb_double-arm > -# > -# 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/>. > +""" > +Print the top N most executed functions in QEMU using perf. > + > +Syntax: > +topN_perf.py [-h] [-n <number of displayed top functions>] -- \ > + <qemu executable> [<qemu executable options>] \ > + <target executable> [<target execurable options>] > + > +[-h] - Print the script arguments help message. > +[-n] - Specify the number of top functions to print. > + - If this flag is not specified, the tool defaults to 25. > + > +Example of usage: > +topN_perf.py -n 20 -- qemu-arm coulomb_double-arm > + Based on discussion we've had upstream since you sent this, I think we will be keeping license and authorship information out of the docstrings, so this part can stay a comment. > +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 argparse > import os > import subprocess > import sys > +import tempfile > > > # Parse the command line arguments > -parser = argparse.ArgumentParser( > - usage='topN_perf.py [-h] [-n] <number of displayed top functions > -- ' > +PARSER = argparse.ArgumentParser( > + usage='topN_perf.py [-h] [-n <number of displayed top functions>] -- ' > '<qemu executable> [<qemu executable options>] ' > '<target executable> [<target executable options>]') > This is a little odd; generally we can avoid having such globals by making a main() function that defines a parser as a local instead. e.g., def main(): parser = ... parser.add_argument(...) args = parser.parse_args() ... return 0 if __name__ == '__main__': sys.exit(main()) > -parser.add_argument('-n', dest='top', type=int, default=25, > +PARSER.add_argument('-n', dest='top', type=int, default=25, > help='Specify the number of top functions to print.') > > -parser.add_argument('command', type=str, nargs='+', help=argparse.SUPPRESS) > +PARSER.add_argument('command', type=str, nargs='+', help=argparse.SUPPRESS) > > -args = parser.parse_args() > +ARGS = PARSER.parse_args() > > # Extract the needed variables from the args > -command = args.command > -top = args.top > +COMMAND = ARGS.command > +TOP = ARGS.top > > # Insure that perf is installed > -check_perf_presence = subprocess.run(["which", "perf"], > - stdout=subprocess.DEVNULL) > -if check_perf_presence.returncode: > +CHECK_PERF_PRESENCE = subprocess.run(["which", "perf"], > + stdout=subprocess.DEVNULL, > + check=False) > +if CHECK_PERF_PRESENCE.returncode: > sys.exit("Please install perf before running the script!") > > # Insure user has previllage to run perf > -check_perf_executability = subprocess.run(["perf", "stat", "ls", "/"], > +CHECK_PERF_EXECUTABILITY = subprocess.run(["perf", "stat", "ls", "/"], > stdout=subprocess.DEVNULL, > - stderr=subprocess.DEVNULL) > -if check_perf_executability.returncode: > - sys.exit( > -""" > + stderr=subprocess.DEVNULL, > + check=False) > +if CHECK_PERF_EXECUTABILITY.returncode: > + sys.exit(""" > Error: > You may not have permission to collect stats. > > @@ -85,43 +90,42 @@ To make this setting permanent, edit /etc/sysctl.conf too, e.g.: > kernel.perf_event_paranoid = -1 > > * Alternatively, you can run this script under sudo privileges. > -""" > -) > - > -# Run perf record > -perf_record = subprocess.run((["perf", "record", "--output=/tmp/perf.data"] + > - command), > - stdout=subprocess.DEVNULL, > - stderr=subprocess.PIPE) > -if perf_record.returncode: > - os.unlink('/tmp/perf.data') > - sys.exit(perf_record.stderr.decode("utf-8")) > - > -# Save perf report output to /tmp/perf_report.out > -with open("/tmp/perf_report.out", "w") as output: > - perf_report = subprocess.run( > - ["perf", "report", "--input=/tmp/perf.data", "--stdio"], > - stdout=output, > - stderr=subprocess.PIPE) > - if perf_report.returncode: > - os.unlink('/tmp/perf.data') > - output.close() > - os.unlink('/tmp/perf_report.out') > - sys.exit(perf_report.stderr.decode("utf-8")) > - > -# Read the reported data to functions[] > -functions = [] > -with open("/tmp/perf_report.out", "r") as data: > - # Only read lines that are not comments (comments start with #) > - # Only read lines that are not empty > - functions = [line for line in data.readlines() if line and line[0] > - != '#' and line[0] != "\n"] > - > -# Limit the number of top functions to "top" > -number_of_top_functions = top if len(functions) > top else len(functions) > - > -# Store the data of the top functions in top_functions[] > -top_functions = functions[:number_of_top_functions] > +""") > + > +# Run perf and save all intermediate files in a temporary directory > +with tempfile.TemporaryDirectory() as tmpdir: > + RECORD_PATH = os.path.join(tmpdir, "record.data") > + REPORT_PATH = os.path.join(tmpdir, "report.txt") > + > + PERF_RECORD = subprocess.run((["perf", "record", "--output="+RECORD_PATH] + > + COMMAND), > + stdout=subprocess.DEVNULL, > + stderr=subprocess.PIPE, > + check=False) > + if PERF_RECORD.returncode: > + sys.exit(PERF_RECORD.stderr.decode("utf-8")) > + > + with open(REPORT_PATH, "w") as output: > + PERF_REPORT = subprocess.run( > + ["perf", "report", "--input="+RECORD_PATH, "--stdio"], > + stdout=output, > + stderr=subprocess.PIPE, > + check=False) > + if PERF_REPORT.returncode: > + sys.exit(PERF_REPORT.stderr.decode("utf-8")) > + > + # Save the reported data to FUNCTIONS[] > + with open(REPORT_PATH, "r") as data: > + # Only read lines that are not comments (comments start with #) > + # Only read lines that are not empty > + FUNCTIONS = [line for line in data.readlines() if line and > + line[0] != '#' and line[0] != "\n"] > + > +# Limit the number of top functions to "TOP" > +NO_TOP_FUNCTIONS = TOP if len(FUNCTIONS) > TOP else len(FUNCTIONS) > + > +# Store the data of the top functions in TOP_FUNCTIONS[] > +TOP_FUNCTIONS = FUNCTIONS[:NO_TOP_FUNCTIONS] > > # Print table header > print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.', > @@ -134,7 +138,7 @@ print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.', > '-' * 25)) > > # Print top N functions > -for (index, function) in enumerate(top_functions, start=1): > +for (index, function) in enumerate(TOP_FUNCTIONS, start=1): > function_data = function.split() > function_percentage = function_data[0] > function_name = function_data[-1] > @@ -143,7 +147,3 @@ for (index, function) in enumerate(top_functions, start=1): > function_percentage, > function_name, > function_invoker)) > - > -# Remove intermediate files > -os.unlink('/tmp/perf.data') > -os.unlink('/tmp/perf_report.out') >
On 10/1/20 4:41 PM, John Snow wrote: > I realize this review comes well after you are no longer being paid to > work on this, so I am offering my time to help polish your patches if > you would like. Actually, I see now that you are adding your name to the MAINTAINERS file here, so I suspect you probably rather want to be more involved than not. I cleaned up patch 1/9 provisionally with my own style preferences, but it's all just style stuff, and it's mostly things I wouldn't actually require you to do (...I went way overboard.) https://gitlab.com/jsnow/qemu/-/commit/c66a4a6ca8ccc3d406b92796935f92057bf1e48d What I'd recommend for your cleanup is actually *much* simpler; Use pylint 2.6.0 and flake8 3.8.3: > pip3 install --user pylint==2.6.0 flake8==3.8.3 flake8's default settings should be pretty good, but pylint has a lot of warnings you can ignore. In particular, it's OK to use script-style python (Scripts with a #!/usr/bin/env python3, and where you do not use python functions to avoid side-effects that occur on 'import'.) In this case, IGNORE any of pylint's warnings telling you that you have too many lines, that you need to UPPERCASE variable names, etc. It just hurts readability here. So I'd actually ask that you revise these patches to remove all of the UPPERCASE variable names, and then check your code with these: flake8 topN_perf.py pylint --disable=invalid-name topN_perf.py Use your best judgment -- If something seems like it looks worse, it probably is. If in doubt, please reach out and ask. --js
diff --git a/scripts/performance/topN_perf.py b/scripts/performance/topN_perf.py index 07be195fc8..56b100da87 100755 --- a/scripts/performance/topN_perf.py +++ b/scripts/performance/topN_perf.py @@ -1,72 +1,77 @@ #!/usr/bin/env python3 -# Print the top N most executed functions in QEMU using perf. -# Syntax: -# topN_perf.py [-h] [-n] <number of displayed top functions> -- \ -# <qemu executable> [<qemu executable options>] \ -# <target executable> [<target execurable options>] -# -# [-h] - Print the script arguments help message. -# [-n] - Specify the number of top functions to print. -# - If this flag is not specified, the tool defaults to 25. -# -# Example of usage: -# topN_perf.py -n 20 -- qemu-arm coulomb_double-arm -# -# 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/>. +""" +Print the top N most executed functions in QEMU using perf. + +Syntax: +topN_perf.py [-h] [-n <number of displayed top functions>] -- \ + <qemu executable> [<qemu executable options>] \ + <target executable> [<target execurable options>] + +[-h] - Print the script arguments help message. +[-n] - Specify the number of top functions to print. + - If this flag is not specified, the tool defaults to 25. + +Example of usage: +topN_perf.py -n 20 -- qemu-arm coulomb_double-arm + +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 argparse import os import subprocess import sys +import tempfile # Parse the command line arguments -parser = argparse.ArgumentParser( - usage='topN_perf.py [-h] [-n] <number of displayed top functions > -- ' +PARSER = argparse.ArgumentParser( + usage='topN_perf.py [-h] [-n <number of displayed top functions>] -- ' '<qemu executable> [<qemu executable options>] ' '<target executable> [<target executable options>]') -parser.add_argument('-n', dest='top', type=int, default=25, +PARSER.add_argument('-n', dest='top', type=int, default=25, help='Specify the number of top functions to print.') -parser.add_argument('command', type=str, nargs='+', help=argparse.SUPPRESS) +PARSER.add_argument('command', type=str, nargs='+', help=argparse.SUPPRESS) -args = parser.parse_args() +ARGS = PARSER.parse_args() # Extract the needed variables from the args -command = args.command -top = args.top +COMMAND = ARGS.command +TOP = ARGS.top # Insure that perf is installed -check_perf_presence = subprocess.run(["which", "perf"], - stdout=subprocess.DEVNULL) -if check_perf_presence.returncode: +CHECK_PERF_PRESENCE = subprocess.run(["which", "perf"], + stdout=subprocess.DEVNULL, + check=False) +if CHECK_PERF_PRESENCE.returncode: sys.exit("Please install perf before running the script!") # Insure user has previllage to run perf -check_perf_executability = subprocess.run(["perf", "stat", "ls", "/"], +CHECK_PERF_EXECUTABILITY = subprocess.run(["perf", "stat", "ls", "/"], stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL) -if check_perf_executability.returncode: - sys.exit( -""" + stderr=subprocess.DEVNULL, + check=False) +if CHECK_PERF_EXECUTABILITY.returncode: + sys.exit(""" Error: You may not have permission to collect stats. @@ -85,43 +90,42 @@ To make this setting permanent, edit /etc/sysctl.conf too, e.g.: kernel.perf_event_paranoid = -1 * Alternatively, you can run this script under sudo privileges. -""" -) - -# Run perf record -perf_record = subprocess.run((["perf", "record", "--output=/tmp/perf.data"] + - command), - stdout=subprocess.DEVNULL, - stderr=subprocess.PIPE) -if perf_record.returncode: - os.unlink('/tmp/perf.data') - sys.exit(perf_record.stderr.decode("utf-8")) - -# Save perf report output to /tmp/perf_report.out -with open("/tmp/perf_report.out", "w") as output: - perf_report = subprocess.run( - ["perf", "report", "--input=/tmp/perf.data", "--stdio"], - stdout=output, - stderr=subprocess.PIPE) - if perf_report.returncode: - os.unlink('/tmp/perf.data') - output.close() - os.unlink('/tmp/perf_report.out') - sys.exit(perf_report.stderr.decode("utf-8")) - -# Read the reported data to functions[] -functions = [] -with open("/tmp/perf_report.out", "r") as data: - # Only read lines that are not comments (comments start with #) - # Only read lines that are not empty - functions = [line for line in data.readlines() if line and line[0] - != '#' and line[0] != "\n"] - -# Limit the number of top functions to "top" -number_of_top_functions = top if len(functions) > top else len(functions) - -# Store the data of the top functions in top_functions[] -top_functions = functions[:number_of_top_functions] +""") + +# Run perf and save all intermediate files in a temporary directory +with tempfile.TemporaryDirectory() as tmpdir: + RECORD_PATH = os.path.join(tmpdir, "record.data") + REPORT_PATH = os.path.join(tmpdir, "report.txt") + + PERF_RECORD = subprocess.run((["perf", "record", "--output="+RECORD_PATH] + + COMMAND), + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + check=False) + if PERF_RECORD.returncode: + sys.exit(PERF_RECORD.stderr.decode("utf-8")) + + with open(REPORT_PATH, "w") as output: + PERF_REPORT = subprocess.run( + ["perf", "report", "--input="+RECORD_PATH, "--stdio"], + stdout=output, + stderr=subprocess.PIPE, + check=False) + if PERF_REPORT.returncode: + sys.exit(PERF_REPORT.stderr.decode("utf-8")) + + # Save the reported data to FUNCTIONS[] + with open(REPORT_PATH, "r") as data: + # Only read lines that are not comments (comments start with #) + # Only read lines that are not empty + FUNCTIONS = [line for line in data.readlines() if line and + line[0] != '#' and line[0] != "\n"] + +# Limit the number of top functions to "TOP" +NO_TOP_FUNCTIONS = TOP if len(FUNCTIONS) > TOP else len(FUNCTIONS) + +# Store the data of the top functions in TOP_FUNCTIONS[] +TOP_FUNCTIONS = FUNCTIONS[:NO_TOP_FUNCTIONS] # Print table header print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.', @@ -134,7 +138,7 @@ print('{:>4} {:>10} {:<30} {}\n{} {} {} {}'.format('No.', '-' * 25)) # Print top N functions -for (index, function) in enumerate(top_functions, start=1): +for (index, function) in enumerate(TOP_FUNCTIONS, start=1): function_data = function.split() function_percentage = function_data[0] function_name = function_data[-1] @@ -143,7 +147,3 @@ for (index, function) in enumerate(top_functions, start=1): function_percentage, function_name, function_invoker)) - -# Remove intermediate files -os.unlink('/tmp/perf.data') -os.unlink('/tmp/perf_report.out')
- Apply pylint and flake8 formatting rules to the script. - Use 'tempfile' instead of '/tmp' for creating temporary files. Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com> --- scripts/performance/topN_perf.py | 174 +++++++++++++++---------------- 1 file changed, 87 insertions(+), 87 deletions(-)