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

Optimize time-consuming tasks #17

Merged
merged 10 commits into from
Aug 15, 2024
Merged

Optimize time-consuming tasks #17

merged 10 commits into from
Aug 15, 2024

Conversation

iteyelmp
Copy link
Collaborator

@iteyelmp iteyelmp commented Aug 8, 2024

The kzgwasm.blobToKzgCommitment function will take 600-700ms, change it to concurrent.

Compare files for changes
Total file size: 79.2 MB
Number of files: 988
Unoptimized SDK, Number of concurrencies is 1: 2,161,958 ms = 36 minutes
Unoptimized SDK, Number of concurrencies is 6: 744,458 ms = 12.4 minutes

Optimized SDK for KZG, Number of concurrencies is 6: 281,896 ms = 4.7 minutes
Optimized SDK for KZG, Number of concurrencies is 9: 194,627 ms = 3.2 minutes
Optimized SDK for KZG, Number of concurrencies is 12: 155,411 ms = 2.6 minutes
Optimized SDK for KZG, Number of concurrencies is 15: 126,103 ms = 2.1 minutes
Optimized SDK for KZG, Number of concurrencies is 18: 116,898 ms = 1.95 minutes

If c-kzg is used, The kzg.blobToKzgCommitment function will take 80ms

Some public RPCs have request limits, and the number of concurrencies set at 5-6 will not exceed the limit.
Error: exceeded maximum retry limit (
info: {
requestUrl: 'https://nova.arbitrum.io/rpc',
responseBody: '{"jsonrpc":"2.0","error":{"code":429,"message":"Public RPC Rate Limit Hit, limit will reset in 60 seconds"}}',
responseStatus: '599 CLIENT ESCALATED SERVER ERROR (429 Too Many Requests; exceeded maximum retry limit)'
},
shortMessage: 'exceeded maximum retry limit'
}

@iteyelmp iteyelmp requested review from qzhodl and syntrust August 8, 2024 06:00
src/flatdirectory.js Outdated Show resolved Hide resolved
src/flatdirectory.js Show resolved Hide resolved
@iteyelmp iteyelmp requested a review from qzhodl August 9, 2024 03:51
rollup.config.mjs Show resolved Hide resolved
src/flatdirectory.js Show resolved Hide resolved
@qizhou
Copy link

qizhou commented Aug 20, 2024

Will we retry if the rate limit is reached?

@qizhou
Copy link

qizhou commented Aug 20, 2024

The result seems to be very good!

$ ethfs-cli --version
1.1.1
$ time ethfs-cli upload ...
... cpu 20:20.85 total
$ npm -g install ethfs-cli
$ ethfs-cli --version
1.1.2
$ time ethfs-cli upload ...
... cpu 3:26.05 total
$ time ethfs-cli upload ... -s 6
... cpu 32.539 total
$ time ethfs-cli upload ...
... cpu 3:25.67 total
$ time ethfs-cli upload ... -s 12
... cpu 31.263 total

It surprises me that with -s 6 and without -s (default pool size) yield significantly different results. I wonder whether the parameter is properly read from CLI and set.

@qizhou
Copy link

qizhou commented Aug 20, 2024

BTW: I would suggest printing concurrentThread parameter so that it is easy to verify that the parameter is effective.

@qzhodl
Copy link

qzhodl commented Aug 22, 2024

The result seems to be very good!

$ ethfs-cli --version
1.1.1
$ time ethfs-cli upload ...
... cpu 20:20.85 total
$ npm -g install ethfs-cli
$ ethfs-cli --version
1.1.2
$ time ethfs-cli upload ...
... cpu 3:26.05 total
$ time ethfs-cli upload ... -s 6
... cpu 32.539 total
$ time ethfs-cli upload ...
... cpu 3:25.67 total
$ time ethfs-cli upload ... -s 12
... cpu 31.263 total

It surprises me that with -s 6 and without -s (default pool size) yield significantly different results. I wonder whether the parameter is properly read from CLI and set.

The fixed PR is ethstorage/ethfs-cli#18

@iteyelmp iteyelmp deleted the syncpool branch August 22, 2024 08:25
@qizhou
Copy link

qizhou commented Aug 22, 2024

Updated results

$ ethfs-cli --version
1.1.3
$ time ethfs-cli upload ...
... cpu 3:19.84 total
$ time ethfs-cli upload ... -s 6
... cpu 3:50.14 total
$ time ethfs-cli upload ... -s 15
... cpu 1:22.55 total
$ time ethfs-cli upload ... -s 24
... cpu 57.579 total

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