[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/1] scripts/performance: Add bisect.py script
|
From: |
Ahmed Karaman |
|
Subject: |
Re: [PATCH 1/1] scripts/performance: Add bisect.py script |
|
Date: |
Sat, 25 Jul 2020 22:48:30 +0200 |
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