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

feat: benchmark some python script #1356

Merged
merged 7 commits into from
Apr 12, 2023
Merged

feat: benchmark some python script #1356

merged 7 commits into from
Apr 12, 2023

Conversation

discord9
Copy link
Contributor

@discord9 discord9 commented Apr 11, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Use criterion to benchmark RustPython/CPython Script, found out in heavy computation load, CPython is still better, and in api heavy computation, RustPython have basically same speed with CPython

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Benchmark Results

On a 11th Gen Intel(R) Core(TM) i5-11260H @ 2.60GHz within a WSL Ubuntu 20.04.5 LTS
Every script is first compiled, then run repeatly for ten times.
Not very good, even for CPython, only api heavy computation have a slightly acceptable perfermance, future optimizations:

  • create new process for CPython(Lets' call it workers) to avoid GIL
  • use PyPy interpreter instead(A jit compiler), which have lesser third-party package support
fib 20 rspy             time:   [150.72 ms 151.78 ms 152.96 ms]
                        change: [+1.3900% +2.1250% +2.9568%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe

fib 20 pyo3             time:   [18.477 ms 18.560 ms 18.654 ms]
                        change: [+3.1509% +3.8387% +4.6357%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  4 (4.00%) high mild
  2 (2.00%) high severe

Benchmarking par fib 20 rspy: Warming up for 3.0000 s
par fib 20 rspy         time:   [776.14 ms 781.27 ms 786.50 ms]
                        change: [-3.3698% -2.1335% -0.8766%] (p = 0.00 < 0.05)
                        Change within noise threshold.

Benchmarking par fib 20 pyo3: Warming up for 3.0000 s
par fib 20 pyo3         time:   [310.77 ms 311.37 ms 312.12 ms]
                        change: [-2.7750% -1.9295% -1.1246%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

Benchmarking loop 1M rspy: Warming up for 3.0000 s
loop 1M rspy            time:   [660.67 ms 664.23 ms 668.14 ms]
                        change: [+0.8738% +2.1143% +3.2083%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

Benchmarking loop 1M pyo3: Warming up for 3.0000 s
loop 1M pyo3            time:   [146.17 ms 146.63 ms 147.11 ms]
                        change: [+0.9481% +1.7598% +2.4832%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild

Benchmarking api heavy rspy: Warming up for 3.0000 s
api heavy rspy          time:   [76.961 ms 78.046 ms 79.250 ms]
                        change: [+2.0373% +4.5200% +6.9584%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 9 outliers among 100 measurements (9.00%)
  7 (7.00%) high mild
  2 (2.00%) high severe

Benchmarking api heavy pyo3: Warming up for 3.0000 s
api heavy pyo3          time:   [73.525 ms 74.142 ms 74.788 ms]
                        change: [+3.2784% +5.3485% +7.4066%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild

@discord9 discord9 marked this pull request as ready for review April 11, 2023 02:52
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has totally checked 888 files.

Valid Invalid Ignored Fixed
683 1 204 0
Click to see the invalid file list
  • src/script/benches/py_benchmark.rs

src/script/benches/py_benchmark.rs Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 11, 2023

Codecov Report

Merging #1356 (ca2dc7c) into develop (09f003d) will increase coverage by 0.55%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1356      +/-   ##
===========================================
+ Coverage    85.49%   86.04%   +0.55%     
===========================================
  Files          514      523       +9     
  Lines        77471    78396     +925     
===========================================
+ Hits         66230    67459    +1229     
+ Misses       11241    10937     -304     

waynexia and others added 2 commits April 11, 2023 11:37
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

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

Generally looks good to me. Except for the thread spawning part. I'm not sure if criterion will create a huge amount of new threads for us. Let's keep an eye on it after merging this PR.

src/script/benches/py_benchmark.rs Outdated Show resolved Hide resolved
@discord9
Copy link
Contributor Author

Generally looks good to me. Except for the thread spawning part. I'm not sure if criterion will create a huge amount of new threads for us. Let's keep an eye on it after merging this PR.

Replace it with create a new rayon::ThreadPool instead, should work well with criterion since it's backing parallel is also rayon?

@waynexia waynexia requested a review from killme2008 April 12, 2023 08:48
@killme2008
Copy link
Contributor

What's the benchmark result? @discord9 Can you attach it to comment?

@discord9
Copy link
Contributor Author

What's the benchmark result? @discord9 Can you attach it to comment?

Updated in desc

Copy link
Contributor

@killme2008 killme2008 left a comment

Choose a reason for hiding this comment

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

LGTM

@killme2008 killme2008 merged commit 716bde8 into develop Apr 12, 2023
@killme2008 killme2008 deleted the py_benchmark branch April 12, 2023 10:19
@v0y4g3r v0y4g3r mentioned this pull request Apr 13, 2023
2 tasks
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
* test: bench rspy&pyo3

* docs: add TODO

* api heavy

* Update src/script/benches/py_benchmark.rs

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* style: toml fmt

* test: use `rayon` for threadpool

* test: compile first, run later

---------

Co-authored-by: Ruihang Xia <waynestxia@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants