PR benchmark stats

[jlabedo]

This PR will run benchmark on PR and compare to main branch.

It will comment on each PR with this template:

PR stats

ecotone benchmarks

subject	mem_peak	mode	rstdev
bench_running_ecotone_lite	5.645mb 0.00%	144.330ms +1.85%	±0.53% -56.69%
bench_running_ecotone_lite_with_fail_fast	5.662mb 0.00%	150.465ms -0.70%	±0.50% -26.91%
bench_running_ecotone_lite_with_cache	12.569mb 0.00%	14.925ms +6.53%	±3.42% +312.03%

symfony benchmarks

subject	mem_peak	mode	rstdev
bench_kernel_boot_on_prod	1.778mb 0.00%	186.272μs -1.16%	±1.36% -87.27%
bench_kernel_boot_on_dev	1.778mb 0.00%	185.777μs -0.37%	±0.80% -89.32%
bench_messaging_boot_on_prod	15.360mb 0.00%	8.457ms +1.27%	±6.49% +43.55%
bench_messaging_boot_on_dev	14.809mb 0.00%	22.393ms +0.90%	±3.41% -7.16%

laravel benchmarks

subject	mem_peak	mode	rstdev
bench_kernel_boot_on_prod	7.917mb 0.00%	139.412ms +0.24%	±0.40% -51.92%
bench_kernel_boot_on_dev	7.917mb 0.00%	141.491ms +1.54%	±4.63% +448.84%
bench_messaging_boot_on_prod	9.188mb 0.00%	144.847ms -0.72%	±1.02% -67.74%
bench_messaging_boot_on_dev	9.188mb 0.00%	145.450ms -1.17%	±0.40% -65.63%

[jlabedo]

@dgafka , I think you need to enable PR write permissions on pull requests if you agree: https://github.com/marketplace/actions/sticky-pull-request-comment#error-resource-not-accessible-by-integration

[dgafka]

Great PR, If we will polish this a bit more, this will make a huge difference into the feedback for PR creator and the reviewer :)

[dgafka]

Can you add a comment what is this for?

[dgafka]

I think what you've mentioned the other day, that jobs may be actually executed on different runners, which makes comparing those not accurate is crucial.
What we could do instead in here is to first run the benchmarks on PR branch and then checkout to the main branch.

I think be possible if we will use actions/checkout@v2 and fetch-depth: '0' so it fetch all available branches.
This way within single job, we could run two benchmarks and compare the results, avoiding the pitfall of different underlying hardware (the caching part would not not be needed in that case).

Wdyt?

[jlabedo]

I read that we may have 10-20% performance difference on ubuntu linux. But yes, I expect the benchmark in the same runner to have less difference. But the job will be 2 times longer.

[dgafka]

I read that we may have 10-20% performance difference on ubuntu linux.

That's actually significant amount.

But the job will be 2 times longer.

That's not a problem. Other tests will actually take more time and it will run in parallel :)
Can we go for that in that case?

[dgafka]

I wonder, if we run tests directly from the root on those catalogs? So we don't need to install dependencies for each of them.

[jlabedo]

Indeed I think it may be possible and better. And wdyt of having dedicated Lite/Symfony/Laravel apps in the monorepo root for benchmarks ?

[dgafka]

If we put them under quickstart, in some benchmark subcatalog then all fine by me :)
Then we could have same implementation for those three and see how they are comparing.

I think we can use those benchmarks for testing out projection rebuilds too. There is a lot of tricks for optimalization those, so that would show us how much we improved.

[jlabedo]

Hello @dgafka

• now all benchmarks are at the monorepo root which makes the comparison between packages easier, and will allow to have dedicated apps to benchmark in the quickstart folder
• I removed the php script in favor of a bash script to transform the table to a markdown table. As it is only related to ci, the logic of creating the comment is written in the benchmark-pr.yaml github action file, which makes sense to me
• the benchmark on main will run on each PR, but it has its drawbacks:
  • composer dependencies are installed on PR code, then main is checked out, and benchmark is runned. So main benchmark will run on the dependencies of the PR.
  • same for phpbench.json, the github action workflow file is the one of the PR, but we will have the phpbench.json configuration of main branch. That is why the step on main is failing for now, as the phpbench.json does not exist on main

[dgafka]

Dependencies between PR and Main may be incompatibile.
Can we run composer update once on the Main and once on the PR's branch?

[jlabedo]

here you go

[dgafka]

@dgafka , I think you need to enable PR write permissions on pull requests if you agree: https://github.com/marketplace/actions/sticky-pull-request-comment#error-resource-not-accessible-by-integration

I've not found an safe way to set it up.
As enabling write permissions on forks, can make people commit code to the repo, making it unsafe to use.

Can we drop it for the time being and just echo the result inside the job?

[jlabedo]

Now they are hidden there: https://github.com/ecotoneframework/ecotone-dev/actions/runs/5181629323?pr=138

I also added --retry-threshold=5 option to benchmark so it will drop iterations that are far from other iterations and retry

[dgafka]

This is amazing PR @jlabedo, thank you very much for putting your energy into that :)