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

Improve sexp_buffer_from_stream performance #100

Merged
merged 2 commits into from
Aug 12, 2021

Conversation

ChiaMineJP
Copy link
Contributor

@ChiaMineJP ChiaMineJP commented Jul 30, 2021

Summary

When I develop clvm-js as a JavaScript implementation of clvm, I was suffered in poor performance in test_very_deep_tree at tests/serialize_test.

Later I found out that the root cause was huge number of memory allocation/copy invoked upon byte deserialization of sexp instance.

Looking back to clvm of original Python implementation, I noticed that one of the optimizations I applied to my JavaScript implementation can also be applied to the Python's.

So here I am proposing the pure performance enhancement of sexp_buffer_from_stream.

Performance measurement

Test environment

- -
OS Windows10 Pro
CPU AMD Ryzen9 5900X
MEM 64GB@3200MHz
Storage NVMe m.2 SSD Corsair MP600
Python 3.7.8
# Powershell
cd clvm/tests
for($i=0; $i -lt 10; $i++){ python -m unittest serialize_test.SerializeTest.test_very_deep_tree }

Result

Roughly 10-20% faster than before.

test_very_deep_tree

n Before the PR After the PR
1 5.023s 4.014s
2 4.696s 3.996s
3 4.882s 4.021s
4 4.403s 3.993s
5 4.596s 3.997s
6 4.561s 3.983s
7 4.453s 3.987s
8 4.442s 3.998s
9 4.493s 3.996s
10 4.468s 3.990s
AVG 4.602s 3.998s

P.S.

If you're interested, I also compared performance between clvm of Python and clvm of JavaScript.

Test execution

Python

# Powershell
cd <directory>
git clone https://github.com/Chia-Network/clvm
cd clvm/tests
for($i=0; $i -lt 10; $i++){ python -m unittest serialize_test.SerializeTest.test_very_deep_tree }
for($i=0; $i -lt 10; $i++){ python -m unittest serialize_test.SerializeTest.test_very_long_blobs }

JS

# Powershell
cd <directory>
git clone https://github.com/Chia-Mine/clvm-js
cd clvm-js
git checkout v0.0.19
yarn # Or npm install
for($i=0; $i -lt 10; $i++){ yarn test serialize_test --testNamePattern test_very_deep_tree } 
for($i=0; $i -lt 10; $i++){ yarn test serialize_test --testNamePattern test_very_long_blobs } 

Result

test_very_deep_tree

Python JS
1 4.014s 2.29s
2 3.996s 2.3s
3 4.021s 2.288s
4 3.993s 2.251s
5 3.997s 2.254s
6 3.983s 2.289s
7 3.987s 2.253s
8 3.998s 2.26s
9 3.996s 2.301s
10s 3.99s 2.282s
AVG 3.998s 2.277s

test_very_long_blobs

Python JS
1 0.454s 0.791s
2 0.455s 0.741s
3 0.478s 0.77s
4 0.466s 0.798s
5 0.474s 0.799s
6 0.479s 0.807s
7 0.467s 0.818s
8 0.488s 0.751s
9 0.423s 0.758s
10s 0.47s 0.743s
AVG 0.4654s 0.7778s

@ChiaMineJP ChiaMineJP changed the title Improve sexp_buffer_from_stream performance Improve sexp_buffer_from_stream performance Jul 30, 2021
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

the python implementation is not currently used by the full node itself, so its performance isn't critical (the full node uses clvm_rs). However, it is used by our tooling, and this seems like a straight-forward improvement.

@richardkiss richardkiss merged commit 2722c78 into Chia-Network:main Aug 12, 2021
@richardkiss
Copy link
Contributor

Thank you!

@ChiaMineJP ChiaMineJP deleted the faster branch August 13, 2021 01:16
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