1

I'm trying to limit download speed in a multithreaded downloader application (like IDM). I've done this so far with the help of this answer, but it doesn't work and I have no idea why. Here is my wrapper class:

public class DownloadStream
{
    InputStream in;
    long timestamp;
    static int counter = 0;
    static int INTERVAL = 1000;
    static int LIMIT;
    static boolean speedLimited;

    public DownloadStream(InputStream stream, boolean speedLimited, int kbytesPerSecond)
    {
        LIMIT = kbytesPerSecond * 1000;
        this.speedLimited = speedLimited;
        in = stream;
    }

    public int read(byte[] buffer) throws IOException
    {
        if(!speedLimited)
            return in.read(buffer);

        check();

        int res = in.read(buffer);
        if (res >= 0) {
            counter += res;
        }
        return res;
    }

    public synchronized void check()
    {
        if (counter > LIMIT)
        {
            long now = System.currentTimeMillis();
            if (timestamp + INTERVAL >= now) {
                try
                {
                    Thread.sleep(timestamp + INTERVAL - now);
                }
                catch (InterruptedException e)
                {
                    e.printStackTrace();
                }
            }
            timestamp = now;
            counter -= LIMIT;
        }
    }
}

I use it like an InputStream in each of my downloader threads:

byte[] buffer = new byte[4096];
while((length = input.read(buffer)) != -1)
{
    output.write(buffer,0,length);
}

Download speed goes up to 16 kilobytes per second if I enter 1 as kbytesPerSecond with eight threads. I've made counter static to prevent from this, but it doesn't work.

Community
  • 1
  • 1
John Matters
  • 39
  • 1
  • 9
  • You seem to not initialize `timestamp`, so `(timestamp + INTERVAL >= now)` is always false. – Victor Sorokin Jun 29 '15 at 21:18
  • 1
    @VictorSorokin I saw that, but after that there is a statement `timestamp = now`. So I don't think that's the problem. – John Matters Jun 29 '15 at 21:32
  • Making random fields static is never going to work in a multi-threaded application. *None* of those fields should be static except for INTERVAL, and that should be final. You should make this class extend `FilterInputStream` to make it more usable. – user207421 Jun 29 '15 at 23:11
  • @EJP I tried to make all not `static` first (like in the example). Probably making `counter` not `static` will be better, but I don't understand why `speedLimited` and `LIMIT` should not be `static` since all threads will check them. I don't have to change them for all instances this way. – John Matters Jun 29 '15 at 23:21
  • You have to *provide* them for all instances anyway, because you've defined them as constructor arguments. Make up your mind. If all threads will check them why provide them in the constructor? Constructor arguments refer to instance data. – user207421 Jun 30 '15 at 00:56

0 Answers0