[PATCH 0/1] Add bisect.py script

[PATCH 0/1] Add bisect.py script

[Ahmed Karaman]

Hi,

This series adds the new bisect.py script introduced in report 5 of
the "TCG Continuous Benchmarking" GSoC project.

The script is used for locating the commit that caused a performance
degradation or improvement in QEMU using the git bisect command
(binary search).

To learn more about how the script works and how it can be used for
detecting two commits, one that introduced a performance degradation
in PowerPC targets, and the other introducing a performance
improvement in MIPS, please check the
"Finding Commits Affecting QEMU Performance" report.

Report link:
https://lists.nongnu.org/archive/html/qemu-devel/2020-07/msg05769.html

Best regards,
Ahmed Karaman

Ahmed Karaman (1):
  scripts/performance: Add bisect.py script

 scripts/performance/bisect.py | 374 ++++++++++++++++++++++++++++++++++
 1 file changed, 374 insertions(+)
 create mode 100755 scripts/performance/bisect.py

-- 
2.17.1

[PATCH 1/1] scripts/performance: Add bisect.py script

[Ahmed Karaman]

Python script that locates the commit that caused a performance
degradation or improvement in QEMU using the git bisect command
(binary search).

Syntax:
bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
--target TARGET --tool {perf,callgrind} -- \
<target executable> [<target executable options>]

[-h] - Print the script arguments help message
-s,--start START - First commit hash in the search range
[-e,--end END] - Last commit hash in the search range
                (default: Latest commit)
[-q,--qemu QEMU] - QEMU path.
                (default: Path to a GitHub QEMU clone)
--target TARGET - QEMU target name
--tool {perf,callgrind} - Underlying tool used for measurements

Example of usage:
bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc \
--tool=perf -- coulomb_double-ppc -n 1000

Example output:
Start Commit Instructions:     12,710,790,060
End Commit Instructions:       13,031,083,512
Performance Change:            -2.458%

Estimated Number of Steps:     10

*****************BISECT STEP 1*****************
Instructions:        13,031,097,790
Status:              slow commit
*****************BISECT STEP 2*****************
Instructions:        12,710,805,265
Status:              fast commit
*****************BISECT STEP 3*****************
Instructions:        13,031,028,053
Status:              slow commit
*****************BISECT STEP 4*****************
Instructions:        12,711,763,211
Status:              fast commit
*****************BISECT STEP 5*****************
Instructions:        13,031,027,292
Status:              slow commit
*****************BISECT STEP 6*****************
Instructions:        12,711,748,738
Status:              fast commit
*****************BISECT STEP 7*****************
Instructions:        12,711,748,788
Status:              fast commit
*****************BISECT STEP 8*****************
Instructions:        13,031,100,493
Status:              slow commit
*****************BISECT STEP 9*****************
Instructions:        12,714,472,954
Status:              fast commit
****************BISECT STEP 10*****************
Instructions:        12,715,409,153
Status:              fast commit
****************BISECT STEP 11*****************
Instructions:        12,715,394,739
Status:              fast commit

*****************BISECT RESULT*****************
commit 0673ecdf6cb2b1445a85283db8cbacb251c46516
Author: Richard Henderson <richard.henderson@linaro.org>
Date:   Tue May 5 10:40:23 2020 -0700

    softfloat: Inline float64 compare specializations

    Replace the float64 compare specializations with inline functions
    that call the standard float64_compare{,_quiet} functions.
    Use bool as the return type.
***********************************************

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

diff --git a/scripts/performance/bisect.py b/scripts/performance/bisect.py
new file mode 100755
index 0000000000..869cc69ef4
--- /dev/null
+++ b/scripts/performance/bisect.py
@@ -0,0 +1,374 @@
+#!/usr/bin/env python3
+
+#  Locate the commit that caused a performance degradation or improvement in
+#  QEMU using the git bisect command (binary search).
+#
+#  Syntax:
+#  bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
+#  --target TARGET --tool {perf,callgrind} -- \
+#  <target executable> [<target executable options>]
+#
+#  [-h] - Print the script arguments help message
+#  -s,--start START - First commit hash in the search range
+#  [-e,--end END] - Last commit hash in the search range
+#             (default: Latest commit)
+#  [-q,--qemu QEMU] - QEMU path.
+#              (default: Path to a GitHub QEMU clone)
+#  --target TARGET - QEMU target name
+#  --tool {perf,callgrind} - Underlying tool used for measurements
+
+#  Example of usage:
+#  bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc --tool=perf \
+#  -- coulomb_double-ppc -n 1000
+#
+#  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 multiprocessing
+import tempfile
+import os
+import shutil
+import subprocess
+import sys
+
+
+############################ GIT WRAPPERS ############################
+def git_bisect(qemu_path, command, args=None):
+    """
+    Wrapper function for running git bisect.
+
+    Parameters:
+    qemu_path (str): QEMU path.
+    command (str):   bisect command (start|fast|slow|reset).
+    args (list):     Optional arguments.
+
+    Returns:
+    (str):           git bisect stdout.
+    """
+    process = ["git", "bisect", command]
+    if args:
+        process += args
+    bisect = subprocess.run(process,
+                            cwd=qemu_path,
+                            stdout=subprocess.PIPE,
+                            stderr=subprocess.PIPE)
+    if bisect.returncode:
+        sys.exit(bisect.stderr.decode("utf-8"))
+    return bisect.stdout.decode("utf-8")
+
+
+def git_checkout(commit, qemu_path):
+    """
+    Wrapper function for checking out a given git commit.
+
+    Parameters:
+    commit (str):    Commit hash of a git commit.
+    qemu_path (str): QEMU path.
+    """
+    checkout_commit = subprocess.run(["git",
+                                      "checkout",
+                                      commit],
+                                     cwd=qemu_path,
+                                     stdout=subprocess.DEVNULL,
+                                     stderr=subprocess.PIPE)
+    if checkout_commit.returncode:
+        sys.exit(checkout_commit.stderr.decode("utf-8"))
+
+
+def git_clone(qemu_path):
+    """
+    Wrapper function for cloning QEMU git repo from GitHub.
+
+    Parameters:
+    qemu_path (str): Path to clone the QEMU repo to.
+    """
+    clone_qemu = subprocess.run(["git",
+                                 "clone",
+                                 "https://github.com/qemu/qemu.git",
+                                 qemu_path],
+                                stderr=subprocess.STDOUT)
+    if clone_qemu.returncode:
+        sys.exit("Failed to clone QEMU!")
+######################################################################
+
+
+def check_requirements(tool):
+    """
+    Verify that all script requirements are installed (perf|callgrind & git).
+
+    Parameters:
+    tool (str): Tool used for the measurement (perf or callgrind).
+    """
+    if tool == "perf":
+        check_perf_installation = subprocess.run(["which", "perf"],
+                                                 stdout=subprocess.DEVNULL)
+        if check_perf_installation.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", "/"],
+                                                  stdout=subprocess.DEVNULL,
+                                                  stderr=subprocess.DEVNULL)
+        if check_perf_executability.returncode:
+            sys.exit("""
+        Error:
+        You may not have permission to collect stats.
+        Consider tweaking /proc/sys/kernel/perf_event_paranoid,
+        which controls use of the performance events system by
+        unprivileged users (without CAP_SYS_ADMIN).
+        -1: Allow use of (almost) all events by all users
+            Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK
+        0: Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN
+            Disallow raw tracepoint access by users without CAP_SYS_ADMIN
+        1: Disallow CPU event access by users without CAP_SYS_ADMIN
+        2: Disallow kernel profiling by users without CAP_SYS_ADMIN
+        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.
+        """)
+    elif tool == "callgrind":
+        check_valgrind_installation = subprocess.run(["which", "valgrind"],
+                                                     stdout=subprocess.DEVNULL)
+        if check_valgrind_installation.returncode:
+            sys.exit("Please install valgrind before running the script.")
+
+    # Insure that git is installed
+    check_git_installation = subprocess.run(["which", "git"],
+                                            stdout=subprocess.DEVNULL)
+    if check_git_installation.returncode:
+        sys.exit("Please install git before running the script.")
+
+
+def make(qemu_build_path):
+    """
+    Build QEMU by running the Makefile.
+
+    Parameters:
+    qemu_build_path (str): Path to the build directory with configuration files.
+    """
+    run_make = subprocess.run(["make",
+                               "-j",
+                               str(multiprocessing.cpu_count())],
+                              cwd=qemu_build_path,
+                              stdout=subprocess.DEVNULL,
+                              stderr=subprocess.PIPE)
+    if run_make.returncode:
+        sys.exit(run_make.stderr.decode("utf-8"))
+
+
+def measure_instructions(tool, qemu_exe_path, command):
+    """
+    Measure the number of instructions when running an program with QEMU.
+
+    Parameters:
+    tool (str):          Tool used for the measurement (perf|callgrind).
+    qemu_exe_path (str): Path to the QEMU executable of the equivalent target.
+    command (list):      Program path and arguments.
+
+    Returns:
+    (int):               Number of instructions.
+    """
+    if tool == "perf":
+        run_perf = subprocess.run((["perf",
+                                    "stat",
+                                    "-x",
+                                    " ",
+                                    "-e",
+                                    "instructions",
+                                    qemu_exe_path]
+                                   + command),
+                                  stdout=subprocess.DEVNULL,
+                                  stderr=subprocess.PIPE)
+        if run_perf.returncode:
+            sys.exit(run_perf.stderr.decode("utf-8"))
+        else:
+            perf_output = run_perf.stderr.decode("utf-8").split(" ")
+            return int(perf_output[0])
+
+    elif tool == "callgrind":
+        with tempfile.NamedTemporaryFile() as tmpfile:
+            run_callgrind = subprocess.run((["valgrind",
+                                             "--tool=callgrind",
+                                             "--callgrind-out-file={}".format(
+                                                 tmpfile.name),
+                                             qemu_exe_path]
+                                            + command),
+                                           stdout=subprocess.DEVNULL,
+                                           stderr=subprocess.PIPE)
+        if run_callgrind.returncode:
+            sys.exit(run_callgrind.stderr.decode("utf-8"))
+        else:
+            callgrind_output = run_callgrind.stderr.decode("utf-8").split("\n")
+            return int(callgrind_output[8].split(" ")[-1])
+
+
+def main():
+    # Parse the command line arguments
+    parser = argparse.ArgumentParser(
+        usage="bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] "
+        "--target TARGET --tool {perf,callgrind} -- "
+        "<target executable> [<target executable options>]")
+
+    parser.add_argument("-s", "--start", dest="start", type=str, required=True,
+                        help="First commit hash in the search range")
+    parser.add_argument("-e", "--end", dest="end", type=str, default="master",
+                        help="Last commit hash in the search range")
+    parser.add_argument("-q", "--qemu", dest="qemu", type=str, default="",
+                        help="QEMU path")
+    parser.add_argument("--target", dest="target", type=str, required=True,
+                        help="QEMU target")
+    parser.add_argument("--tool", dest="tool", choices=["perf", "callgrind"],
+                        required=True, help="Tool used for measurements")
+
+    parser.add_argument("command", type=str, nargs="+", help=argparse.SUPPRESS)
+
+    args = parser.parse_args()
+
+    # Extract the needed variables from the args
+    start_commit = args.start
+    end_commit = args.end
+    qemu = args.qemu
+    target = args.target
+    tool = args.tool
+    command = args.command
+
+    # Set QEMU path
+    if qemu == "":
+        # Create a temp directory for cloning QEMU
+        tmpdir = tempfile.TemporaryDirectory()
+        qemu_path = os.path.join(tmpdir.name, "qemu")
+
+        # Clone QEMU into the temporary directory
+        print("Fetching QEMU: ", end="", flush=True)
+        git_clone(qemu_path)
+        print()
+    else:
+        qemu_path = qemu
+
+    # Create the build directory
+    qemu_build_path = os.path.join(qemu_path, "tmp-build-gcc")
+    if not os.path.exists(qemu_build_path):
+        os.mkdir(qemu_build_path)
+    else:
+        sys.exit("A build directory with the same name (tmp-build-gcc) used in "
+                 "the script is already in the provided QEMU path.")
+
+    qemu_exe_path = os.path.join(qemu_build_path,
+                                 "{}-linux-user".format(target),
+                                 "qemu-{}".format(target))
+
+    # Configure QEMU
+    configure = subprocess.run(["../configure",
+                                "--target-list={}-linux-user".format(target)],
+                               cwd=qemu_build_path,
+                               stdout=subprocess.DEVNULL,
+                               stderr=subprocess.PIPE)
+    if configure.returncode:
+        sys.exit(configure.stderr.decode("utf-8"))
+
+    # Do performance measurements for the start commit
+    git_checkout(start_commit, qemu_path)
+    make(qemu_build_path)
+    start_commit_instructions = measure_instructions(tool,
+                                                     qemu_exe_path,
+                                                     command)
+    print("{:<30} {}".format("Start Commit Instructions:",
+                             format(start_commit_instructions, ",")))
+
+    # Do performance measurements for the end commit
+    git_checkout(end_commit, qemu_path)
+    make(qemu_build_path)
+    end_commit_instructions = measure_instructions(tool,
+                                                   qemu_exe_path,
+                                                   command)
+    print("{:<30} {}".format("End Commit Instructions:",
+                             format(end_commit_instructions, ",")))
+
+    # Calculate performance difference between start and end commits
+    performance_difference = \
+        (start_commit_instructions - end_commit_instructions) / \
+        max(end_commit_instructions, start_commit_instructions) * 100
+    performance_change = "+" if performance_difference > 0 else "-"
+    print("{:<30} {}".format("Performance Change:",
+                             performance_change +
+                             str(round(abs(performance_difference), 3))+"%"))
+
+    # Set the custom terms used for progressing in "git bisect"
+    term_old = "fast" if performance_difference < 0 else "slow"
+    term_new = "slow" if term_old == "fast" else "fast"
+
+    # Start git bisect
+    git_bisect(qemu_path, "start", [
+               "--term-old", term_old, "--term-new", term_new])
+    # Set start commit state
+    git_bisect(qemu_path, term_old, [start_commit])
+    # Set end commit state
+    bisect_output = git_bisect(qemu_path, term_new, [end_commit])
+    # Print estimated bisect steps
+    print("\n{:<30} {}\n".format(
+        "Estimated Number of Steps:", bisect_output.split()[9]))
+
+    # Initialize bisect_count to track the number of performed
+    bisect_count = 1
+
+    while True:
+        print("**************BISECT STEP {}**************".format(bisect_count))
+
+        make(qemu_build_path)
+
+        instructions = measure_instructions(tool, qemu_exe_path, command)
+        # Find the difference between the current instructions and start/end
+        # instructions.
+        diff_end = abs(instructions - end_commit_instructions)
+        diff_start = abs(instructions - start_commit_instructions)
+
+        # If current number of insructions is closer to that of start,
+        # set current commit as term_old.
+        # Else, set current commit as term_new.
+        if diff_end > diff_start:
+            bisect_command = term_old
+        else:
+            bisect_command = term_new
+
+        print("{:<20} {}".format("Instructions:", format(instructions, ",")))
+        print("{:<20} {}".format("Status:", "{} commit".format(bisect_command)))
+
+        bisect_output = git_bisect(qemu_path, bisect_command)
+
+        # Continue if still bisecting,
+        # else, print result and break.
+        if not bisect_output.split(" ")[0] == "Bisecting:":
+            print("\n*****************BISECT RESULT*****************")
+            commit_message_start = bisect_output.find("commit\n") + 7
+            commit_message_end = bisect_output.find(":040000") - 1
+            print(bisect_output[commit_message_start:commit_message_end])
+            break
+
+        bisect_count += 1
+
+    # Reset git bisect
+    git_bisect(qemu_path, "reset")
+
+    # Delete temp build directory
+    shutil.rmtree(qemu_build_path)
+
+
+if __name__ == "__main__":
+    main()
-- 
2.17.1

