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: add possibility to specify DL and UL chunk size #174

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

berezovskyi-oleksandr
Copy link

@berezovskyi-oleksandr berezovskyi-oleksandr commented Feb 1, 2024

It might be crucial to have the possibility to configure the chunk sizes. I observed that in some cases chunk size impacts the results.

@r3inbowari
Copy link
Collaborator

Thank you very much for your PR. Could you provide their experiental report?

In fact, chunk size is only an approximate value which always less than the preset value(jpg images are compressed). So should it be marked with approximate value on tip?

@berezovskyi-oleksandr
Copy link
Author

berezovskyi-oleksandr commented Feb 8, 2024

I observe significant improvement in results with 4k "size" for shitty linlks.
Ran a set of tests with tc+netem:
tc qdisc change dev eth0 root handle 1: netem delay 300ms 25ms loss 1%

Experiment 1:

Download Upload Latency
`speedtest-go` with 1k chunk 6263587 B/s 658662 B/s 297 ms
`speedtest-cli` 26171990 B/s 636274 B/s 304 ms
24% 103%

Experiment 2:

Download Upload Latency
`speedtest-go` with 4k chunk 8072362 B/s 747100 B/s 296 ms
`speedtest-cli` 8929612 B/s 637856 B/s 300 ms
90% 117%

On artificially degraded link with stable conditions, 4k version of speedtest-go shows results significantly closer to those, shown by 1k: 10% vs 76% difference from the reference implementation

@berezovskyi-oleksandr
Copy link
Author

@r3inbowari are there any chances to get this merged?

@r3inbowari
Copy link
Collaborator

are there any chances to get this merged?

Yes, we will merge it after vacation.

@r3inbowari
Copy link
Collaborator

Hi, @berezovskyi-oleksandr, I make a change to fix multi server test error. could you test it, if ok, we can merge it now.

@eric
Copy link
Contributor

eric commented Mar 30, 2024

I'd like to be able to specify this as well.

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