wget-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Wget-dev] wget2 | WIP: Implemented limit-rate feature (!420)


From: Rohan Fletcher
Subject: Re: [Wget-dev] wget2 | WIP: Implemented limit-rate feature (!420)
Date: Sun, 21 Apr 2019 18:11:51 +0000



Thanks for the feedback! Updated the commit message and removed the TODO - 
please let me know if it is still an issue. I have also updated the code and 
added a comment referencing where the algorithm came from (it was influenced by 
rsync's sleep_for_bwlimit function)

> The docs give an example of `=2.5k`, which gives me an error. Would be good 
> to have this working.

I could not reproduce your error with `--limit-rate=2.5k` set as an argument. 
Could you please give me more context? The command line I used to test this was:
```
./src/wget2_noinstall -P tmp -m --progress bar \
                      --limit-rate=2.5k https://speed.hetzner.de/
```

> Having a low rate like `--limit-rate=2k` together with `--progress=bar` 
> reveals a little issue with the KB/s display of the progress bar. It only 
> starts to update after \~100k. Maybe you can address that within your MR (a 
> second commit for that is fine).

I think it has to do with this:

- The speed will not print until the bytes ring buffer is full.
- Each call to _bar_update_speed_stats with a value of bytes_downloaded that 
has changed since last call increments the position of the bytes ring buffer.
- If `wget_bar_update` is called with a non-changed bytes_downloaded value, the 
call will not increment the position of the bytes ring buffer

Also - the data length I get passed into `_get_body` is either 16k or 100k... 
At 2.5k limit rate there will be a sleep time of up to 20 seconds at the end of 
every call...

> Do you see any chance to add a test for this ?

Tests that could be added:

- Test actual download speed is approximately matching limit-rate
- Test download speed is not affected when limit-rate is not set

I had a look at the test suite briefly... Any suggestions on how to make this 
simpler to do?

-- 
Reply to this email directly or view it on GitLab: 
https://gitlab.com/gnuwget/wget2/merge_requests/420#note_162764892
You're receiving this email because of your account on gitlab.com.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]