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

> On 7/25/20 8:31 AM, Aleksandar Markovic wrote:
>
>>
>>
>> On Wednesday, July 22, 2020, Ahmed Karaman <ahmedkhaledkara...@gmail.com
>> <mailto:ahmedkhaledkara...@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.hender...@linaro.org
>>     <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 <ahmedkhaledkara...@gmail.com
>>     <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 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 <ahmedkhaledkara...@gmail.com
>>     <mailto:ahmedkhaledkara...@gmail.com>>
>>     +#  Copyright (C) 2020  Aleksandar Markovic
>>     <aleksandar.qemu.de...@gmail.com
>>     <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 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
>
>

Reply via email to