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

[Aleksandar Markovic]

On Monday, July 27, 2020, John Snow  wrote:

> On 7/25/20 8:31 AM, Aleksandar Markovic wrote:
>
>>
>>
>> On Wednesday, July 22, 2020, 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} -- \
>>  []
>>
>> [-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 > >
>> 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 > >
>> ---
>>   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 00..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} -- \
>> +#   []
>> +#
>> +#  [-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 

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 > 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} -- \
 []

[-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 mailto:richard.hender...@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 mailto:ahmedkhaledkara...@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 00..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} -- \
+#   []
+#
+#  [-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 mailto:ahmedkhaledkara...@gmail.com>>
+#  Copyright (C) 2020  Aleksandar Markovic
mailto:aleksandar.qemu.de...@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 

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

[Ahmed Karaman]

On Sat, Jul 25, 2020 at 9:48 PM Aleksandar Markovic
 wrote:
>
>
>
> On Saturday, July 25, 2020, Ahmed Karaman  
> wrote:
>>
>> On Sat, Jul 25, 2020 at 2:31 PM Aleksandar Markovic 
>>  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 .
 +
 +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",
 +  

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

[Aleksandar Markovic]

On Saturday, July 25, 2020, Ahmed Karaman 
wrote:

> On Sat, Jul 25, 2020 at 2:31 PM Aleksandar Markovic <
> aleksandar.qemu.de...@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 >> >.
>>> +
>>> +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],
>>> +

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

[Ahmed Karaman]

On Sat, Jul 25, 2020 at 2:31 PM Aleksandar Markovic <
aleksandar.qemu.de...@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 .
>> +
>> +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 

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

[Aleksandar Markovic]

On Wednesday, July 22, 2020, 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} -- \
>  []
>
> [-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 
> 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 
> ---
>  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 00..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} -- \
> +#   []
> +#
> +#  [-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 
> +#  Copyright (C) 2020  Aleksandar Markovic  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 

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} -- \
 []

[-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 
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 


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 00..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} -- \
+#   []
+#
+#  [-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 
+#  Copyright (C) 2020  Aleksandar Markovic 
+#
+#  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