Re: [PATCH 1/1] scripts/performance: Add bisect.py script

[John Snow]

On 7/21/20 7:15 PM, Ahmed Karaman wrote:
> Python script that locates the commit that caused a performance
> degradation or improvement in QEMU using the git bisect command
> (binary search).
> 
> Syntax:
> bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
> --target TARGET --tool {perf,callgrind} -- \
> <target executable> [<target executable options>]
> 
> [-h] - Print the script arguments help message
> -s,--start START - First commit hash in the search range
> [-e,--end END] - Last commit hash in the search range
>                  (default: Latest commit)
> [-q,--qemu QEMU] - QEMU path.
>                  (default: Path to a GitHub QEMU clone)
> --target TARGET - QEMU target name
> --tool {perf,callgrind} - Underlying tool used for measurements
> 
> Example of usage:
> bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc \
> --tool=perf -- coulomb_double-ppc -n 1000
> 
> Example output:
> Start Commit Instructions:     12,710,790,060
> End Commit Instructions:       13,031,083,512
> Performance Change:            -2.458%
> 
> Estimated Number of Steps:     10
> 
> *****************BISECT STEP 1*****************
> Instructions:        13,031,097,790
> Status:              slow commit
> *****************BISECT STEP 2*****************
> Instructions:        12,710,805,265
> Status:              fast commit
> *****************BISECT STEP 3*****************
> Instructions:        13,031,028,053
> Status:              slow commit
> *****************BISECT STEP 4*****************
> Instructions:        12,711,763,211
> Status:              fast commit
> *****************BISECT STEP 5*****************
> Instructions:        13,031,027,292
> Status:              slow commit
> *****************BISECT STEP 6*****************
> Instructions:        12,711,748,738
> Status:              fast commit
> *****************BISECT STEP 7*****************
> Instructions:        12,711,748,788
> Status:              fast commit
> *****************BISECT STEP 8*****************
> Instructions:        13,031,100,493
> Status:              slow commit
> *****************BISECT STEP 9*****************
> Instructions:        12,714,472,954
> Status:              fast commit
> ****************BISECT STEP 10*****************
> Instructions:        12,715,409,153
> Status:              fast commit
> ****************BISECT STEP 11*****************
> Instructions:        12,715,394,739
> Status:              fast commit
> 
> *****************BISECT RESULT*****************
> commit 0673ecdf6cb2b1445a85283db8cbacb251c46516
> Author: Richard Henderson <richard.henderson@linaro.org>
> Date:   Tue May 5 10:40:23 2020 -0700
> 
>      softfloat: Inline float64 compare specializations
> 
>      Replace the float64 compare specializations with inline functions
>      that call the standard float64_compare{,_quiet} functions.
>      Use bool as the return type.
> ***********************************************
> 
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>

Howdy!

Can you run this through some python linters?
Try pylint 2.4.4+ and flake8 3.8.3+.

`pylint --disable=too-many-locals,too-many-statements bisect.py` would 
be a good target to hit; there's not too many warnings. Mostly, it wants 
you to specify the check= parameter to subprocess.run().

> ---
>   scripts/performance/bisect.py | 374 ++++++++++++++++++++++++++++++++++
>   1 file changed, 374 insertions(+)
>   create mode 100755 scripts/performance/bisect.py
> 
> diff --git a/scripts/performance/bisect.py b/scripts/performance/bisect.py
> new file mode 100755
> index 0000000000..869cc69ef4
> --- /dev/null
> +++ b/scripts/performance/bisect.py
> @@ -0,0 +1,374 @@
> +#!/usr/bin/env python3
> +

The following preamble could be a docstring:

