Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add benchmark for the cloud output #1224

Merged
merged 2 commits into from
Nov 4, 2019
Merged

add benchmark for the cloud output #1224

merged 2 commits into from
Nov 4, 2019

Conversation

mstoykov
Copy link
Contributor

@mstoykov mstoykov commented Nov 1, 2019

The benchmark is testing only for httpext.Trail and such that have
different urls and names which is probably the worst case performance
wise. The benchmark is somewhat flaky ... but with more runs it seems
consitant in 5% margins ;)

The benchmark is testing only for httpext.Trail and such that have
different urls and names which is probably the worst case performance
wise. The benchmark is somewhat flaky ... but with more runs it seems
consitant in 5% margins ;)
@mstoykov
Copy link
Contributor Author

mstoykov commented Nov 1, 2019

$ benchcmp -mag 9e1c5c1.perf 78858ef.perf

benchmark            old ns/op     new ns/op     delta
BenchmarkCloud-4     19685045      23013543      +16.91%
BenchmarkCloud-4     21060987      19451558      -7.64%
BenchmarkCloud-4     22526058      20867068      -7.36%
BenchmarkCloud-4     20830405      21631604      +3.85%
BenchmarkCloud-4     22701274      22245438      -2.01%

benchmark            old allocs     new allocs     delta
BenchmarkCloud-4     93190          93872          +0.73%
BenchmarkCloud-4     93833          93342          -0.52%
BenchmarkCloud-4     93111          93381          +0.29%
BenchmarkCloud-4     93091          93071          -0.02%
BenchmarkCloud-4     93249          93267          +0.02%

benchmark            old bytes     new bytes     delta
BenchmarkCloud-4     13756303      13806000      +0.36%
BenchmarkCloud-4     13822686      13786573      -0.26%
BenchmarkCloud-4     13790461      13817672      +0.20%
BenchmarkCloud-4     13792657      13819228      +0.19%
BenchmarkCloud-4     13793277      13769954      -0.17%

$ benchcmp -mag 78858ef.perf a948b4d.perf

benchmark            old ns/op     new ns/op     delta
BenchmarkCloud-4     23013543      31868265      +38.48%
BenchmarkCloud-4     19451558      26698255      +37.26%
BenchmarkCloud-4     20867068      27512414      +31.85%
BenchmarkCloud-4     21631604      26224220      +21.23%
BenchmarkCloud-4     22245438      25525690      +14.75%

benchmark            old allocs     new allocs     delta
BenchmarkCloud-4     93267          133888         +43.55%
BenchmarkCloud-4     93071          133210         +43.13%
BenchmarkCloud-4     93381          133397         +42.85%
BenchmarkCloud-4     93342          133194         +42.69%
BenchmarkCloud-4     93872          133472         +42.19%

benchmark            old bytes     new bytes     delta
BenchmarkCloud-4     13817672      19473203      +40.93%
BenchmarkCloud-4     13806000      19442325      +40.83%
BenchmarkCloud-4     13769954      19371700      +40.68%
BenchmarkCloud-4     13786573      19379163      +40.57%
BenchmarkCloud-4     13819228      19399568      +40.38%

$ benchcmp -mag a948b4d.perf 9554c6e.perf

benchmark            old ns/op     new ns/op     delta
BenchmarkCloud-4     31868265      29050273      -8.84%
BenchmarkCloud-4     26698255      27439261      +2.78%
BenchmarkCloud-4     27512414      28170350      +2.39%
BenchmarkCloud-4     26224220      25671548      -2.11%
BenchmarkCloud-4     25525690      25834813      +1.21%

benchmark            old allocs     new allocs     delta
BenchmarkCloud-4     133210         134313         +0.83%
BenchmarkCloud-4     133194         133753         +0.42%
BenchmarkCloud-4     133888         133707         -0.14%
BenchmarkCloud-4     133397         133275         -0.09%
BenchmarkCloud-4     133472         133428         -0.03%

benchmark            old bytes     new bytes     delta
BenchmarkCloud-4     19371700      19480151      +0.56%
BenchmarkCloud-4     19379163      19419020      +0.21%
BenchmarkCloud-4     19442325      19410276      -0.16%
BenchmarkCloud-4     19399568      19416469      +0.09%
BenchmarkCloud-4     19473203      19466236      -0.04%

$ benchcmp -mag 9554c6e.perf b6650a5.perf

benchmark            old ns/op     new ns/op     delta
BenchmarkCloud-4     25834813      29113323      +12.69%
BenchmarkCloud-4     28170350      26871712      -4.61%
BenchmarkCloud-4     25671548      26679223      +3.93%
BenchmarkCloud-4     27439261      26988018      -1.64%
BenchmarkCloud-4     29050273      28921436      -0.44%

