wouldn't it then be easier and smarter to use the data->progress.current_speed number instead? We already have it and it is the transfer speed during the last 5 seconds.

No, it actually gives different - and less useful - results (and also,
current_speed is a single value, while there could be different limits
set for upload & download, so it would need to be "duplicated"). It was
also my first idea, and what I originally tried.

But things aren't as good then, I believe it has to do with the fact
that current_speed is indeed "time-based". A simple case to illustrate
is: imagine a transfer that stops at some point for a few feconds, the
current speed would then stay constant for 5s (as all data counters
stay the same), and then once we loop in the speeders it drops to 0.
Limiting behavior based on the current speed wasn't performing as hoped.
For limits, it's fine in such a case to simply wait until enough data
has been transfered (however long, or short, that may be) and then
calculate speed and impose wait time if needed.

So having a "fixed" starting point to calculate the limit gives much
better result, and then moving said starting point after enough data has
been transfered allows to keep things "current" and not "average" -
which gives the best results.

A simple case to illustrate is: imagine a transfer that stops at some point for a few feconds, the current speed would then stay constant for 5s (as all data counters stay the same), and then once we loop in the speeders it drops to 0.

I don't get that. If the transfer pauses, the current_speed will drop rather sharply every second until it reaches zero after 5 seconds. Right?

But I agree that it would need to be duplicated (one for each direction) to be possible to be used for this.

For limits, it's fine in such a case to simply wait until enough data has been transferred (however long, or short, that may be) and then calculate speed and impose wait time if needed.

So let me check if I understand your suggested algorithm (for a single direction).

1. store a time stamp
2. wait until the speed limit amount of data has been received
3. check how long it took and deem how long to wait (if at all), then GOTO 1

Is that what you're proposing?

A simple case to illustrate is: imagine a transfer that stops at
some point for a few feconds, the current speed would then stay
constant for 5s (as all data counters stay the same), and then once
we loop in the speeders it drops to 0.

I don't get that. If the transfer pauses, the current_speed will
drop rather sharply every second until it reaches zero after 5
seconds. Right?

Well, depends, because the "current speed" is really an average over
the last 5 seconds. It is calculated using current amount transfered,
and amount transfered 5s ago.

Imagine this, with no transfer happening for the last 5s:

t : speed based on amount of data from t-5
t+1s: speed based on same amount, so same speed
t+2s: still based on same amount, so still same speed
...
t+6s: speed based on amount from t, i.e. current one, speed drops to 0

It also wouldn't get to the actual current speed right away. That is,
if after such a "pause" data comes in with 1M in a second,
current_speed wouldn't say 1M/s because it would average it over the
last 5s, so ~205K/s only.

Anyways, because the current speed is in fact the average over the last
5s (which is probably good for what it is, so it doesn't jump
constantly), it doesn't work as well/have good reactivity, so it's not
ideal for implementing speed limits.

So let me check if I understand your suggested algorithm (for a
single direction).

1. store a time stamp
2. wait until the speed limit amount of data has been received
3. check how long it took and deem how long to wait (if at all), then
   GOTO 1

Is that what you're proposing?

Yes. It's a timestamp & amount of data received then, ofc, but yes
that's it. That way we get instant reactivity and
calculation over "current speed".

FYI, this was inspired from openssh's scp, because after trying to use
the current speed and not getting satisfying results I did some looking
around, and scp is a tool that gives me satisfying results with its
limit, so I looked how they did it, which is basically this.

Ok, yes that all makes sense. Thanks for explaining. This approach has one additional benefit too: it will support changing the limits in run-time much better than it currently does. That is something that I keep hearing users ask for. I think it could make sense to actually document that fact as well in association with landing this patch.

Thanks a lot!