"""

> +#  Locate the commit that caused a performance degradation or improvement in
> +#  QEMU using the git bisect command (binary search).
> +#
> +#  Syntax:
> +#  bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
> +#  --target TARGET --tool {perf,callgrind} -- \
> +#  <target executable> [<target executable options>]
> +#
> +#  [-h] - Print the script arguments help message
> +#  -s,--start START - First commit hash in the search range
> +#  [-e,--end END] - Last commit hash in the search range
> +#             (default: Latest commit)
> +#  [-q,--qemu QEMU] - QEMU path.
> +#              (default: Path to a GitHub QEMU clone)
> +#  --target TARGET - QEMU target name
> +#  --tool {perf,callgrind} - Underlying tool used for measurements
> +
> +#  Example of usage:
> +#  bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc --tool=perf \
> +#  -- coulomb_double-ppc -n 1000
> +#
> +#  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 multiprocessing
> +import tempfile
> +import os
> +import shutil
> +import subprocess
> +import sys
> +
> +
> +############################ GIT WRAPPERS ############################
> +def git_bisect(qemu_path, command, args=None):
> +    """
> +    Wrapper function for running git bisect.
> +
> +    Parameters:
> +    qemu_path (str): QEMU path.
> +    command (str):   bisect command (start|fast|slow|reset).
> +    args (list):     Optional arguments.
> +
> +    Returns:
> +    (str):           git bisect stdout.
> +    """
> +    process = ["git", "bisect", command]
> +    if args:
> +        process += args
> +    bisect = subprocess.run(process,
> +                            cwd=qemu_path,
> +                            stdout=subprocess.PIPE,
> +                            stderr=subprocess.PIPE)
> +    if bisect.returncode:
> +        sys.exit(bisect.stderr.decode("utf-8"))
> +    return bisect.stdout.decode("utf-8")
> +
> +
> +def git_checkout(commit, qemu_path):
> +    """
> +    Wrapper function for checking out a given git commit.
> +
> +    Parameters:
> +    commit (str):    Commit hash of a git commit.
> +    qemu_path (str): QEMU path.
> +    """
> +    checkout_commit = subprocess.run(["git",
> +                                      "checkout",
> +                                      commit],
> +                                     cwd=qemu_path,
> +                                     stdout=subprocess.DEVNULL,
> +                                     stderr=subprocess.PIPE)
> +    if checkout_commit.returncode:
> +        sys.exit(checkout_commit.stderr.decode("utf-8"))
> +
> +
> +def git_clone(qemu_path):
> +    """
> +    Wrapper function for cloning QEMU git repo from GitHub.
> +
> +    Parameters:
> +    qemu_path (str): Path to clone the QEMU repo to.
> +    """
> +    clone_qemu = subprocess.run(["git",
> +                                 "clone",
> +                                 "https://github.com/qemu/qemu.git",
> +                                 qemu_path],
> +                                stderr=subprocess.STDOUT)
> +    if clone_qemu.returncode:
> +        sys.exit("Failed to clone QEMU!")
> +######################################################################
> +
> +
> +def check_requirements(tool):
> +    """
> +    Verify that all script requirements are installed (perf|callgrind & git).
> +
> +    Parameters:
> +    tool (str): Tool used for the measurement (perf or callgrind).
> +    """
> +    if tool == "perf":
> +        check_perf_installation = subprocess.run(["which", "perf"],
> +                                                 stdout=subprocess.DEVNULL)
> +        if check_perf_installation.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", "/"],
> +                                                  stdout=subprocess.DEVNULL,
> +                                                  stderr=subprocess.DEVNULL)
> +        if check_perf_executability.returncode:
> +            sys.exit("""
> +        Error:
> +        You may not have permission to collect stats.
> +        Consider tweaking /proc/sys/kernel/perf_event_paranoid,
> +        which controls use of the performance events system by
> +        unprivileged users (without CAP_SYS_ADMIN).
> +        -1: Allow use of (almost) all events by all users
> +            Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK
> +        0: Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN
> +            Disallow raw tracepoint access by users without CAP_SYS_ADMIN
> +        1: Disallow CPU event access by users without CAP_SYS_ADMIN
> +        2: Disallow kernel profiling by users without CAP_SYS_ADMIN
> +        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.
> +        """)
> +    elif tool == "callgrind":
> +        check_valgrind_installation = subprocess.run(["which", "valgrind"],
> +                                                     stdout=subprocess.DEVNULL)
> +        if check_valgrind_installation.returncode:
> +            sys.exit("Please install valgrind before running the script.")
> +
> +    # Insure that git is installed
> +    check_git_installation = subprocess.run(["which", "git"],
> +                                            stdout=subprocess.DEVNULL)
> +    if check_git_installation.returncode:
> +        sys.exit("Please install git before running the script.")
> +
> +
> +def make(qemu_build_path):
> +    """
> +    Build QEMU by running the Makefile.
> +
> +    Parameters:
> +    qemu_build_path (str): Path to the build directory with configuration files.
> +    """

Instead of annotating types using plaintext in the docstring, have you 
considered using mypy?

I've been using it to clean up ./python/qemu and /tests/qemu-iotests, 
and for simple python scripts where you are not using highly dynamic 
data types, it seems to work pretty well.

(I'm not insisting you do that for this patch, but since you went 
through the trouble to annotate it, I thought I'd mention it.)

> +    run_make = subprocess.run(["make",
> +                               "-j",
> +                               str(multiprocessing.cpu_count())],
> +                              cwd=qemu_build_path,
> +                              stdout=subprocess.DEVNULL,
> +                              stderr=subprocess.PIPE)
> +    if run_make.returncode:
> +        sys.exit(run_make.stderr.decode("utf-8"))
> +
> +
> +def measure_instructions(tool, qemu_exe_path, command):

e.x.

def measure_instructions(tool: str,
                          qemu_exe_path: str,
                          command: str) -> int:

> +    """
> +    Measure the number of instructions when running an program with QEMU.
> +
> +    Parameters:
> +    tool (str):          Tool used for the measurement (perf|callgrind).
> +    qemu_exe_path (str): Path to the QEMU executable of the equivalent target.
> +    command (list):      Program path and arguments.
> +
> +    Returns:
> +    (int):               Number of instructions.
> +    """
> +    if tool == "perf":
> +        run_perf = subprocess.run((["perf",
> +                                    "stat",
> +                                    "-x",
> +                                    " ",
> +                                    "-e",
> +                                    "instructions",
> +                                    qemu_exe_path]
> +                                   + command),
> +                                  stdout=subprocess.DEVNULL,
> +                                  stderr=subprocess.PIPE)
> +        if run_perf.returncode:
> +            sys.exit(run_perf.stderr.decode("utf-8"))
> +        else:
> +            perf_output = run_perf.stderr.decode("utf-8").split(" ")
> +            return int(perf_output[0])
> +
> +    elif tool == "callgrind":
> +        with tempfile.NamedTemporaryFile() as tmpfile:
> +            run_callgrind = subprocess.run((["valgrind",
> +                                             "--tool=callgrind",
> +                                             "--callgrind-out-file={}".format(
> +                                                 tmpfile.name),
> +                                             qemu_exe_path]
> +                                            + command),
> +                                           stdout=subprocess.DEVNULL,
> +                                           stderr=subprocess.PIPE)
> +        if run_callgrind.returncode:
> +            sys.exit(run_callgrind.stderr.decode("utf-8"))
> +        else:
> +            callgrind_output = run_callgrind.stderr.decode("utf-8").split("\n")
> +            return int(callgrind_output[8].split(" ")[-1])
> +
> +
> +def main():
> +    # Parse the command line arguments
> +    parser = argparse.ArgumentParser(
> +        usage="bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] "
> +        "--target TARGET --tool {perf,callgrind} -- "
> +        "<target executable> [<target executable options>]")
> +
> +    parser.add_argument("-s", "--start", dest="start", type=str, required=True,
> +                        help="First commit hash in the search range")
> +    parser.add_argument("-e", "--end", dest="end", type=str, default="master",
> +                        help="Last commit hash in the search range")
> +    parser.add_argument("-q", "--qemu", dest="qemu", type=str, default="",
> +                        help="QEMU path")
> +    parser.add_argument("--target", dest="target", type=str, required=True,
> +                        help="QEMU target")
> +    parser.add_argument("--tool", dest="tool", choices=["perf", "callgrind"],
> +                        required=True, help="Tool used for measurements")
> +
> +    parser.add_argument("command", type=str, nargs="+", help=argparse.SUPPRESS)
> +
> +    args = parser.parse_args()
> +
> +    # Extract the needed variables from the args
> +    start_commit = args.start
> +    end_commit = args.end
> +    qemu = args.qemu
> +    target = args.target
> +    tool = args.tool
> +    command = args.command
> +
> +    # Set QEMU path
> +    if qemu == "":
> +        # Create a temp directory for cloning QEMU
> +        tmpdir = tempfile.TemporaryDirectory()
> +        qemu_path = os.path.join(tmpdir.name, "qemu")
> +
> +        # Clone QEMU into the temporary directory
> +        print("Fetching QEMU: ", end="", flush=True)
> +        git_clone(qemu_path)
> +        print()
> +    else:
> +        qemu_path = qemu
> +
> +    # Create the build directory
> +    qemu_build_path = os.path.join(qemu_path, "tmp-build-gcc")
> +    if not os.path.exists(qemu_build_path):
> +        os.mkdir(qemu_build_path)
> +    else:
> +        sys.exit("A build directory with the same name (tmp-build-gcc) used in "
> +                 "the script is already in the provided QEMU path.")
> +
> +    qemu_exe_path = os.path.join(qemu_build_path,
> +                                 "{}-linux-user".format(target),
> +                                 "qemu-{}".format(target))
> +
> +    # Configure QEMU
> +    configure = subprocess.run(["../configure",
> +                                "--target-list={}-linux-user".format(target)],
> +                               cwd=qemu_build_path,
> +                               stdout=subprocess.DEVNULL,
> +                               stderr=subprocess.PIPE)
> +    if configure.returncode:
> +        sys.exit(configure.stderr.decode("utf-8"))
> +
> +    # Do performance measurements for the start commit
> +    git_checkout(start_commit, qemu_path)
> +    make(qemu_build_path)
> +    start_commit_instructions = measure_instructions(tool,
> +                                                     qemu_exe_path,
> +                                                     command)
> +    print("{:<30} {}".format("Start Commit Instructions:",
> +                             format(start_commit_instructions, ",")))
> +
> +    # Do performance measurements for the end commit
> +    git_checkout(end_commit, qemu_path)
> +    make(qemu_build_path)
> +    end_commit_instructions = measure_instructions(tool,
> +                                                   qemu_exe_path,
> +                                                   command)
> +    print("{:<30} {}".format("End Commit Instructions:",
> +                             format(end_commit_instructions, ",")))
> +
> +    # Calculate performance difference between start and end commits
> +    performance_difference = \
> +        (start_commit_instructions - end_commit_instructions) / \
> +        max(end_commit_instructions, start_commit_instructions) * 100
> +    performance_change = "+" if performance_difference > 0 else "-"
> +    print("{:<30} {}".format("Performance Change:",
> +                             performance_change +
> +                             str(round(abs(performance_difference), 3))+"%"))
> +
> +    # Set the custom terms used for progressing in "git bisect"
> +    term_old = "fast" if performance_difference < 0 else "slow"
> +    term_new = "slow" if term_old == "fast" else "fast"
> +
> +    # Start git bisect
> +    git_bisect(qemu_path, "start", [
> +               "--term-old", term_old, "--term-new", term_new])
> +    # Set start commit state
> +    git_bisect(qemu_path, term_old, [start_commit])
> +    # Set end commit state
> +    bisect_output = git_bisect(qemu_path, term_new, [end_commit])
> +    # Print estimated bisect steps
> +    print("\n{:<30} {}\n".format(
> +        "Estimated Number of Steps:", bisect_output.split()[9]))
> +
> +    # Initialize bisect_count to track the number of performed
> +    bisect_count = 1
> +
> +    while True:
> +        print("**************BISECT STEP {}**************".format(bisect_count))
> +
> +        make(qemu_build_path)
> +
> +        instructions = measure_instructions(tool, qemu_exe_path, command)
> +        # Find the difference between the current instructions and start/end
> +        # instructions.
> +        diff_end = abs(instructions - end_commit_instructions)
> +        diff_start = abs(instructions - start_commit_instructions)
> +
> +        # If current number of insructions is closer to that of start,
> +        # set current commit as term_old.
> +        # Else, set current commit as term_new.
> +        if diff_end > diff_start:
> +            bisect_command = term_old
> +        else:
> +            bisect_command = term_new
> +
> +        print("{:<20} {}".format("Instructions:", format(instructions, ",")))
> +        print("{:<20} {}".format("Status:", "{} commit".format(bisect_command)))
> +
> +        bisect_output = git_bisect(qemu_path, bisect_command)
> +
> +        # Continue if still bisecting,
> +        # else, print result and break.
> +        if not bisect_output.split(" ")[0] == "Bisecting:":
> +            print("\n*****************BISECT RESULT*****************")
> +            commit_message_start = bisect_output.find("commit\n") + 7
> +            commit_message_end = bisect_output.find(":040000") - 1
> +            print(bisect_output[commit_message_start:commit_message_end])
> +            break
> +
> +        bisect_count += 1
> +
> +    # Reset git bisect
> +    git_bisect(qemu_path, "reset")
> +
> +    # Delete temp build directory
> +    shutil.rmtree(qemu_build_path)
> +
> +
> +if __name__ == "__main__":
> +    main()
> 

Re: [PATCH 1/1] scripts/performance: Add bisect.py script

[Aleksandar Markovic]

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

On Wednesday, July 22, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
wrote:

> Python script that locates the commit that caused a performance
> degradation or improvement in QEMU using the git bisect command
> (binary search).
>
> Syntax:
> bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
> --target TARGET --tool {perf,callgrind} -- \
> <target executable> [<target executable options>]
>
> [-h] - Print the script arguments help message
> -s,--start START - First commit hash in the search range
> [-e,--end END] - Last commit hash in the search range
>                 (default: Latest commit)
> [-q,--qemu QEMU] - QEMU path.
>                 (default: Path to a GitHub QEMU clone)
> --target TARGET - QEMU target name
> --tool {perf,callgrind} - Underlying tool used for measurements
>
> Example of usage:
> bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc \
> --tool=perf -- coulomb_double-ppc -n 1000
>
> Example output:
> Start Commit Instructions:     12,710,790,060
> End Commit Instructions:       13,031,083,512
> Performance Change:            -2.458%
>
> Estimated Number of Steps:     10
>
> *****************BISECT STEP 1*****************
> Instructions:        13,031,097,790
> Status:              slow commit
> *****************BISECT STEP 2*****************
> Instructions:        12,710,805,265
> Status:              fast commit
> *****************BISECT STEP 3*****************
> Instructions:        13,031,028,053
> Status:              slow commit
> *****************BISECT STEP 4*****************
> Instructions:        12,711,763,211
> Status:              fast commit
> *****************BISECT STEP 5*****************
> Instructions:        13,031,027,292
> Status:              slow commit
> *****************BISECT STEP 6*****************
> Instructions:        12,711,748,738
> Status:              fast commit
> *****************BISECT STEP 7*****************
> Instructions:        12,711,748,788
> Status:              fast commit
> *****************BISECT STEP 8*****************
> Instructions:        13,031,100,493
> Status:              slow commit
> *****************BISECT STEP 9*****************
> Instructions:        12,714,472,954
> Status:              fast commit
> ****************BISECT STEP 10*****************
> Instructions:        12,715,409,153
> Status:              fast commit
> ****************BISECT STEP 11*****************
> Instructions:        12,715,394,739
> Status:              fast commit
>
> *****************BISECT RESULT*****************
> commit 0673ecdf6cb2b1445a85283db8cbacb251c46516
> Author: Richard Henderson <richard.henderson@linaro.org>
> Date:   Tue May 5 10:40:23 2020 -0700
>
>     softfloat: Inline float64 compare specializations
>
>     Replace the float64 compare specializations with inline functions
>     that call the standard float64_compare{,_quiet} functions.
>     Use bool as the return type.
> ***********************************************
>
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> ---
>  scripts/performance/bisect.py | 374 ++++++++++++++++++++++++++++++++++
>  1 file changed, 374 insertions(+)
>  create mode 100755 scripts/performance/bisect.py
>
> diff --git a/scripts/performance/bisect.py b/scripts/performance/bisect.py
> new file mode 100755
> index 0000000000..869cc69ef4
> --- /dev/null
> +++ b/scripts/performance/bisect.py
> @@ -0,0 +1,374 @@
> +#!/usr/bin/env python3
> +
> +#  Locate the commit that caused a performance degradation or improvement
> in
> +#  QEMU using the git bisect command (binary search).
> +#
> +#  Syntax:
> +#  bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
> +#  --target TARGET --tool {perf,callgrind} -- \
> +#  <target executable> [<target executable options>]
> +#
> +#  [-h] - Print the script arguments help message
> +#  -s,--start START - First commit hash in the search range
> +#  [-e,--end END] - Last commit hash in the search range
> +#             (default: Latest commit)
> +#  [-q,--qemu QEMU] - QEMU path.
> +#              (default: Path to a GitHub QEMU clone)
> +#  --target TARGET - QEMU target name
> +#  --tool {perf,callgrind} - Underlying tool used for measurements
> +
> +#  Example of usage:
> +#  bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc
> --tool=perf \
> +#  -- coulomb_double-ppc -n 1000
> +#
> +#  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>
> +#


Hi, Ahmed.

Yes, somewhat related to John's hints on these comments, it is customary to
have just a brief description before "Copyright" lines. This means one
sentence, or a short paragraph (3-4 sentences max). The lenghty syntax
commemt should be, in my opinion, moved after the license preamble, just
before the start of real Python code.

One question:

What is the behavior in case of the executable architecture and "target"
command line option mismatch (for example, one specifies m68k target, but
passes hppa executable? Would that be detected before bisect search, or the
bisect procedure will be applied even though such cases do not make sense?

Yours, Aleksandar



> +#  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 multiprocessing
> +import tempfile
> +import os
> +import shutil
> +import subprocess
> +import sys
> +
> +
> +############################ GIT WRAPPERS ############################
> +def git_bisect(qemu_path, command, args=None):
> +    """
> +    Wrapper function for running git bisect.
> +
> +    Parameters:
> +    qemu_path (str): QEMU path.
> +    command (str):   bisect command (start|fast|slow|reset).
> +    args (list):     Optional arguments.
> +
> +    Returns:
> +    (str):           git bisect stdout.
> +    """
> +    process = ["git", "bisect", command]
> +    if args:
> +        process += args
> +    bisect = subprocess.run(process,
> +                            cwd=qemu_path,
> +                            stdout=subprocess.PIPE,
> +                            stderr=subprocess.PIPE)
> +    if bisect.returncode:
> +        sys.exit(bisect.stderr.decode("utf-8"))
> +    return bisect.stdout.decode("utf-8")
> +
> +
> +def git_checkout(commit, qemu_path):
> +    """
> +    Wrapper function for checking out a given git commit.
> +
> +    Parameters:
> +    commit (str):    Commit hash of a git commit.
> +    qemu_path (str): QEMU path.
> +    """
> +    checkout_commit = subprocess.run(["git",
> +                                      "checkout",
> +                                      commit],
> +                                     cwd=qemu_path,
> +                                     stdout=subprocess.DEVNULL,
> +                                     stderr=subprocess.PIPE)
> +    if checkout_commit.returncode:
> +        sys.exit(checkout_commit.stderr.decode("utf-8"))
> +
> +
> +def git_clone(qemu_path):
> +    """
> +    Wrapper function for cloning QEMU git repo from GitHub.
> +
> +    Parameters:
> +    qemu_path (str): Path to clone the QEMU repo to.
> +    """
> +    clone_qemu = subprocess.run(["git",
> +                                 "clone",
> +                                 "https://github.com/qemu/qemu.git",
> +                                 qemu_path],
> +                                stderr=subprocess.STDOUT)
> +    if clone_qemu.returncode:
> +        sys.exit("Failed to clone QEMU!")
> +######################################################################
> +
> +
> +def check_requirements(tool):
> +    """
> +    Verify that all script requirements are installed (perf|callgrind &
> git).
> +
> +    Parameters:
> +    tool (str): Tool used for the measurement (perf or callgrind).
> +    """
> +    if tool == "perf":
> +        check_perf_installation = subprocess.run(["which", "perf"],
> +
>  stdout=subprocess.DEVNULL)
> +        if check_perf_installation.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",
> "/"],
> +
> stdout=subprocess.DEVNULL,
> +
> stderr=subprocess.DEVNULL)
> +        if check_perf_executability.returncode:
> +            sys.exit("""
> +        Error:
> +        You may not have permission to collect stats.
> +        Consider tweaking /proc/sys/kernel/perf_event_paranoid,
> +        which controls use of the performance events system by
> +        unprivileged users (without CAP_SYS_ADMIN).
> +        -1: Allow use of (almost) all events by all users
> +            Ignore mlock limit after perf_event_mlock_kb without
> CAP_IPC_LOCK
> +        0: Disallow ftrace function tracepoint by users without
> CAP_SYS_ADMIN
> +            Disallow raw tracepoint access by users without CAP_SYS_ADMIN
> +        1: Disallow CPU event access by users without CAP_SYS_ADMIN
> +        2: Disallow kernel profiling by users without CAP_SYS_ADMIN
> +        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.
> +        """)
> +    elif tool == "callgrind":
> +        check_valgrind_installation = subprocess.run(["which",
> "valgrind"],
> +
>  stdout=subprocess.DEVNULL)
> +        if check_valgrind_installation.returncode:
> +            sys.exit("Please install valgrind before running the script.")
> +
> +    # Insure that git is installed
> +    check_git_installation = subprocess.run(["which", "git"],
> +                                            stdout=subprocess.DEVNULL)
> +    if check_git_installation.returncode:
> +        sys.exit("Please install git before running the script.")
> +
> +
> +def make(qemu_build_path):
> +    """
> +    Build QEMU by running the Makefile.
> +
> +    Parameters:
> +    qemu_build_path (str): Path to the build directory with configuration
> files.
> +    """
> +    run_make = subprocess.run(["make",
> +                               "-j",
> +                               str(multiprocessing.cpu_count())],
> +                              cwd=qemu_build_path,
> +                              stdout=subprocess.DEVNULL,
> +                              stderr=subprocess.PIPE)
> +    if run_make.returncode:
> +        sys.exit(run_make.stderr.decode("utf-8"))
> +
> +
> +def measure_instructions(tool, qemu_exe_path, command):
> +    """
> +    Measure the number of instructions when running an program with QEMU.
> +
> +    Parameters:
> +    tool (str):          Tool used for the measurement (perf|callgrind).
> +    qemu_exe_path (str): Path to the QEMU executable of the equivalent
> target.
> +    command (list):      Program path and arguments.
> +
> +    Returns:
> +    (int):               Number of instructions.
> +    """
> +    if tool == "perf":
> +        run_perf = subprocess.run((["perf",
> +                                    "stat",
> +                                    "-x",
> +                                    " ",
> +                                    "-e",
> +                                    "instructions",
> +                                    qemu_exe_path]
> +                                   + command),
> +                                  stdout=subprocess.DEVNULL,
> +                                  stderr=subprocess.PIPE)
> +        if run_perf.returncode:
> +            sys.exit(run_perf.stderr.decode("utf-8"))
> +        else:
> +            perf_output = run_perf.stderr.decode("utf-8").split(" ")
> +            return int(perf_output[0])
> +
> +    elif tool == "callgrind":
> +        with tempfile.NamedTemporaryFile() as tmpfile:
> +            run_callgrind = subprocess.run((["valgrind",
> +                                             "--tool=callgrind",
> +                                             "--callgrind-out-file={}".
> format(
> +                                                 tmpfile.name),
> +                                             qemu_exe_path]
> +                                            + command),
> +                                           stdout=subprocess.DEVNULL,
> +                                           stderr=subprocess.PIPE)
> +        if run_callgrind.returncode:
> +            sys.exit(run_callgrind.stderr.decode("utf-8"))
> +        else:
> +            callgrind_output = run_callgrind.stderr.decode("
> utf-8").split("\n")
> +            return int(callgrind_output[8].split(" ")[-1])
> +
> +
> +def main():
> +    # Parse the command line arguments
> +    parser = argparse.ArgumentParser(
> +        usage="bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu
> QEMU] "
> +        "--target TARGET --tool {perf,callgrind} -- "
> +        "<target executable> [<target executable options>]")
> +
> +    parser.add_argument("-s", "--start", dest="start", type=str,
> required=True,
> +                        help="First commit hash in the search range")
> +    parser.add_argument("-e", "--end", dest="end", type=str,
> default="master",
> +                        help="Last commit hash in the search range")
> +    parser.add_argument("-q", "--qemu", dest="qemu", type=str, default="",
> +                        help="QEMU path")
> +    parser.add_argument("--target", dest="target", type=str,
> required=True,
> +                        help="QEMU target")
> +    parser.add_argument("--tool", dest="tool", choices=["perf",
> "callgrind"],
> +                        required=True, help="Tool used for measurements")
> +
> +    parser.add_argument("command", type=str, nargs="+",
> help=argparse.SUPPRESS)
> +
> +    args = parser.parse_args()
> +
> +    # Extract the needed variables from the args
> +    start_commit = args.start
> +    end_commit = args.end
> +    qemu = args.qemu
> +    target = args.target
> +    tool = args.tool
> +    command = args.command
> +
> +    # Set QEMU path
> +    if qemu == "":
> +        # Create a temp directory for cloning QEMU
> +        tmpdir = tempfile.TemporaryDirectory()
> +        qemu_path = os.path.join(tmpdir.name, "qemu")
> +
> +        # Clone QEMU into the temporary directory
> +        print("Fetching QEMU: ", end="", flush=True)
> +        git_clone(qemu_path)
> +        print()
> +    else:
> +        qemu_path = qemu
> +
> +    # Create the build directory
> +    qemu_build_path = os.path.join(qemu_path, "tmp-build-gcc")
> +    if not os.path.exists(qemu_build_path):
> +        os.mkdir(qemu_build_path)
> +    else:
> +        sys.exit("A build directory with the same name (tmp-build-gcc)
> used in "
> +                 "the script is already in the provided QEMU path.")
> +
> +    qemu_exe_path = os.path.join(qemu_build_path,
> +                                 "{}-linux-user".format(target),
> +                                 "qemu-{}".format(target))
> +
> +    # Configure QEMU
> +    configure = subprocess.run(["../configure",
> +                                "--target-list={}-linux-user".
> format(target)],
> +                               cwd=qemu_build_path,
> +                               stdout=subprocess.DEVNULL,
> +                               stderr=subprocess.PIPE)
> +    if configure.returncode:
> +        sys.exit(configure.stderr.decode("utf-8"))
> +
> +    # Do performance measurements for the start commit
> +    git_checkout(start_commit, qemu_path)
> +    make(qemu_build_path)
> +    start_commit_instructions = measure_instructions(tool,
> +                                                     qemu_exe_path,
> +                                                     command)
> +    print("{:<30} {}".format("Start Commit Instructions:",
> +                             format(start_commit_instructions, ",")))
> +
> +    # Do performance measurements for the end commit
> +    git_checkout(end_commit, qemu_path)
> +    make(qemu_build_path)
> +    end_commit_instructions = measure_instructions(tool,
> +                                                   qemu_exe_path,
> +                                                   command)
> +    print("{:<30} {}".format("End Commit Instructions:",
> +                             format(end_commit_instructions, ",")))
> +
> +    # Calculate performance difference between start and end commits
> +    performance_difference = \
> +        (start_commit_instructions - end_commit_instructions) / \
> +        max(end_commit_instructions, start_commit_instructions) * 100
> +    performance_change = "+" if performance_difference > 0 else "-"
> +    print("{:<30} {}".format("Performance Change:",
> +                             performance_change +
> +                             str(round(abs(performance_difference),
> 3))+"%"))
> +
> +    # Set the custom terms used for progressing in "git bisect"
> +    term_old = "fast" if performance_difference < 0 else "slow"
> +    term_new = "slow" if term_old == "fast" else "fast"
> +
> +    # Start git bisect
> +    git_bisect(qemu_path, "start", [
> +               "--term-old", term_old, "--term-new", term_new])
> +    # Set start commit state
> +    git_bisect(qemu_path, term_old, [start_commit])
> +    # Set end commit state
> +    bisect_output = git_bisect(qemu_path, term_new, [end_commit])
> +    # Print estimated bisect steps
> +    print("\n{:<30} {}\n".format(
> +        "Estimated Number of Steps:", bisect_output.split()[9]))
> +
> +    # Initialize bisect_count to track the number of performed
> +    bisect_count = 1
> +
> +    while True:
> +        print("**************BISECT STEP {}**************".format(
> bisect_count))
> +
> +        make(qemu_build_path)
> +
> +        instructions = measure_instructions(tool, qemu_exe_path, command)
> +        # Find the difference between the current instructions and
> start/end
> +        # instructions.
> +        diff_end = abs(instructions - end_commit_instructions)
> +        diff_start = abs(instructions - start_commit_instructions)
> +
> +        # If current number of insructions is closer to that of start,
> +        # set current commit as term_old.
> +        # Else, set current commit as term_new.
> +        if diff_end > diff_start:
> +            bisect_command = term_old
> +        else:
> +            bisect_command = term_new
> +
> +        print("{:<20} {}".format("Instructions:", format(instructions,
> ",")))
> +        print("{:<20} {}".format("Status:", "{}
> commit".format(bisect_command)))
> +
> +        bisect_output = git_bisect(qemu_path, bisect_command)
> +
> +        # Continue if still bisecting,
> +        # else, print result and break.
> +        if not bisect_output.split(" ")[0] == "Bisecting:":
> +            print("\n*****************BISECT RESULT*****************")
> +            commit_message_start = bisect_output.find("commit\n") + 7
> +            commit_message_end = bisect_output.find(":040000") - 1
> +            print(bisect_output[commit_message_start:commit_message_end])
> +            break
> +
> +        bisect_count += 1
> +
> +    # Reset git bisect
> +    git_bisect(qemu_path, "reset")
> +
> +    # Delete temp build directory
> +    shutil.rmtree(qemu_build_path)
> +
> +
> +if __name__ == "__main__":
> +    main()
> --
> 2.17.1
>
>

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

Re: [PATCH 1/1] scripts/performance: Add bisect.py script

[Ahmed Karaman]

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

On Sat, Jul 25, 2020 at 2:31 PM Aleksandar Markovic <
aleksandar.qemu.devel@gmail.com> wrote:

>
> Hi, Ahmed.
>
> Yes, somewhat related to John's hints on these comments, it is customary
> to have just a brief description before "Copyright" lines. This means one
> sentence, or a short paragraph (3-4 sentences max). The lenghty syntax
> commemt should be, in my opinion, moved after the license preamble, just
> before the start of real Python code.
>

Thanks Mr. John and Aleksandar for your feedback. I will update the script
accordingly.


>
> One question:
>
> What is the behavior in case of the executable architecture and "target"
> command line option mismatch (for example, one specifies m68k target, but
> passes hppa executable? Would that be detected before bisect search, or the
> bisect procedure will be applied even though such cases do not make sense?
>

The script will exit with an error of something along the lines of "Invalid
ELF image for this architecture".
This is done before starting "bisect" and after the initial "configure" and
"make".


> Yours, Aleksandar
>
>
>
>> +#  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 multiprocessing
>> +import tempfile
>> +import os
>> +import shutil
>> +import subprocess
>> +import sys
>> +
>> +
>> +############################ GIT WRAPPERS ############################
>> +def git_bisect(qemu_path, command, args=None):
>> +    """
>> +    Wrapper function for running git bisect.
>> +
>> +    Parameters:
>> +    qemu_path (str): QEMU path.
>> +    command (str):   bisect command (start|fast|slow|reset).
>> +    args (list):     Optional arguments.
>> +
>> +    Returns:
>> +    (str):           git bisect stdout.
>> +    """
>> +    process = ["git", "bisect", command]
>> +    if args:
>> +        process += args
>> +    bisect = subprocess.run(process,
>> +                            cwd=qemu_path,
>> +                            stdout=subprocess.PIPE,
>> +                            stderr=subprocess.PIPE)
>> +    if bisect.returncode:
>> +        sys.exit(bisect.stderr.decode("utf-8"))
>> +    return bisect.stdout.decode("utf-8")
>> +
>> +
>> +def git_checkout(commit, qemu_path):
>> +    """
>> +    Wrapper function for checking out a given git commit.
>> +
>> +    Parameters:
>> +    commit (str):    Commit hash of a git commit.
>> +    qemu_path (str): QEMU path.
>> +    """
>> +    checkout_commit = subprocess.run(["git",
>> +                                      "checkout",
>> +                                      commit],
>> +                                     cwd=qemu_path,
>> +                                     stdout=subprocess.DEVNULL,
>> +                                     stderr=subprocess.PIPE)
>> +    if checkout_commit.returncode:
>> +        sys.exit(checkout_commit.stderr.decode("utf-8"))
>> +
>> +
>> +def git_clone(qemu_path):
>> +    """
>> +    Wrapper function for cloning QEMU git repo from GitHub.
>> +
>> +    Parameters:
>> +    qemu_path (str): Path to clone the QEMU repo to.
>> +    """
>> +    clone_qemu = subprocess.run(["git",
>> +                                 "clone",
>> +                                 "https://github.com/qemu/qemu.git",
>> +                                 qemu_path],
>> +                                stderr=subprocess.STDOUT)
>> +    if clone_qemu.returncode:
>> +        sys.exit("Failed to clone QEMU!")
>> +######################################################################
>> +
>> +
>> +def check_requirements(tool):
>> +    """
>> +    Verify that all script requirements are installed (perf|callgrind &
>> git).
>> +
>> +    Parameters:
>> +    tool (str): Tool used for the measurement (perf or callgrind).
>> +    """
>> +    if tool == "perf":
>> +        check_perf_installation = subprocess.run(["which", "perf"],
>> +
>>  stdout=subprocess.DEVNULL)
>> +        if check_perf_installation.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",
>> "/"],
>> +
>> stdout=subprocess.DEVNULL,
>> +
>> stderr=subprocess.DEVNULL)
>> +        if check_perf_executability.returncode:
>> +            sys.exit("""
>> +        Error:
>> +        You may not have permission to collect stats.
>> +        Consider tweaking /proc/sys/kernel/perf_event_paranoid,
>> +        which controls use of the performance events system by
>> +        unprivileged users (without CAP_SYS_ADMIN).
>> +        -1: Allow use of (almost) all events by all users
>> +            Ignore mlock limit after perf_event_mlock_kb without
>> CAP_IPC_LOCK
>> +        0: Disallow ftrace function tracepoint by users without
>> CAP_SYS_ADMIN
>> +            Disallow raw tracepoint access by users without CAP_SYS_ADMIN
>> +        1: Disallow CPU event access by users without CAP_SYS_ADMIN
>> +        2: Disallow kernel profiling by users without CAP_SYS_ADMIN
>> +        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.
>> +        """)
>> +    elif tool == "callgrind":
>> +        check_valgrind_installation = subprocess.run(["which",
>> "valgrind"],
>> +
>>  stdout=subprocess.DEVNULL)
>> +        if check_valgrind_installation.returncode:
>> +            sys.exit("Please install valgrind before running the
>> script.")
>> +
>> +    # Insure that git is installed
>> +    check_git_installation = subprocess.run(["which", "git"],
>> +                                            stdout=subprocess.DEVNULL)
>> +    if check_git_installation.returncode:
>> +        sys.exit("Please install git before running the script.")
>> +
>> +
>> +def make(qemu_build_path):
>> +    """
>> +    Build QEMU by running the Makefile.
>> +
>> +    Parameters:
>> +    qemu_build_path (str): Path to the build directory with
>> configuration files.
>> +    """
>> +    run_make = subprocess.run(["make",
>> +                               "-j",
>> +                               str(multiprocessing.cpu_count())],
>> +                              cwd=qemu_build_path,
>> +                              stdout=subprocess.DEVNULL,
>> +                              stderr=subprocess.PIPE)
>> +    if run_make.returncode:
>> +        sys.exit(run_make.stderr.decode("utf-8"))
>> +
>> +
>> +def measure_instructions(tool, qemu_exe_path, command):
>> +    """
>> +    Measure the number of instructions when running an program with QEMU.
>> +
>> +    Parameters:
>> +    tool (str):          Tool used for the measurement (perf|callgrind).
>> +    qemu_exe_path (str): Path to the QEMU executable of the equivalent
>> target.
>> +    command (list):      Program path and arguments.
>> +
>> +    Returns:
>> +    (int):               Number of instructions.
>> +    """
>> +    if tool == "perf":
>> +        run_perf = subprocess.run((["perf",
>> +                                    "stat",
>> +                                    "-x",
>> +                                    " ",
>> +                                    "-e",
>> +                                    "instructions",
>> +                                    qemu_exe_path]
>> +                                   + command),
>> +                                  stdout=subprocess.DEVNULL,
>> +                                  stderr=subprocess.PIPE)
>> +        if run_perf.returncode:
>> +            sys.exit(run_perf.stderr.decode("utf-8"))
>> +        else:
>> +            perf_output = run_perf.stderr.decode("utf-8").split(" ")
>> +            return int(perf_output[0])
>> +
>> +    elif tool == "callgrind":
>> +        with tempfile.NamedTemporaryFile() as tmpfile:
>> +            run_callgrind = subprocess.run((["valgrind",
>> +                                             "--tool=callgrind",
>> +
>>  "--callgrind-out-file={}".format(
>> +                                                 tmpfile.name),
>> +                                             qemu_exe_path]
>> +                                            + command),
>> +                                           stdout=subprocess.DEVNULL,
>> +                                           stderr=subprocess.PIPE)
>> +        if run_callgrind.returncode:
>> +            sys.exit(run_callgrind.stderr.decode("utf-8"))
>> +        else:
>> +            callgrind_output =
>> run_callgrind.stderr.decode("utf-8").split("\n")
>> +            return int(callgrind_output[8].split(" ")[-1])
>> +
>> +
>> +def main():
>> +    # Parse the command line arguments
>> +    parser = argparse.ArgumentParser(
>> +        usage="bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu
>> QEMU] "
>> +        "--target TARGET --tool {perf,callgrind} -- "
>> +        "<target executable> [<target executable options>]")
>> +
>> +    parser.add_argument("-s", "--start", dest="start", type=str,
>> required=True,
>> +                        help="First commit hash in the search range")
>> +    parser.add_argument("-e", "--end", dest="end", type=str,
>> default="master",
>> +                        help="Last commit hash in the search range")
>> +    parser.add_argument("-q", "--qemu", dest="qemu", type=str,
>> default="",
>> +                        help="QEMU path")
>> +    parser.add_argument("--target", dest="target", type=str,
>> required=True,
>> +                        help="QEMU target")
>> +    parser.add_argument("--tool", dest="tool", choices=["perf",
>> "callgrind"],
>> +                        required=True, help="Tool used for measurements")
>> +
>> +    parser.add_argument("command", type=str, nargs="+",
>> help=argparse.SUPPRESS)
>> +
>> +    args = parser.parse_args()
>> +
>> +    # Extract the needed variables from the args
>> +    start_commit = args.start
>> +    end_commit = args.end
>> +    qemu = args.qemu
>> +    target = args.target
>> +    tool = args.tool
>> +    command = args.command
>> +
>> +    # Set QEMU path
>> +    if qemu == "":
>> +        # Create a temp directory for cloning QEMU
>> +        tmpdir = tempfile.TemporaryDirectory()
>> +        qemu_path = os.path.join(tmpdir.name, "qemu")
>> +
>> +        # Clone QEMU into the temporary directory
>> +        print("Fetching QEMU: ", end="", flush=True)
>> +        git_clone(qemu_path)
>> +        print()
>> +    else:
>> +        qemu_path = qemu
>> +
>> +    # Create the build directory
>> +    qemu_build_path = os.path.join(qemu_path, "tmp-build-gcc")
>> +    if not os.path.exists(qemu_build_path):
>> +        os.mkdir(qemu_build_path)
>> +    else:
>> +        sys.exit("A build directory with the same name (tmp-build-gcc)
>> used in "
>> +                 "the script is already in the provided QEMU path.")
>> +
>> +    qemu_exe_path = os.path.join(qemu_build_path,
>> +                                 "{}-linux-user".format(target),
>> +                                 "qemu-{}".format(target))
>> +
>> +    # Configure QEMU
>> +    configure = subprocess.run(["../configure",
>> +
>> "--target-list={}-linux-user".format(target)],
>> +                               cwd=qemu_build_path,
>> +                               stdout=subprocess.DEVNULL,
>> +                               stderr=subprocess.PIPE)
>> +    if configure.returncode:
>> +        sys.exit(configure.stderr.decode("utf-8"))
>> +
>> +    # Do performance measurements for the start commit
>> +    git_checkout(start_commit, qemu_path)
>> +    make(qemu_build_path)
>> +    start_commit_instructions = measure_instructions(tool,
>> +                                                     qemu_exe_path,
>> +                                                     command)
>> +    print("{:<30} {}".format("Start Commit Instructions:",
>> +                             format(start_commit_instructions, ",")))
>> +
>> +    # Do performance measurements for the end commit
>> +    git_checkout(end_commit, qemu_path)
>> +    make(qemu_build_path)
>> +    end_commit_instructions = measure_instructions(tool,
>> +                                                   qemu_exe_path,
>> +                                                   command)
>> +    print("{:<30} {}".format("End Commit Instructions:",
>> +                             format(end_commit_instructions, ",")))
>> +
>> +    # Calculate performance difference between start and end commits
>> +    performance_difference = \
>> +        (start_commit_instructions - end_commit_instructions) / \
>> +        max(end_commit_instructions, start_commit_instructions) * 100
>> +    performance_change = "+" if performance_difference > 0 else "-"
>> +    print("{:<30} {}".format("Performance Change:",
>> +                             performance_change +
>> +                             str(round(abs(performance_difference),
>> 3))+"%"))
>> +
>> +    # Set the custom terms used for progressing in "git bisect"
>> +    term_old = "fast" if performance_difference < 0 else "slow"
>> +    term_new = "slow" if term_old == "fast" else "fast"
>> +
>> +    # Start git bisect
>> +    git_bisect(qemu_path, "start", [
>> +               "--term-old", term_old, "--term-new", term_new])
>> +    # Set start commit state
>> +    git_bisect(qemu_path, term_old, [start_commit])
>> +    # Set end commit state
>> +    bisect_output = git_bisect(qemu_path, term_new, [end_commit])
>> +    # Print estimated bisect steps
>> +    print("\n{:<30} {}\n".format(
>> +        "Estimated Number of Steps:", bisect_output.split()[9]))
>> +
>> +    # Initialize bisect_count to track the number of performed
>> +    bisect_count = 1
>> +
>> +    while True:
>> +        print("**************BISECT STEP
>> {}**************".format(bisect_count))
>> +
>> +        make(qemu_build_path)
>> +
>> +        instructions = measure_instructions(tool, qemu_exe_path, command)
>> +        # Find the difference between the current instructions and
>> start/end
>> +        # instructions.
>> +        diff_end = abs(instructions - end_commit_instructions)
>> +        diff_start = abs(instructions - start_commit_instructions)
>> +
>> +        # If current number of insructions is closer to that of start,
>> +        # set current commit as term_old.
>> +        # Else, set current commit as term_new.
>> +        if diff_end > diff_start:
>> +            bisect_command = term_old
>> +        else:
>> +            bisect_command = term_new
>> +
>> +        print("{:<20} {}".format("Instructions:", format(instructions,
>> ",")))
>> +        print("{:<20} {}".format("Status:", "{}
>> commit".format(bisect_command)))
>> +
>> +        bisect_output = git_bisect(qemu_path, bisect_command)
>> +
>> +        # Continue if still bisecting,
>> +        # else, print result and break.
>> +        if not bisect_output.split(" ")[0] == "Bisecting:":
>> +            print("\n*****************BISECT RESULT*****************")
>> +            commit_message_start = bisect_output.find("commit\n") + 7
>> +            commit_message_end = bisect_output.find(":040000") - 1
>> +            print(bisect_output[commit_message_start:commit_message_end])
>> +            break
>> +
>> +        bisect_count += 1
>> +
>> +    # Reset git bisect
>> +    git_bisect(qemu_path, "reset")
>> +
>> +    # Delete temp build directory
>> +    shutil.rmtree(qemu_build_path)
>> +
>> +
>> +if __name__ == "__main__":
>> +    main()
>> --
>> 2.17.1
>>
>>

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

Re: [PATCH 1/1] scripts/performance: Add bisect.py script

[Aleksandar Markovic]

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

On Saturday, July 25, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
wrote:

> On Sat, Jul 25, 2020 at 2:31 PM Aleksandar Markovic <
> aleksandar.qemu.devel@gmail.com> wrote:
>
>>
>> Hi, Ahmed.
>>
>> Yes, somewhat related to John's hints on these comments, it is customary
>> to have just a brief description before "Copyright" lines. This means one
>> sentence, or a short paragraph (3-4 sentences max). The lenghty syntax
>> commemt should be, in my opinion, moved after the license preamble, just
>> before the start of real Python code.
>>
>
> Thanks Mr. John and Aleksandar for your feedback. I will update the script
> accordingly.
>
>
>>
>> One question:
>>
>> What is the behavior in case of the executable architecture and "target"
>> command line option mismatch (for example, one specifies m68k target, but
>> passes hppa executable? Would that be detected before bisect search, or the
>> bisect procedure will be applied even though such cases do not make sense?
>>
>
> The script will exit with an error of something along the lines of
> "Invalid ELF image for this architecture".
> This is done before starting "bisect" and after the initial "configure"
> and "make".
>
>
This is good enough (the moment of detection). However, are all cleanups
done? Is temporary directory deleted?

The same questions for the scenario where the user specifies non-existant
commit ID as the start or the end commit.

Does the script work if user specifies a tag, instead of commit ID? I think
it should work. For example, can the user specify v3.1.0 as start commit,
and v4.2.0 as the end commit, in order to detect degradation/improvement
between QEMU 3.1 and QEMU 4.2? Please test if such scenario works. If it
works, I think you should insert "commit ID or tag ID" instead of "commit"
only in the commit massage and applicable code comments (including also the
user-visible help outputed on "-h").

Lastly, what happens if specified start and end commits are existant, but
in the wrong order (end is "before" start)?

Thanks,
Aleksandar





>
>> Yours, Aleksandar
>>
>>
>>
>>> +#  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 multiprocessing
>>> +import tempfile
>>> +import os
>>> +import shutil
>>> +import subprocess
>>> +import sys
>>> +
>>> +
>>> +############################ GIT WRAPPERS ############################
>>> +def git_bisect(qemu_path, command, args=None):
>>> +    """
>>> +    Wrapper function for running git bisect.
>>> +
>>> +    Parameters:
>>> +    qemu_path (str): QEMU path.
>>> +    command (str):   bisect command (start|fast|slow|reset).
>>> +    args (list):     Optional arguments.
>>> +
>>> +    Returns:
>>> +    (str):           git bisect stdout.
>>> +    """
>>> +    process = ["git", "bisect", command]
>>> +    if args:
>>> +        process += args
>>> +    bisect = subprocess.run(process,
>>> +                            cwd=qemu_path,
>>> +                            stdout=subprocess.PIPE,
>>> +                            stderr=subprocess.PIPE)
>>> +    if bisect.returncode:
>>> +        sys.exit(bisect.stderr.decode("utf-8"))
>>> +    return bisect.stdout.decode("utf-8")
>>> +
>>> +
>>> +def git_checkout(commit, qemu_path):
>>> +    """
>>> +    Wrapper function for checking out a given git commit.
>>> +
>>> +    Parameters:
>>> +    commit (str):    Commit hash of a git commit.
>>> +    qemu_path (str): QEMU path.
>>> +    """
>>> +    checkout_commit = subprocess.run(["git",
>>> +                                      "checkout",
>>> +                                      commit],
>>> +                                     cwd=qemu_path,
>>> +                                     stdout=subprocess.DEVNULL,
>>> +                                     stderr=subprocess.PIPE)
>>> +    if checkout_commit.returncode:
>>> +        sys.exit(checkout_commit.stderr.decode("utf-8"))
>>> +
>>> +
>>> +def git_clone(qemu_path):
>>> +    """
>>> +    Wrapper function for cloning QEMU git repo from GitHub.
>>> +
>>> +    Parameters:
>>> +    qemu_path (str): Path to clone the QEMU repo to.
>>> +    """
>>> +    clone_qemu = subprocess.run(["git",
>>> +                                 "clone",
>>> +                                 "https://github.com/qemu/qemu.git",
>>> +                                 qemu_path],
>>> +                                stderr=subprocess.STDOUT)
>>> +    if clone_qemu.returncode:
>>> +        sys.exit("Failed to clone QEMU!")
>>> +######################################################################
>>> +
>>> +
>>> +def check_requirements(tool):
>>> +    """
>>> +    Verify that all script requirements are installed (perf|callgrind &
>>> git).
>>> +
>>> +    Parameters:
>>> +    tool (str): Tool used for the measurement (perf or callgrind).
>>> +    """
>>> +    if tool == "perf":
>>> +        check_perf_installation = subprocess.run(["which", "perf"],
>>> +
>>>  stdout=subprocess.DEVNULL)
>>> +        if check_perf_installation.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", "/"],
>>> +
>>> stdout=subprocess.DEVNULL,
>>> +
>>> stderr=subprocess.DEVNULL)
>>> +        if check_perf_executability.returncode:
>>> +            sys.exit("""
>>> +        Error:
>>> +        You may not have permission to collect stats.
>>> +        Consider tweaking /proc/sys/kernel/perf_event_paranoid,
>>> +        which controls use of the performance events system by
>>> +        unprivileged users (without CAP_SYS_ADMIN).
>>> +        -1: Allow use of (almost) all events by all users
>>> +            Ignore mlock limit after perf_event_mlock_kb without
>>> CAP_IPC_LOCK
>>> +        0: Disallow ftrace function tracepoint by users without
>>> CAP_SYS_ADMIN
>>> +            Disallow raw tracepoint access by users without
>>> CAP_SYS_ADMIN
>>> +        1: Disallow CPU event access by users without CAP_SYS_ADMIN
>>> +        2: Disallow kernel profiling by users without CAP_SYS_ADMIN
>>> +        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.
>>> +        """)
>>> +    elif tool == "callgrind":
>>> +        check_valgrind_installation = subprocess.run(["which",
>>> "valgrind"],
>>> +
>>>  stdout=subprocess.DEVNULL)
>>> +        if check_valgrind_installation.returncode:
>>> +            sys.exit("Please install valgrind before running the
>>> script.")
>>> +
>>> +    # Insure that git is installed
>>> +    check_git_installation = subprocess.run(["which", "git"],
>>> +                                            stdout=subprocess.DEVNULL)
>>> +    if check_git_installation.returncode:
>>> +        sys.exit("Please install git before running the script.")
>>> +
>>> +
>>> +def make(qemu_build_path):
>>> +    """
>>> +    Build QEMU by running the Makefile.
>>> +
>>> +    Parameters:
>>> +    qemu_build_path (str): Path to the build directory with
>>> configuration files.
>>> +    """
>>> +    run_make = subprocess.run(["make",
>>> +                               "-j",
>>> +                               str(multiprocessing.cpu_count())],
>>> +                              cwd=qemu_build_path,
>>> +                              stdout=subprocess.DEVNULL,
>>> +                              stderr=subprocess.PIPE)
>>> +    if run_make.returncode:
>>> +        sys.exit(run_make.stderr.decode("utf-8"))
>>> +
>>> +
>>> +def measure_instructions(tool, qemu_exe_path, command):
>>> +    """
>>> +    Measure the number of instructions when running an program with
>>> QEMU.
>>> +
>>> +    Parameters:
>>> +    tool (str):          Tool used for the measurement (perf|callgrind).
>>> +    qemu_exe_path (str): Path to the QEMU executable of the equivalent
>>> target.
>>> +    command (list):      Program path and arguments.
>>> +
>>> +    Returns:
>>> +    (int):               Number of instructions.
>>> +    """
>>> +    if tool == "perf":
>>> +        run_perf = subprocess.run((["perf",
>>> +                                    "stat",
>>> +                                    "-x",
>>> +                                    " ",
>>> +                                    "-e",
>>> +                                    "instructions",
>>> +                                    qemu_exe_path]
>>> +                                   + command),
>>> +                                  stdout=subprocess.DEVNULL,
>>> +                                  stderr=subprocess.PIPE)
>>> +        if run_perf.returncode:
>>> +            sys.exit(run_perf.stderr.decode("utf-8"))
>>> +        else:
>>> +            perf_output = run_perf.stderr.decode("utf-8").split(" ")
>>> +            return int(perf_output[0])
>>> +
>>> +    elif tool == "callgrind":
>>> +        with tempfile.NamedTemporaryFile() as tmpfile:
>>> +            run_callgrind = subprocess.run((["valgrind",
>>> +                                             "--tool=callgrind",
>>> +                                             "--callgrind-out-file={}".
>>> format(
>>> +                                                 tmpfile.name),
>>> +                                             qemu_exe_path]
>>> +                                            + command),
>>> +                                           stdout=subprocess.DEVNULL,
>>> +                                           stderr=subprocess.PIPE)
>>> +        if run_callgrind.returncode:
>>> +            sys.exit(run_callgrind.stderr.decode("utf-8"))
>>> +        else:
>>> +            callgrind_output = run_callgrind.stderr.decode("
>>> utf-8").split("\n")
>>> +            return int(callgrind_output[8].split(" ")[-1])
>>> +
>>> +
>>> +def main():
>>> +    # Parse the command line arguments
>>> +    parser = argparse.ArgumentParser(
>>> +        usage="bisect.py [-h] -s,--start START [-e,--end END]
>>> [-q,--qemu QEMU] "
>>> +        "--target TARGET --tool {perf,callgrind} -- "
>>> +        "<target executable> [<target executable options>]")
>>> +
>>> +    parser.add_argument("-s", "--start", dest="start", type=str,
>>> required=True,
>>> +                        help="First commit hash in the search range")
>>> +    parser.add_argument("-e", "--end", dest="end", type=str,
>>> default="master",
>>> +                        help="Last commit hash in the search range")
>>> +    parser.add_argument("-q", "--qemu", dest="qemu", type=str,
>>> default="",
>>> +                        help="QEMU path")
>>> +    parser.add_argument("--target", dest="target", type=str,
>>> required=True,
>>> +                        help="QEMU target")
>>> +    parser.add_argument("--tool", dest="tool", choices=["perf",
>>> "callgrind"],
>>> +                        required=True, help="Tool used for
>>> measurements")
>>> +
>>> +    parser.add_argument("command", type=str, nargs="+",
>>> help=argparse.SUPPRESS)
>>> +
>>> +    args = parser.parse_args()
>>> +
>>> +    # Extract the needed variables from the args
>>> +    start_commit = args.start
>>> +    end_commit = args.end
>>> +    qemu = args.qemu
>>> +    target = args.target
>>> +    tool = args.tool
>>> +    command = args.command
>>> +
>>> +    # Set QEMU path
>>> +    if qemu == "":
>>> +        # Create a temp directory for cloning QEMU
>>> +        tmpdir = tempfile.TemporaryDirectory()
>>> +        qemu_path = os.path.join(tmpdir.name, "qemu")
>>> +
>>> +        # Clone QEMU into the temporary directory
>>> +        print("Fetching QEMU: ", end="", flush=True)
>>> +        git_clone(qemu_path)
>>> +        print()
>>> +    else:
>>> +        qemu_path = qemu
>>> +
>>> +    # Create the build directory
>>> +    qemu_build_path = os.path.join(qemu_path, "tmp-build-gcc")
>>> +    if not os.path.exists(qemu_build_path):
>>> +        os.mkdir(qemu_build_path)
>>> +    else:
>>> +        sys.exit("A build directory with the same name (tmp-build-gcc)
>>> used in "
>>> +                 "the script is already in the provided QEMU path.")
>>> +
>>> +    qemu_exe_path = os.path.join(qemu_build_path,
>>> +                                 "{}-linux-user".format(target),
>>> +                                 "qemu-{}".format(target))
>>> +
>>> +    # Configure QEMU
>>> +    configure = subprocess.run(["../configure",
>>> +                                "--target-list={}-linux-user".
>>> format(target)],
>>> +                               cwd=qemu_build_path,
>>> +                               stdout=subprocess.DEVNULL,
>>> +                               stderr=subprocess.PIPE)
>>> +    if configure.returncode:
>>> +        sys.exit(configure.stderr.decode("utf-8"))
>>> +
>>> +    # Do performance measurements for the start commit
>>> +    git_checkout(start_commit, qemu_path)
>>> +    make(qemu_build_path)
>>> +    start_commit_instructions = measure_instructions(tool,
>>> +                                                     qemu_exe_path,
>>> +                                                     command)
>>> +    print("{:<30} {}".format("Start Commit Instructions:",
>>> +                             format(start_commit_instructions, ",")))
>>> +
>>> +    # Do performance measurements for the end commit
>>> +    git_checkout(end_commit, qemu_path)
>>> +    make(qemu_build_path)
>>> +    end_commit_instructions = measure_instructions(tool,
>>> +                                                   qemu_exe_path,
>>> +                                                   command)
>>> +    print("{:<30} {}".format("End Commit Instructions:",
>>> +                             format(end_commit_instructions, ",")))
>>> +
>>> +    # Calculate performance difference between start and end commits
>>> +    performance_difference = \
>>> +        (start_commit_instructions - end_commit_instructions) / \
>>> +        max(end_commit_instructions, start_commit_instructions) * 100
>>> +    performance_change = "+" if performance_difference > 0 else "-"
>>> +    print("{:<30} {}".format("Performance Change:",
>>> +                             performance_change +
>>> +                             str(round(abs(performance_difference),
>>> 3))+"%"))
>>> +
>>> +    # Set the custom terms used for progressing in "git bisect"
>>> +    term_old = "fast" if performance_difference < 0 else "slow"
>>> +    term_new = "slow" if term_old == "fast" else "fast"
>>> +
>>> +    # Start git bisect
>>> +    git_bisect(qemu_path, "start", [
>>> +               "--term-old", term_old, "--term-new", term_new])
>>> +    # Set start commit state
>>> +    git_bisect(qemu_path, term_old, [start_commit])
>>> +    # Set end commit state
>>> +    bisect_output = git_bisect(qemu_path, term_new, [end_commit])
>>> +    # Print estimated bisect steps
>>> +    print("\n{:<30} {}\n".format(
>>> +        "Estimated Number of Steps:", bisect_output.split()[9]))
>>> +
>>> +    # Initialize bisect_count to track the number of performed
>>> +    bisect_count = 1
>>> +
>>> +    while True:
>>> +        print("**************BISECT STEP {}**************".format(
>>> bisect_count))
>>> +
>>> +        make(qemu_build_path)
>>> +
>>> +        instructions = measure_instructions(tool, qemu_exe_path,
>>> command)
>>> +        # Find the difference between the current instructions and
>>> start/end
>>> +        # instructions.
>>> +        diff_end = abs(instructions - end_commit_instructions)
>>> +        diff_start = abs(instructions - start_commit_instructions)
>>> +
>>> +        # If current number of insructions is closer to that of start,
>>> +        # set current commit as term_old.
>>> +        # Else, set current commit as term_new.
>>> +        if diff_end > diff_start:
>>> +            bisect_command = term_old
>>> +        else:
>>> +            bisect_command = term_new
>>> +
>>> +        print("{:<20} {}".format("Instructions:", format(instructions,
>>> ",")))
>>> +        print("{:<20} {}".format("Status:", "{}
>>> commit".format(bisect_command)))
>>> +
>>> +        bisect_output = git_bisect(qemu_path, bisect_command)
>>> +
>>> +        # Continue if still bisecting,
>>> +        # else, print result and break.
>>> +        if not bisect_output.split(" ")[0] == "Bisecting:":
>>> +            print("\n*****************BISECT RESULT*****************")
>>> +            commit_message_start = bisect_output.find("commit\n") + 7
>>> +            commit_message_end = bisect_output.find(":040000") - 1
>>> +            print(bisect_output[commit_message_start:commit_message_
>>> end])
>>> +            break
>>> +
>>> +        bisect_count += 1
>>> +
>>> +    # Reset git bisect
>>> +    git_bisect(qemu_path, "reset")
>>> +
>>> +    # Delete temp build directory
>>> +    shutil.rmtree(qemu_build_path)
>>> +
>>> +
>>> +if __name__ == "__main__":
>>> +    main()
>>> --
>>> 2.17.1
>>>
>>>

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

Re: [PATCH 1/1] scripts/performance: Add bisect.py script

[Ahmed Karaman]

On Sat, Jul 25, 2020 at 9:48 PM Aleksandar Markovic
<aleksandar.qemu.devel@gmail.com> wrote:
>
>
>
> On Saturday, July 25, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com> wrote:
>>
>> On Sat, Jul 25, 2020 at 2:31 PM Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> wrote:
>>>
>>>
>>> Hi, Ahmed.
>>>
>>> Yes, somewhat related to John's hints on these comments, it is customary to have just a brief description before "Copyright" lines. This means one sentence, or a short paragraph (3-4 sentences max). The lenghty syntax commemt should be, in my opinion, moved after the license preamble, just before the start of real Python code.
>>
>>
>> Thanks Mr. John and Aleksandar for your feedback. I will update the script accordingly.
>>
>>>
>>>
>>> One question:
>>>
>>> What is the behavior in case of the executable architecture and "target" command line option mismatch (for example, one specifies m68k target, but passes hppa executable? Would that be detected before bisect search, or the bisect procedure will be applied even though such cases do not make sense?
>>
>>
>> The script will exit with an error of something along the lines of "Invalid ELF image for this architecture".
>> This is done before starting "bisect" and after the initial "configure" and "make".
>>
>
> This is good enough (the moment of detection). However, are all cleanups done? Is temporary directory deleted?

This is a thing I missed, I will add a clean_up() function to be
called before any exit.

>
> The same questions for the scenario where the user specifies non-existant commit ID as the start or the end commit.
>

The script will exit with a message from "git" saying that this ID
doesn't exist. This will be done during the initial measurements of
the two boundary commits which is also before the bisect process.

> Does the script work if user specifies a tag, instead of commit ID? I think it should work. For example, can the user specify v3.1.0 as start commit, and v4.2.0 as the end commit, in order to detect degradation/improvement between QEMU 3.1 and QEMU 4.2? Please test if such scenario works. If it works, I think you should insert "commit ID or tag ID" instead of "commit" only in the commit massage and applicable code comments (including also the user-visible help outputed on "-h").

Yes, tags also work. Basically, anything that works with "git bisect"
as "start" and "end" values works with the script.

>
> Lastly, what happens if specified start and end commits are existant, but in the wrong order (end is "before" start)?

The script will also exit with an error before starting the bisect
process. The error would say:
"Some slow revs are not ancestors of the fast rev.
git bisect cannot work properly in this case.
Maybe you mistook slow and fast revs?"


>
> Thanks,
> Aleksandar
>
>
>
>
>>>
>>>
>>> Yours, Aleksandar
>>>
>>>
>>>>
>>>> +#  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 multiprocessing
>>>> +import tempfile
>>>> +import os
>>>> +import shutil
>>>> +import subprocess
>>>> +import sys
>>>> +
>>>> +
>>>> +############################ GIT WRAPPERS ############################
>>>> +def git_bisect(qemu_path, command, args=None):
>>>> +    """
>>>> +    Wrapper function for running git bisect.
>>>> +
>>>> +    Parameters:
>>>> +    qemu_path (str): QEMU path.
>>>> +    command (str):   bisect command (start|fast|slow|reset).
>>>> +    args (list):     Optional arguments.
>>>> +
>>>> +    Returns:
>>>> +    (str):           git bisect stdout.
>>>> +    """
>>>> +    process = ["git", "bisect", command]
>>>> +    if args:
>>>> +        process += args
>>>> +    bisect = subprocess.run(process,
>>>> +                            cwd=qemu_path,
>>>> +                            stdout=subprocess.PIPE,
>>>> +                            stderr=subprocess.PIPE)
>>>> +    if bisect.returncode:
>>>> +        sys.exit(bisect.stderr.decode("utf-8"))
>>>> +    return bisect.stdout.decode("utf-8")
>>>> +
>>>> +
>>>> +def git_checkout(commit, qemu_path):
>>>> +    """
>>>> +    Wrapper function for checking out a given git commit.
>>>> +
>>>> +    Parameters:
>>>> +    commit (str):    Commit hash of a git commit.
>>>> +    qemu_path (str): QEMU path.
>>>> +    """
>>>> +    checkout_commit = subprocess.run(["git",
>>>> +                                      "checkout",
>>>> +                                      commit],
>>>> +                                     cwd=qemu_path,
>>>> +                                     stdout=subprocess.DEVNULL,
>>>> +                                     stderr=subprocess.PIPE)
>>>> +    if checkout_commit.returncode:
>>>> +        sys.exit(checkout_commit.stderr.decode("utf-8"))
>>>> +
>>>> +
>>>> +def git_clone(qemu_path):
>>>> +    """
>>>> +    Wrapper function for cloning QEMU git repo from GitHub.
>>>> +
>>>> +    Parameters:
>>>> +    qemu_path (str): Path to clone the QEMU repo to.
>>>> +    """
>>>> +    clone_qemu = subprocess.run(["git",
>>>> +                                 "clone",
>>>> +                                 "https://github.com/qemu/qemu.git",
>>>> +                                 qemu_path],
>>>> +                                stderr=subprocess.STDOUT)
>>>> +    if clone_qemu.returncode:
>>>> +        sys.exit("Failed to clone QEMU!")
>>>> +######################################################################
>>>> +
>>>> +
>>>> +def check_requirements(tool):
>>>> +    """
>>>> +    Verify that all script requirements are installed (perf|callgrind & git).
>>>> +
>>>> +    Parameters:
>>>> +    tool (str): Tool used for the measurement (perf or callgrind).
>>>> +    """
>>>> +    if tool == "perf":
>>>> +        check_perf_installation = subprocess.run(["which", "perf"],
>>>> +                                                 stdout=subprocess.DEVNULL)
>>>> +        if check_perf_installation.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", "/"],
>>>> +                                                  stdout=subprocess.DEVNULL,
>>>> +                                                  stderr=subprocess.DEVNULL)
>>>> +        if check_perf_executability.returncode:
>>>> +            sys.exit("""
>>>> +        Error:
>>>> +        You may not have permission to collect stats.
>>>> +        Consider tweaking /proc/sys/kernel/perf_event_paranoid,
>>>> +        which controls use of the performance events system by
>>>> +        unprivileged users (without CAP_SYS_ADMIN).
>>>> +        -1: Allow use of (almost) all events by all users
>>>> +            Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK
>>>> +        0: Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN
>>>> +            Disallow raw tracepoint access by users without CAP_SYS_ADMIN
>>>> +        1: Disallow CPU event access by users without CAP_SYS_ADMIN
>>>> +        2: Disallow kernel profiling by users without CAP_SYS_ADMIN
>>>> +        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.
>>>> +        """)
>>>> +    elif tool == "callgrind":
>>>> +        check_valgrind_installation = subprocess.run(["which", "valgrind"],
>>>> +                                                     stdout=subprocess.DEVNULL)
>>>> +        if check_valgrind_installation.returncode:
>>>> +            sys.exit("Please install valgrind before running the script.")
>>>> +
>>>> +    # Insure that git is installed
>>>> +    check_git_installation = subprocess.run(["which", "git"],
>>>> +                                            stdout=subprocess.DEVNULL)
>>>> +    if check_git_installation.returncode:
>>>> +        sys.exit("Please install git before running the script.")
>>>> +
>>>> +
>>>> +def make(qemu_build_path):
>>>> +    """
>>>> +    Build QEMU by running the Makefile.
>>>> +
>>>> +    Parameters:
>>>> +    qemu_build_path (str): Path to the build directory with configuration files.
>>>> +    """
>>>> +    run_make = subprocess.run(["make",
>>>> +                               "-j",
>>>> +                               str(multiprocessing.cpu_count())],
>>>> +                              cwd=qemu_build_path,
>>>> +                              stdout=subprocess.DEVNULL,
>>>> +                              stderr=subprocess.PIPE)
>>>> +    if run_make.returncode:
>>>> +        sys.exit(run_make.stderr.decode("utf-8"))
>>>> +
>>>> +
>>>> +def measure_instructions(tool, qemu_exe_path, command):
>>>> +    """
>>>> +    Measure the number of instructions when running an program with QEMU.
>>>> +
>>>> +    Parameters:
>>>> +    tool (str):          Tool used for the measurement (perf|callgrind).
>>>> +    qemu_exe_path (str): Path to the QEMU executable of the equivalent target.
>>>> +    command (list):      Program path and arguments.
>>>> +
>>>> +    Returns:
>>>> +    (int):               Number of instructions.
>>>> +    """
>>>> +    if tool == "perf":
>>>> +        run_perf = subprocess.run((["perf",
>>>> +                                    "stat",
>>>> +                                    "-x",
>>>> +                                    " ",
>>>> +                                    "-e",
>>>> +                                    "instructions",
>>>> +                                    qemu_exe_path]
>>>> +                                   + command),
>>>> +                                  stdout=subprocess.DEVNULL,
>>>> +                                  stderr=subprocess.PIPE)
>>>> +        if run_perf.returncode:
>>>> +            sys.exit(run_perf.stderr.decode("utf-8"))
>>>> +        else:
>>>> +            perf_output = run_perf.stderr.decode("utf-8").split(" ")
>>>> +            return int(perf_output[0])
>>>> +
>>>> +    elif tool == "callgrind":
>>>> +        with tempfile.NamedTemporaryFile() as tmpfile:
>>>> +            run_callgrind = subprocess.run((["valgrind",
>>>> +                                             "--tool=callgrind",
>>>> +                                             "--callgrind-out-file={}".format(
>>>> +                                                 tmpfile.name),
>>>> +                                             qemu_exe_path]
>>>> +                                            + command),
>>>> +                                           stdout=subprocess.DEVNULL,
>>>> +                                           stderr=subprocess.PIPE)
>>>> +        if run_callgrind.returncode:
>>>> +            sys.exit(run_callgrind.stderr.decode("utf-8"))
>>>> +        else:
>>>> +            callgrind_output = run_callgrind.stderr.decode("utf-8").split("\n")
>>>> +            return int(callgrind_output[8].split(" ")[-1])
>>>> +
>>>> +
>>>> +def main():
>>>> +    # Parse the command line arguments
>>>> +    parser = argparse.ArgumentParser(
>>>> +        usage="bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] "
>>>> +        "--target TARGET --tool {perf,callgrind} -- "
>>>> +        "<target executable> [<target executable options>]")
>>>> +
>>>> +    parser.add_argument("-s", "--start", dest="start", type=str, required=True,
>>>> +                        help="First commit hash in the search range")
>>>> +    parser.add_argument("-e", "--end", dest="end", type=str, default="master",
>>>> +                        help="Last commit hash in the search range")
>>>> +    parser.add_argument("-q", "--qemu", dest="qemu", type=str, default="",
>>>> +                        help="QEMU path")
>>>> +    parser.add_argument("--target", dest="target", type=str, required=True,
>>>> +                        help="QEMU target")
>>>> +    parser.add_argument("--tool", dest="tool", choices=["perf", "callgrind"],
>>>> +                        required=True, help="Tool used for measurements")
>>>> +
>>>> +    parser.add_argument("command", type=str, nargs="+", help=argparse.SUPPRESS)
>>>> +
>>>> +    args = parser.parse_args()
>>>> +
>>>> +    # Extract the needed variables from the args
>>>> +    start_commit = args.start
>>>> +    end_commit = args.end
>>>> +    qemu = args.qemu
>>>> +    target = args.target
>>>> +    tool = args.tool
>>>> +    command = args.command
>>>> +
>>>> +    # Set QEMU path
>>>> +    if qemu == "":
>>>> +        # Create a temp directory for cloning QEMU
>>>> +        tmpdir = tempfile.TemporaryDirectory()
>>>> +        qemu_path = os.path.join(tmpdir.name, "qemu")
>>>> +
>>>> +        # Clone QEMU into the temporary directory
>>>> +        print("Fetching QEMU: ", end="", flush=True)
>>>> +        git_clone(qemu_path)
>>>> +        print()
>>>> +    else:
>>>> +        qemu_path = qemu
>>>> +
>>>> +    # Create the build directory
>>>> +    qemu_build_path = os.path.join(qemu_path, "tmp-build-gcc")
>>>> +    if not os.path.exists(qemu_build_path):
>>>> +        os.mkdir(qemu_build_path)
>>>> +    else:
>>>> +        sys.exit("A build directory with the same name (tmp-build-gcc) used in "
>>>> +                 "the script is already in the provided QEMU path.")
>>>> +
>>>> +    qemu_exe_path = os.path.join(qemu_build_path,
>>>> +                                 "{}-linux-user".format(target),
>>>> +                                 "qemu-{}".format(target))
>>>> +
>>>> +    # Configure QEMU
>>>> +    configure = subprocess.run(["../configure",
>>>> +                                "--target-list={}-linux-user".format(target)],
>>>> +                               cwd=qemu_build_path,
>>>> +                               stdout=subprocess.DEVNULL,
>>>> +                               stderr=subprocess.PIPE)
>>>> +    if configure.returncode:
>>>> +        sys.exit(configure.stderr.decode("utf-8"))
>>>> +
>>>> +    # Do performance measurements for the start commit
>>>> +    git_checkout(start_commit, qemu_path)
>>>> +    make(qemu_build_path)
>>>> +    start_commit_instructions = measure_instructions(tool,
>>>> +                                                     qemu_exe_path,
>>>> +                                                     command)
>>>> +    print("{:<30} {}".format("Start Commit Instructions:",
>>>> +                             format(start_commit_instructions, ",")))
>>>> +
>>>> +    # Do performance measurements for the end commit
>>>> +    git_checkout(end_commit, qemu_path)
>>>> +    make(qemu_build_path)
>>>> +    end_commit_instructions = measure_instructions(tool,
>>>> +                                                   qemu_exe_path,
>>>> +                                                   command)
>>>> +    print("{:<30} {}".format("End Commit Instructions:",
>>>> +                             format(end_commit_instructions, ",")))
>>>> +
>>>> +    # Calculate performance difference between start and end commits
>>>> +    performance_difference = \
>>>> +        (start_commit_instructions - end_commit_instructions) / \
>>>> +        max(end_commit_instructions, start_commit_instructions) * 100
>>>> +    performance_change = "+" if performance_difference > 0 else "-"
>>>> +    print("{:<30} {}".format("Performance Change:",
>>>> +                             performance_change +
>>>> +                             str(round(abs(performance_difference), 3))+"%"))
>>>> +
>>>> +    # Set the custom terms used for progressing in "git bisect"
>>>> +    term_old = "fast" if performance_difference < 0 else "slow"
>>>> +    term_new = "slow" if term_old == "fast" else "fast"
>>>> +
>>>> +    # Start git bisect
>>>> +    git_bisect(qemu_path, "start", [
>>>> +               "--term-old", term_old, "--term-new", term_new])
>>>> +    # Set start commit state
>>>> +    git_bisect(qemu_path, term_old, [start_commit])
>>>> +    # Set end commit state
>>>> +    bisect_output = git_bisect(qemu_path, term_new, [end_commit])
>>>> +    # Print estimated bisect steps
>>>> +    print("\n{:<30} {}\n".format(
>>>> +        "Estimated Number of Steps:", bisect_output.split()[9]))
>>>> +
>>>> +    # Initialize bisect_count to track the number of performed
>>>> +    bisect_count = 1
>>>> +
>>>> +    while True:
>>>> +        print("**************BISECT STEP {}**************".format(bisect_count))
>>>> +
>>>> +        make(qemu_build_path)
>>>> +
>>>> +        instructions = measure_instructions(tool, qemu_exe_path, command)
>>>> +        # Find the difference between the current instructions and start/end
>>>> +        # instructions.
>>>> +        diff_end = abs(instructions - end_commit_instructions)
>>>> +        diff_start = abs(instructions - start_commit_instructions)
>>>> +
>>>> +        # If current number of insructions is closer to that of start,
>>>> +        # set current commit as term_old.
>>>> +        # Else, set current commit as term_new.
>>>> +        if diff_end > diff_start:
>>>> +            bisect_command = term_old
>>>> +        else:
>>>> +            bisect_command = term_new
>>>> +
>>>> +        print("{:<20} {}".format("Instructions:", format(instructions, ",")))
>>>> +        print("{:<20} {}".format("Status:", "{} commit".format(bisect_command)))
>>>> +
>>>> +        bisect_output = git_bisect(qemu_path, bisect_command)
>>>> +
>>>> +        # Continue if still bisecting,
>>>> +        # else, print result and break.
>>>> +        if not bisect_output.split(" ")[0] == "Bisecting:":
>>>> +            print("\n*****************BISECT RESULT*****************")
>>>> +            commit_message_start = bisect_output.find("commit\n") + 7
>>>> +            commit_message_end = bisect_output.find(":040000") - 1
>>>> +            print(bisect_output[commit_message_start:commit_message_end])
>>>> +            break
>>>> +
>>>> +        bisect_count += 1
>>>> +
>>>> +    # Reset git bisect
>>>> +    git_bisect(qemu_path, "reset")
>>>> +
>>>> +    # Delete temp build directory
>>>> +    shutil.rmtree(qemu_build_path)
>>>> +
>>>> +
>>>> +if __name__ == "__main__":
>>>> +    main()
>>>> --
>>>> 2.17.1
>>>>

Best regards,
Ahmed Karaman

Re: [PATCH 1/1] scripts/performance: Add bisect.py script

[John Snow]

On 7/25/20 8:31 AM, Aleksandar Markovic wrote:
> 
> 
> On Wednesday, July 22, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com 
> <mailto:ahmedkhaledkaraman@gmail.com>> wrote:
> 
>     Python script that locates the commit that caused a performance
>     degradation or improvement in QEMU using the git bisect command
>     (binary search).
> 
>     Syntax:
>     bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
>     --target TARGET --tool {perf,callgrind} -- \
>     <target executable> [<target executable options>]
> 
>     [-h] - Print the script arguments help message
>     -s,--start START - First commit hash in the search range
>     [-e,--end END] - Last commit hash in the search range
>                      (default: Latest commit)
>     [-q,--qemu QEMU] - QEMU path.
>                      (default: Path to a GitHub QEMU clone)
>     --target TARGET - QEMU target name
>     --tool {perf,callgrind} - Underlying tool used for measurements
> 
>     Example of usage:
>     bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc \
>     --tool=perf -- coulomb_double-ppc -n 1000
> 
>     Example output:
>     Start Commit Instructions:     12,710,790,060
>     End Commit Instructions:       13,031,083,512
>     Performance Change:            -2.458%
> 
>     Estimated Number of Steps:     10
> 
>     *****************BISECT STEP 1*****************
>     Instructions:        13,031,097,790
>     Status:              slow commit
>     *****************BISECT STEP 2*****************
>     Instructions:        12,710,805,265
>     Status:              fast commit
>     *****************BISECT STEP 3*****************
>     Instructions:        13,031,028,053
>     Status:              slow commit
>     *****************BISECT STEP 4*****************
>     Instructions:        12,711,763,211
>     Status:              fast commit
>     *****************BISECT STEP 5*****************
>     Instructions:        13,031,027,292
>     Status:              slow commit
>     *****************BISECT STEP 6*****************
>     Instructions:        12,711,748,738
>     Status:              fast commit
>     *****************BISECT STEP 7*****************
>     Instructions:        12,711,748,788
>     Status:              fast commit
>     *****************BISECT STEP 8*****************
>     Instructions:        13,031,100,493
>     Status:              slow commit
>     *****************BISECT STEP 9*****************
>     Instructions:        12,714,472,954
>     Status:              fast commit
>     ****************BISECT STEP 10*****************
>     Instructions:        12,715,409,153
>     Status:              fast commit
>     ****************BISECT STEP 11*****************
>     Instructions:        12,715,394,739
>     Status:              fast commit
> 
>     *****************BISECT RESULT*****************
>     commit 0673ecdf6cb2b1445a85283db8cbacb251c46516
>     Author: Richard Henderson <richard.henderson@linaro.org
>     <mailto:richard.henderson@linaro.org>>
>     Date:   Tue May 5 10:40:23 2020 -0700
> 
>          softfloat: Inline float64 compare specializations
> 
>          Replace the float64 compare specializations with inline functions
>          that call the standard float64_compare{,_quiet} functions.
>          Use bool as the return type.
>     ***********************************************
> 
>     Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com
>     <mailto:ahmedkhaledkaraman@gmail.com>>
>     ---
>       scripts/performance/bisect.py | 374 ++++++++++++++++++++++++++++++++++
>       1 file changed, 374 insertions(+)
>       create mode 100755 scripts/performance/bisect.py
> 
>     diff --git a/scripts/performance/bisect.py
>     b/scripts/performance/bisect.py
>     new file mode 100755
>     index 0000000000..869cc69ef4
>     --- /dev/null
>     +++ b/scripts/performance/bisect.py
>     @@ -0,0 +1,374 @@
>     +#!/usr/bin/env python3
>     +
>     +#  Locate the commit that caused a performance degradation or
>     improvement in
>     +#  QEMU using the git bisect command (binary search).
>     +#
>     +#  Syntax:
>     +#  bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
>     +#  --target TARGET --tool {perf,callgrind} -- \
>     +#  <target executable> [<target executable options>]
>     +#
>     +#  [-h] - Print the script arguments help message
>     +#  -s,--start START - First commit hash in the search range
>     +#  [-e,--end END] - Last commit hash in the search range
>     +#             (default: Latest commit)
>     +#  [-q,--qemu QEMU] - QEMU path.
>     +#              (default: Path to a GitHub QEMU clone)
>     +#  --target TARGET - QEMU target name
>     +#  --tool {perf,callgrind} - Underlying tool used for measurements
>     +
>     +#  Example of usage:
>     +#  bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc
>     --tool=perf \
>     +#  -- coulomb_double-ppc -n 1000
>     +#
>     +#  This file is a part of the project "TCG Continuous Benchmarking".
>     +#
>     +#  Copyright (C) 2020  Ahmed Karaman <ahmedkhaledkaraman@gmail.com
>     <mailto:ahmedkhaledkaraman@gmail.com>>
>     +#  Copyright (C) 2020  Aleksandar Markovic
>     <aleksandar.qemu.devel@gmail.com
>     <mailto:aleksandar.qemu.devel@gmail.com>>
>     +#
> 
> 
> Hi, Ahmed.
> 
> Yes, somewhat related to John's hints on these comments, it is customary 
> to have just a brief description before "Copyright" lines. This means 
> one sentence, or a short paragraph (3-4 sentences max). The lenghty 
> syntax commemt should be, in my opinion, moved after the license 
> preamble, just before the start of real Python code.
> 

I think it's fine in the module-level docstring. Those are sometimes 
pretty long and generally refer you to other functions/classes/etc of 
interest.

In this case, this is intended to be an executable script / entrypoint, 
so having the syntax up top isn't really such a cumbersome thing.

That said, it might be prone to rot up here. Moving it to a docstring 
for the main() function, near where the parser is actually constructed, 
might help preserve accuracy for longer, at the expense of burying it 
deeper into the file.

It's an extremely minor point, and I'm afraid of getting lost too deeply 
in bikeshedding for a GSoC submission. I will be happy to see 
pylint/flake8 pass. (In pylint's case: some minimum exceptions to turn 
off warnings for too many lines / too many variables is good.)

--js

Re: [PATCH 1/1] scripts/performance: Add bisect.py script

[Aleksandar Markovic]

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

On Monday, July 27, 2020, John Snow <jsnow@redhat.com> wrote:

> On 7/25/20 8:31 AM, Aleksandar Markovic wrote:
>
>>
>>
>> On Wednesday, July 22, 2020, Ahmed Karaman <ahmedkhaledkaraman@gmail.com
>> <mailto:ahmedkhaledkaraman@gmail.com>> wrote:
>>
>>     Python script that locates the commit that caused a performance
>>     degradation or improvement in QEMU using the git bisect command
>>     (binary search).
>>
>>     Syntax:
>>     bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
>>     --target TARGET --tool {perf,callgrind} -- \
>>     <target executable> [<target executable options>]
>>
>>     [-h] - Print the script arguments help message
>>     -s,--start START - First commit hash in the search range
>>     [-e,--end END] - Last commit hash in the search range
>>                      (default: Latest commit)
>>     [-q,--qemu QEMU] - QEMU path.
>>                      (default: Path to a GitHub QEMU clone)
>>     --target TARGET - QEMU target name
>>     --tool {perf,callgrind} - Underlying tool used for measurements
>>
>>     Example of usage:
>>     bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc \
>>     --tool=perf -- coulomb_double-ppc -n 1000
>>
>>     Example output:
>>     Start Commit Instructions:     12,710,790,060
>>     End Commit Instructions:       13,031,083,512
>>     Performance Change:            -2.458%
>>
>>     Estimated Number of Steps:     10
>>
>>     *****************BISECT STEP 1*****************
>>     Instructions:        13,031,097,790
>>     Status:              slow commit
>>     *****************BISECT STEP 2*****************
>>     Instructions:        12,710,805,265
>>     Status:              fast commit
>>     *****************BISECT STEP 3*****************
>>     Instructions:        13,031,028,053
>>     Status:              slow commit
>>     *****************BISECT STEP 4*****************
>>     Instructions:        12,711,763,211
>>     Status:              fast commit
>>     *****************BISECT STEP 5*****************
>>     Instructions:        13,031,027,292
>>     Status:              slow commit
>>     *****************BISECT STEP 6*****************
>>     Instructions:        12,711,748,738
>>     Status:              fast commit
>>     *****************BISECT STEP 7*****************
>>     Instructions:        12,711,748,788
>>     Status:              fast commit
>>     *****************BISECT STEP 8*****************
>>     Instructions:        13,031,100,493
>>     Status:              slow commit
>>     *****************BISECT STEP 9*****************
>>     Instructions:        12,714,472,954
>>     Status:              fast commit
>>     ****************BISECT STEP 10*****************
>>     Instructions:        12,715,409,153
>>     Status:              fast commit
>>     ****************BISECT STEP 11*****************
>>     Instructions:        12,715,394,739
>>     Status:              fast commit
>>
>>     *****************BISECT RESULT*****************
>>     commit 0673ecdf6cb2b1445a85283db8cbacb251c46516
>>     Author: Richard Henderson <richard.henderson@linaro.org
>>     <mailto:richard.henderson@linaro.org>>
>>     Date:   Tue May 5 10:40:23 2020 -0700
>>
>>          softfloat: Inline float64 compare specializations
>>
>>          Replace the float64 compare specializations with inline functions
>>          that call the standard float64_compare{,_quiet} functions.
>>          Use bool as the return type.
>>     ***********************************************
>>
>>     Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com
>>     <mailto:ahmedkhaledkaraman@gmail.com>>
>>     ---
>>       scripts/performance/bisect.py | 374 ++++++++++++++++++++++++++++++
>> ++++
>>       1 file changed, 374 insertions(+)
>>       create mode 100755 scripts/performance/bisect.py
>>
>>     diff --git a/scripts/performance/bisect.py
>>     b/scripts/performance/bisect.py
>>     new file mode 100755
>>     index 0000000000..869cc69ef4
>>     --- /dev/null
>>     +++ b/scripts/performance/bisect.py
>>     @@ -0,0 +1,374 @@
>>     +#!/usr/bin/env python3
>>     +
>>     +#  Locate the commit that caused a performance degradation or
>>     improvement in
>>     +#  QEMU using the git bisect command (binary search).
>>     +#
>>     +#  Syntax:
>>     +#  bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
>>     +#  --target TARGET --tool {perf,callgrind} -- \
>>     +#  <target executable> [<target executable options>]
>>     +#
>>     +#  [-h] - Print the script arguments help message
>>     +#  -s,--start START - First commit hash in the search range
>>     +#  [-e,--end END] - Last commit hash in the search range
>>     +#             (default: Latest commit)
>>     +#  [-q,--qemu QEMU] - QEMU path.
>>     +#              (default: Path to a GitHub QEMU clone)
>>     +#  --target TARGET - QEMU target name
>>     +#  --tool {perf,callgrind} - Underlying tool used for measurements
>>     +
>>     +#  Example of usage:
>>     +#  bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc
>>     --tool=perf \
>>     +#  -- coulomb_double-ppc -n 1000
>>     +#
>>     +#  This file is a part of the project "TCG Continuous Benchmarking".
>>     +#
>>     +#  Copyright (C) 2020  Ahmed Karaman <ahmedkhaledkaraman@gmail.com
>>     <mailto:ahmedkhaledkaraman@gmail.com>>
>>     +#  Copyright (C) 2020  Aleksandar Markovic
>>     <aleksandar.qemu.devel@gmail.com
>>     <mailto:aleksandar.qemu.devel@gmail.com>>
>>     +#
>>
>>
>> Hi, Ahmed.
>>
>> Yes, somewhat related to John's hints on these comments, it is customary
>> to have just a brief description before "Copyright" lines. This means one
>> sentence, or a short paragraph (3-4 sentences max). The lenghty syntax
>> commemt should be, in my opinion, moved after the license preamble, just
>> before the start of real Python code.
>>
>>
> I think it's fine in the module-level docstring. Those are sometimes
> pretty long and generally refer you to other functions/classes/etc of
> interest.
>
> In this case, this is intended to be an executable script / entrypoint, so
> having the syntax up top isn't really such a cumbersome thing.
>
> That said, it might be prone to rot up here. Moving it to a docstring for
> the main() function, near where the parser is actually constructed, might
> help preserve accuracy for longer, at the expense of burying it deeper into
> the file.
>
>
Yes,, I too actually think that is a better position.

It's an extremely minor point, and I'm afraid of getting lost too deeply in
> bikeshedding for a GSoC submission. I will be happy to see pylint/flake8
> pass. (In pylint's case: some minimum exceptions to turn off warnings for
> too many lines / too many variables is good.)
>
>
Close to or even after the end of the project, Ahmed could clean up all
previously submitted scripts as well. It is natural that he perfects his
style over time.

Aleksandar


--js
>
>

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