benchmark            old allocs     new allocs     delta
BenchmarkCloud-4     134313         133309         -0.75%
BenchmarkCloud-4     133275         133646         +0.28%
BenchmarkCloud-4     133753         133524         -0.17%
BenchmarkCloud-4     133707         133571         -0.10%
BenchmarkCloud-4     133428         133472         +0.03%

benchmark            old bytes     new bytes     delta
BenchmarkCloud-4     19480151      19378667      -0.52%
BenchmarkCloud-4     19466236      19407682      -0.30%
BenchmarkCloud-4     19419020      19403899      -0.08%
BenchmarkCloud-4     19410276      19413150      +0.01%
BenchmarkCloud-4     19416469      19416639      +0.00%

@codecov-io
Copy link

codecov-io commented Nov 1, 2019

Codecov Report

Merging #1224 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1224      +/-   ##
==========================================
+ Coverage   75.12%   75.14%   +0.01%     
==========================================
  Files         147      147              
  Lines       10709    10709              
==========================================
+ Hits         8045     8047       +2     
+ Misses       2199     2197       -2     
  Partials      465      465
Impacted Files Coverage Δ
stats/cloud/collector.go 73.23% <0%> (+0.61%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9554c6e...24b3ce4. Read the comment docs.

imiric
imiric previously approved these changes Nov 1, 2019
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm if I'm interpreting the results correctly, that seems like quite a regression in performance since 78858ef onwards. But it's an acceptable trade-off for the url=name workaround, right?

My only concern would be adding even more flaky tests benchmarks. I think we have enough flakiness with tests already. :) And this won't be run in CI, right?

@mstoykov
Copy link
Contributor Author

mstoykov commented Nov 1, 2019

@imiric benchmarks are atm not ran by CI although around #1167 may change.
Performance wise, this is the worst case only in it (IMO) we only have HTTPTrails and all of them are in need of copying ... In a real scenario this will likely be 0% of the cases and even where every http request is with name != url this will still not be 100% of the metrics ;) In general I expect in somewhat bad cases the performance degradation of the cloud output (not the whole k6) to be around 5-10%

@cuonglm
Copy link
Contributor

cuonglm commented Nov 1, 2019

Can you retry with benchstat instead of benchcmp.

@mstoykov
Copy link
Contributor Author

mstoykov commented Nov 1, 2019

@cuonglm
$ benchstat 9e1c5c1.perf 78858ef.perf a948b4d.perf 9554c6e.perf

name \ time/op    9e1c5c11.perf  78858ef4.perf  a948b4df.perf  9554c6e7.perf
Cloud-4             21.4ms ± 8%    21.4ms ± 9%    27.6ms ±16%    27.2ms ± 7%

name \ alloc/op   9e1c5c11.perf  78858ef4.perf  a948b4df.perf  9554c6e7.perf
Cloud-4             13.8MB ± 0%    13.8MB ± 0%    19.4MB ± 0%    19.4MB ± 0%

name \ allocs/op  9e1c5c11.perf  78858ef4.perf  a948b4df.perf  9554c6e7.perf
Cloud-4              93.3k ± 1%     93.4k ± 1%    133.4k ± 0%    133.7k ± 0%

@cuonglm
Copy link
Contributor

cuonglm commented Nov 1, 2019

@cuonglm
$ benchstat 9e1c5c1.perf 78858ef.perf a948b4d.perf 9554c6e.perf

name \ time/op    9e1c5c11.perf  78858ef4.perf  a948b4df.perf  9554c6e7.perf
Cloud-4             21.4ms ± 8%    21.4ms ± 9%    27.6ms ±16%    27.2ms ± 7%

name \ alloc/op   9e1c5c11.perf  78858ef4.perf  a948b4df.perf  9554c6e7.perf
Cloud-4             13.8MB ± 0%    13.8MB ± 0%    19.4MB ± 0%    19.4MB ± 0%

name \ allocs/op  9e1c5c11.perf  78858ef4.perf  a948b4df.perf  9554c6e7.perf
Cloud-4              93.3k ± 1%     93.4k ± 1%    133.4k ± 0%    133.7k ± 0%

Thanks, seems to be a lot of noise when benchmark runs. But the result looks reasonable.

@mstoykov mstoykov requested a review from cuonglm November 4, 2019 13:27
@mstoykov mstoykov requested a review from imiric November 4, 2019 15:32
@mstoykov mstoykov merged commit db0d952 into master Nov 4, 2019
@mstoykov mstoykov deleted the cloudBench branch November 4, 2019 15:48
@mstoykov mstoykov added this to the v0.26.0 milestone Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants