--bwlimit and external project dependency
Opened this issue · 2 comments
In native rsync, io.c, the sleep_for_bwlimit function is commented as follows:
/* Sleep after writing to limit I/O bandwidth usage.
*
* @todo Rather than sleeping after each write, it might be better to
* use some kind of averaging. The current algorithm seems to always
* use a bit less bandwidth than specified, because it doesn't make up
* for slow periods. But arguably this is a feature. In addition, we
* ought to take the time used to write the data into account.
*
* During some phases of big transfers (file FOO is uptodate) this is
* called with a small bytes_written every time. As the kernel has to
* round small waits up to guarantee that we actually wait at least the
* requested number of microseconds, this can become grossly inaccurate.
* We therefore keep track of the bytes we've written over time and only
* sleep when the accumulated delay is at least 1 tenth of a second. */
When adding --bwlimit to yajsync the mentioned improvement of the @todo section above (averaging) should be considered. The token bucket algorithm would fit the requirements of such an improvement.
Taking a look at https://github.com/bbeck/token-bucket I guess this would be an appropriate implementation to simply integrate bandwidth limits. Taking in account you like to keep dependencies low:
- would you propose to re-implement such an algorithm?
- would you propose to re-use existing code but integrate it with its license to the yajsync source code as long as being compatible?
- would you be open for a maven dependency?
I would not like to add that as an external dependency for such a simple thing. If this becomes a problem, then it might be added later (but unlikely, it's not that important to be that exact with the limit). So I would like to have the simple solution for this.
I modified https://github.com/bbeck/token-bucket to remove any dependency on https://github.com/google/guava, see token-bucket-wo-dependencies.zip.
Would it fit into the design of yajsync to add the resulting classes to a separate package like com.github.perlundq.yajsync.internal.util.ratelimit? I could rework the implementation and integrate everything into one class with multiple inner classes if you prefer a totally encapsulated implementation.
In case of bwlimit!=0 the token bucket would be instantiated as a singleton and used at low-level read/write operations in the channels.