From 7579b9c7beab335f516f9b1e464fe16dbf2ec898 Mon Sep 17 00:00:00 2001 From: "duanyi.aster" Date: Sun, 11 Aug 2024 16:52:48 +0800 Subject: [PATCH 1/7] feat: implement bench diff test on bench.py --- scripts/bench.py | 72 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 68 insertions(+), 4 deletions(-) diff --git a/scripts/bench.py b/scripts/bench.py index 80e8f186b..7213ebec7 100755 --- a/scripts/bench.py +++ b/scripts/bench.py @@ -14,6 +14,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import csv +import io import tempfile import os import subprocess @@ -81,10 +83,6 @@ def compare(args): # benchmark main branch main = run_bench(args, "main") - - # diff the result - # benchstat = "go get golang.org/x/perf/cmd/benchstat && go install golang.org/x/perf/cmd/benchstat" - run( "benchstat %s %s"%(main, target)) run("git checkout -- .") # restore branch @@ -92,6 +90,9 @@ def compare(args): run("git checkout %s"%(current_branch)) run("patch -p1 < %s" % (diff)) + + # diff the result + bench_diff(main, target, args.threshold) return target def run_bench(args, name): @@ -99,10 +100,73 @@ def run_bench(args, name): run("%s 2>&1 | tee %s" %(args.cmd, fname)) return fname +def bench_diff(main, target, threshold=0.05): + run("go get golang.org/x/perf/cmd/benchstat && go install golang.org/x/perf/cmd/benchstat") + csv_content = run_s( "benchstat -format=csv %s %s"%(main, target)) + print("benchstat: %s"%csv_content) + + # filter out invalid lines + valid_headers = {',sec/op,CI,sec/op,CI,vs base,P', ',B/s,CI,B/s,CI,vs base,P'} + valid_blocks = [] + current_block = [] + parsing = False + + # Filter out valid CSV blocks + for line in csv_content.splitlines(): + if line in valid_headers: + if current_block: + valid_blocks.append('\n'.join(current_block)) + current_block = [line] + parsing = True + elif parsing: + if line.strip() == '': + parsing = False + if current_block: + valid_blocks.append('\n'.join(current_block)) + current_block = [] + else: + current_block.append(line) + + if current_block: + valid_blocks.append('\n'.join(current_block)) + + # Parse each valid CSV block + significant_decline = [] + for block in valid_blocks: + csv_reader = csv.DictReader(io.StringIO(block)) + for row in csv_reader: + benchmark = row[''] + vs_base = row['vs base'] + if benchmark == 'geomean': + continue + + # Skip rows without a valid "vs base" value + if not vs_base or vs_base == '~': + continue + + # Convert "vs base" to a float percentage + try: + vs_base_percentage = float(vs_base.strip('%')) / 100 + except ValueError: + continue + + # Check if the decline is significant + if vs_base_percentage < 0 and -vs_base_percentage > threshold: + significant_decline.append({ + 'benchmark': benchmark, + 'vs_base': vs_base_percentage + }) + + if significant_decline: + print("found significant decline! %s %f"%(significant_decline[0]['benchmark'], significant_decline[0]['vs_base'])) + exit(2) + + return def main(): argparser = argparse.ArgumentParser(description='Tools to test the performance. Example: ./bench.py "go test -bench=. ./..."') argparser.add_argument('cmd', type=str, help='Golang benchmark command') + argparser.add_argument('-t', type=float, dest='threshold', default=0.1, help='diff bench threshold') argparser.add_argument('-c', '--compare', dest='compare', action='store_true', help='Compare the current branch with the main branch') args = argparser.parse_args() From 5b5c7ee547bef69c5c2f737cc51515453dc82352 Mon Sep 17 00:00:00 2001 From: "duanyi.aster" Date: Sun, 11 Aug 2024 17:40:06 +0800 Subject: [PATCH 2/7] add bench check on CI --- .github/workflows/benchmark-linux.yml | 8 +++++++- generic_test/benchmark_test.go | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/benchmark-linux.yml b/.github/workflows/benchmark-linux.yml index 1e1f4488a..929c4d305 100644 --- a/.github/workflows/benchmark-linux.yml +++ b/.github/workflows/benchmark-linux.yml @@ -27,4 +27,10 @@ jobs: ${{ runner.os }}-go- - name: Benchmark sonic - run: sh scripts/bench.sh \ No newline at end of file + run: | + export SONIC_NO_ASYNC_GC=1 + export SONIC_BENCH_SINGLE=1 + ./scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Encoder|Decoder)_(Generic|Binding)_Sonic' ./..." + ./scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Get|Set)One_Sonic|BenchmarkParseSeven_Sonic' ./ast" + cd ./generic_test + ../scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark' ." diff --git a/generic_test/benchmark_test.go b/generic_test/benchmark_test.go index 377aef025..c3a2b4641 100644 --- a/generic_test/benchmark_test.go +++ b/generic_test/benchmark_test.go @@ -33,6 +33,7 @@ import ( var ( validFlag = os.Getenv("SONIC_VALID_GENERIC_BENCH") != "" pretouchFlag = os.Getenv("SONIC_NO_PRETOUCH_BENCH") == "" + benchSingle = os.Getenv("SONIC_BENCH_SINGLE") != "" ) type jsonLibEntry struct { @@ -51,6 +52,15 @@ var jsonLibs = []jsonLibEntry { {"JsonIterStd", jsoniter.ConfigCompatibleWithStandardLibrary.Marshal, jsoniter.ConfigCompatibleWithStandardLibrary.Unmarshal}, } +func init() { + if benchSingle { + jsonLibs = []jsonLibEntry { + {"Sonic", sonic.Marshal, sonic.Unmarshal}, + {"SonicStd", sonic.ConfigStd.Marshal, sonic.ConfigStd.Unmarshal}, + } + } +} + func BenchmarkUnmarshalConcrete(b *testing.B) { runUnmarshalC(b) } From e18df73263efa997b2589aa26358b6b9f806fe1b Mon Sep 17 00:00:00 2001 From: "duanyi.aster" Date: Sun, 11 Aug 2024 17:57:04 +0800 Subject: [PATCH 3/7] fix --- .github/workflows/benchmark-linux.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/benchmark-linux.yml b/.github/workflows/benchmark-linux.yml index 929c4d305..288b8000b 100644 --- a/.github/workflows/benchmark-linux.yml +++ b/.github/workflows/benchmark-linux.yml @@ -30,7 +30,6 @@ jobs: run: | export SONIC_NO_ASYNC_GC=1 export SONIC_BENCH_SINGLE=1 - ./scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Encoder|Decoder)_(Generic|Binding)_Sonic' ./..." + ./scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Encoder|Decoder)_(Generic|Binding)_Sonic' ./decoder" ./scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Get|Set)One_Sonic|BenchmarkParseSeven_Sonic' ./ast" - cd ./generic_test - ../scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark' ." + ./scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark' ./generic_test" From dbcc946aec28e47b7eee7b9dd6bca026e04f1441 Mon Sep 17 00:00:00 2001 From: "duanyi.aster" Date: Mon, 12 Aug 2024 12:02:25 +0800 Subject: [PATCH 4/7] refactor --- .github/workflows/benchmark-linux.yml | 24 ++++++++++++++++++++---- scripts/bench.py | 4 ++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/.github/workflows/benchmark-linux.yml b/.github/workflows/benchmark-linux.yml index 288b8000b..8e6912084 100644 --- a/.github/workflows/benchmark-linux.yml +++ b/.github/workflows/benchmark-linux.yml @@ -26,10 +26,26 @@ jobs: restore-keys: | ${{ runner.os }}-go- - - name: Benchmark sonic + - name: Benchmark Target run: | export SONIC_NO_ASYNC_GC=1 export SONIC_BENCH_SINGLE=1 - ./scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Encoder|Decoder)_(Generic|Binding)_Sonic' ./decoder" - ./scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Get|Set)One_Sonic|BenchmarkParseSeven_Sonic' ./ast" - ./scripts/bench.py -c -t 0.05 "go test -run ^$ -count=10 -benchmem -bench 'Benchmark' ./generic_test" + go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Encoder|Decoder)_(Generic|Binding)_Sonic' ./decoder >> /var/tmp/sonic_bench_target.out + go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Get|Set)One_Sonic|BenchmarkParseSeven_Sonic' ./ast >> /var/tmp/sonic_bench_target.out + go test -run ^$ -count=5 -benchmem -bench 'Benchmark' ./generic_test >> /var/tmp/sonic_bench_target.out + + - name: Checkout main + uses: actions/checkout@v2 + with: + ref: main + + - name: Benchmark main + run: | + export SONIC_NO_ASYNC_GC=1 + export SONIC_BENCH_SINGLE=1 + go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Encoder|Decoder)_(Generic|Binding)_Sonic' ./decoder >> /var/tmp/sonic_bench_main.out + go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Get|Set)One_Sonic|BenchmarkParseSeven_Sonic' ./ast >> /var/tmp/sonic_bench_main.out + go test -run ^$ -count=5 -benchmem -bench 'Benchmark' ./generic_test >> /var/tmp/sonic_bench_main.out + + - name: Diff bench + run: ./scripts/bench.py -t 0.05 -d /var/tmp/sonic_bench_target.out,/var/tmp/sonic_bench_main.out x diff --git a/scripts/bench.py b/scripts/bench.py index 7213ebec7..41e1b5a08 100755 --- a/scripts/bench.py +++ b/scripts/bench.py @@ -166,6 +166,7 @@ def bench_diff(main, target, threshold=0.05): def main(): argparser = argparse.ArgumentParser(description='Tools to test the performance. Example: ./bench.py "go test -bench=. ./..."') argparser.add_argument('cmd', type=str, help='Golang benchmark command') + argparser.add_argument('-d', type=str, dest='diff', help='diff bench') argparser.add_argument('-t', type=float, dest='threshold', default=0.1, help='diff bench threshold') argparser.add_argument('-c', '--compare', dest='compare', action='store_true', help='Compare the current branch with the main branch') @@ -173,6 +174,9 @@ def main(): if args.compare: compare(args) + elif args.diff: + target, base = args.diff.split(',') + bench_diff(target, base, args.threshold) else: run_bench(args, "target") From da17956138412fb1ad7e0ad0accd6961d4fd25c2 Mon Sep 17 00:00:00 2001 From: "duanyi.aster" Date: Mon, 12 Aug 2024 12:20:23 +0800 Subject: [PATCH 5/7] TODO: not bench generic_test because it has bugs --- .github/workflows/benchmark-linux.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/benchmark-linux.yml b/.github/workflows/benchmark-linux.yml index 8e6912084..abbea54b8 100644 --- a/.github/workflows/benchmark-linux.yml +++ b/.github/workflows/benchmark-linux.yml @@ -32,7 +32,6 @@ jobs: export SONIC_BENCH_SINGLE=1 go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Encoder|Decoder)_(Generic|Binding)_Sonic' ./decoder >> /var/tmp/sonic_bench_target.out go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Get|Set)One_Sonic|BenchmarkParseSeven_Sonic' ./ast >> /var/tmp/sonic_bench_target.out - go test -run ^$ -count=5 -benchmem -bench 'Benchmark' ./generic_test >> /var/tmp/sonic_bench_target.out - name: Checkout main uses: actions/checkout@v2 @@ -45,7 +44,6 @@ jobs: export SONIC_BENCH_SINGLE=1 go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Encoder|Decoder)_(Generic|Binding)_Sonic' ./decoder >> /var/tmp/sonic_bench_main.out go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Get|Set)One_Sonic|BenchmarkParseSeven_Sonic' ./ast >> /var/tmp/sonic_bench_main.out - go test -run ^$ -count=5 -benchmem -bench 'Benchmark' ./generic_test >> /var/tmp/sonic_bench_main.out - name: Diff bench run: ./scripts/bench.py -t 0.05 -d /var/tmp/sonic_bench_target.out,/var/tmp/sonic_bench_main.out x From bebf8b4058db36611c7f918f8e78161980f972d7 Mon Sep 17 00:00:00 2001 From: "duanyi.aster" Date: Mon, 12 Aug 2024 12:58:10 +0800 Subject: [PATCH 6/7] update --- .github/workflows/{benchmark-linux.yml => benchmark.yml} | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) rename .github/workflows/{benchmark-linux.yml => benchmark.yml} (93%) diff --git a/.github/workflows/benchmark-linux.yml b/.github/workflows/benchmark.yml similarity index 93% rename from .github/workflows/benchmark-linux.yml rename to .github/workflows/benchmark.yml index abbea54b8..25b73869f 100644 --- a/.github/workflows/benchmark-linux.yml +++ b/.github/workflows/benchmark.yml @@ -1,4 +1,4 @@ -name: Benchmark Linux-X64 +name: Benchmark on: pull_request @@ -33,6 +33,9 @@ jobs: go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Encoder|Decoder)_(Generic|Binding)_Sonic' ./decoder >> /var/tmp/sonic_bench_target.out go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Get|Set)One_Sonic|BenchmarkParseSeven_Sonic' ./ast >> /var/tmp/sonic_bench_target.out + - name: Clear repository + run: sudo rm -fr $GITHUB_WORKSPACE && mkdir $GITHUB_WORKSPACE + - name: Checkout main uses: actions/checkout@v2 with: From 05c154c26535646a8fa341d82b2e36617064370a Mon Sep 17 00:00:00 2001 From: "duanyi.aster" Date: Mon, 12 Aug 2024 14:18:53 +0800 Subject: [PATCH 7/7] tmp: not enable bench.py first --- .github/workflows/benchmark.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 25b73869f..2ee1030d2 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -49,4 +49,7 @@ jobs: go test -run ^$ -count=10 -benchmem -bench 'Benchmark(Get|Set)One_Sonic|BenchmarkParseSeven_Sonic' ./ast >> /var/tmp/sonic_bench_main.out - name: Diff bench - run: ./scripts/bench.py -t 0.05 -d /var/tmp/sonic_bench_target.out,/var/tmp/sonic_bench_main.out x + run: | + go get golang.org/x/perf/cmd/benchstat && go install golang.org/x/perf/cmd/benchstat + benchstat -format=csv /var/tmp/sonic_bench_target.out /var/tmp/sonic_bench_main.out + # run: ./scripts/bench.py -t 0.05 -d /var/tmp/sonic_bench_target.out,/var/tmp/sonic_bench_main.out x