首页
学习
活动
专区
圈层
工具
发布
社区首页 >问答首页 >提高代码下载速度

提高代码下载速度
EN

Code Review用户
提问于 2014-08-24 06:42:47
回答 1查看 2.7K关注 0票数 8

我正在用Java制作一个下载管理器。我的代码目前正在工作,下载速度为430-450 kb/s。同样的文件,当用下载方式下载时,可以获得440-460的速度,而当使用互联网下载管理器时,它会给出500+ kb/s。

我正在寻找我的代码中的性能缺陷( CPU、内存和网络速度的低效使用)。

代码语言:javascript
复制
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.net.URI;
import java.nio.channels.Channels;
import java.nio.channels.FileChannel;
import java.nio.channels.ReadableByteChannel;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicLongArray;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.CloseableHttpClient;

public class Downloader {

    // States:
    // 1 = downloading
    // 2 = completed
    // 0 = paused
    // -1 = failed
    AtomicInteger currentState = new AtomicInteger(1);
    String downloadDirectory;
    URI uri;
    int segments;
    String fileName;
    File fileFile;
    RandomAccessFile file;
    CloseableHttpClient client;
    FileChannel fileChannel;
    AtomicLong bytesDone;
    int speed;
    int avgSpeed[] = {0, 0};
    AtomicLongArray stateArray;
    States states;
    long sizeOfEachSegment;
    long sizeofFile;
    ExecutorService threadService;
    long[] intialState;

    // Constructor
    public Downloader(String downloadDirectory, URI uri, CloseableHttpClient client,
            long[] initialState, long byteDone, int segments, long sizeOfEachSegment) {
        this.speed = 0;

        this.downloadDirectory = downloadDirectory;
        this.uri = uri;
        this.client = client;
        this.intialState = initialState;
        this.bytesDone = new AtomicLong(byteDone);
        this.segments = segments;
        this.sizeOfEachSegment = sizeOfEachSegment;

        states = States.getInstance();
        fileName = Utilities.getFileName(uri);
        fileFile = new File(downloadDirectory + fileName);
        try {
            file = new RandomAccessFile(fileFile, "rw");
        } catch (FileNotFoundException ex) {
            System.out.println("failed to create file");
        }

        fileChannel = file.getChannel();
        stateArray = new AtomicLongArray(intialState);
        threadService = Executors.newFixedThreadPool(10 + 2);
    }

    public void startDownload() {
        threadService.execute(() -> {
            double currentBytes = bytesDone.doubleValue();
            //Download each segment independently.
            for (int i = 0; i < segments; i++) {
                if (intialState[i] != -1) {
                    threadService.execute(new Segment((i * sizeOfEachSegment)
                            + intialState[i], (i + 1) * sizeOfEachSegment, i));
                }
            }

            if (intialState[segments] != -1) {
                threadService.execute(new Segment((segments * sizeOfEachSegment)
                        + intialState[segments], sizeofFile, segments));
            }

            // Keep saving states of threads. And updating speed.
            while (bytesDone.get() < sizeofFile) {
                for (int i = 0; i < 1; i++) {
                    try {
                        Thread.sleep(5000);
                    } catch (InterruptedException ex) {
                        System.out.println("thread interupted while sleeping");
                    }

                    System.out.println(speed
                            = (int) ((bytesDone.doubleValue() - currentBytes) / 5120));
                    currentBytes = bytesDone.doubleValue();
                    avgSpeed[0] += speed;
                    avgSpeed[1]++;

                }
                states.saveState(stateArray, currentState);
            }
            // Download Complete.
            try {
                fileChannel.close();
                file.close();
            } catch (IOException ex) {
                System.out.println("failed to close file");
            }
            currentState.set(2);
            states.saveState(stateArray, currentState);
            System.out.println("Alhamdullilah Done :)");
            System.out.println("Average Speed : " + avgSpeed[0] / avgSpeed[1]);
        });
    }

    public class Segment implements Runnable {

        long start;
        long end;
        long delta;
        int name;

        public Segment(long start, long end, int name) {
            this.start = start;
            this.end = end;
            this.name = name;
        }

        @Override
        public void run() {
            try {
                HttpGet get = new HttpGet(uri);
                // Range header for defining which segment of file we want to receive.
                String byteRange = start + "-" + end;
                get.setHeader("Range", "bytes=" + byteRange);
                try (CloseableHttpResponse response = client.execute(get)) {
                    ReadableByteChannel inputChannel = Channels.newChannel(
                            response.getEntity().getContent());

                    while (start < end && currentState.get() == 1) {
                        delta = fileChannel.transferFrom(inputChannel, start, 81920);
                        start += delta;
                        bytesDone.addAndGet(delta);
                        stateArray.set(name, start);
                    }

                    stateArray.set(name, -1);
                }
                System.out.println("Thread done: " + name);

            } catch (IOException ex) {
                System.out.println("thread " + name + " failed to download");

            }
        }
    }
}

我所做的是:

有两个构造函数,一个用于创建新下载,另一个用于恢复下载。开始下载启动多个下载线程,然后继续保存下载状态,直到下载完成。

EN

回答 1

Code Review用户

回答已采纳

发布于 2014-09-06 18:48:21

JavaDoc,JavaDoc,JavaDoc

代码语言:javascript
复制
// States:
// 1 = downloading
// 2 = completed
// 0 = paused
// -1 = failed
AtomicInteger currentState = new AtomicInteger(1);

这就是为什么存在Enum的原因。您在这里使用的是一个名为“神奇整数”,由于许多原因,例如可读性、可用性和文档噩梦,都是不好的。

如果您有有意义的数值,请使用Enum

代码语言:javascript
复制
public Downloader(String downloadDirectory, URI uri, CloseableHttpClient client,
        long[] initialState, long byteDone, int segments, long sizeOfEachSegment) {

为什么构造函数接受这么多值?为什么构造函数接受表示已经下载的字节的值?这就是为什么你需要JavaDoc。

此外,构造函数不应该接受这么多的参数,这是令人困惑的。添加合理的默认值和getter/setter来更改这些选项。

代码语言:javascript
复制
    } catch (FileNotFoundException ex) {
        System.out.println("failed to create file");
    }

记录stdout的异常有多种原因,这取决于应用程序的启动方式--可能没有stdout,或者客户端不希望程序打印为stdout。此外,您提供的消息对于调试目的也是无用的。使用记录器记录详细消息和异常。

为什么默认情况下登录到stdout很糟糕?我正在与一家大型软件公司的图书馆合作,该公司确实向stdout打印消息,类似于:

发生异常,对象为空。发生异常,对象为空。发生异常,对象为空。发生异常,对象为空。

每次被使用它都会这样做。它是噪音,更糟糕,它是无用的噪音。我第一次花了大约10分钟,直到我意识到这既不是致命的消息(操作完成成功),也不是来自我的代码。

代码语言:javascript
复制
public class Segment implements Runnable {

这个类需要是public还是非final

在有效的Java第17项中,据说每个未明确设计为子类的类都应该是final

现在,我并没有那么严格,但我确实相信,每一个“一击”类,每个只有单一、受限和范围使用范围的类都应该是final。尤其是可以内联但为可读性而提取的类。让这些类private final发出明确的信息:“这只在这里使用,如果我幸运的话,once...maybe会使用两次”。

代码语言:javascript
复制
avgSpeed[0] += speed;
avgSpeed[1]++;

这是不好的,这应该是两个变量。

总的来说,这个类的设计很奇怪。

代码语言:javascript
复制
class Downloader
    Downloader(...)
    startDownload() // Should be called start() or begin().

    class Segment // Should be private final and called SegmentDownloader

这引起了多方面的思考:

  • 如何通知客户端下载已经完成?
  • 这个类如何处理对startDownload()的多个调用?
  • 它如何处理故障?
  • 为什么有这么多相似的命名变量?
  • 为什么允许我分别设置segmentssizeOfEachSegment
  • 为什么所有变量包都是私有的?

好吧,我试着回答我提出的一些问题。

应该有一种向客户端通知完成的方法,最简单的方法是提供属性isFinished和/或允许为完成事件注册侦听器。

目前,这个类根本不处理对startDownload()的多个调用,它只是从那里走下坡路。应该在startDownload的开头进行状态检查,如果下载正在进行,或者(如果愿意)下载完成,那么让我们的方法失败。

失败从来没有以一种有用的方式报告。从stdout打印某些东西只在编写代码时用于调试目的。使用适当的记录器并在必要时抛出异常。例如,startDownload应该测试目标文件是否存在,目标位置是否可以被写入,如果失败,它应该抛出一个异常。

你有很多变数。您确定在类范围内需要所有这些内容吗?你能限制其中一些的范围吗?

构造函数接受太多的参数,也就是混乱的参数。您应该删除它们中的大多数,而提供getter/setter,以便在初始化后可以更改它们,如有必要,如下所示:

代码语言:javascript
复制
Downloader downloader = new Donwloader(fromThisAddress, targetLocation);
downloader.setSegmentCount(5);
downloader.start();

另外,为什么允许我设置Downloader的某种内部状态,比如HTTP和初始状态?

所有变量都是包私有的,这很糟糕。始终将所有事情的范围限制在尽可能小的范围内,只有在必要时才能扩展它。局部变量应该始终是private,除非它们显式地需要从外部被触摸,或者您计划让一个扩展类去触摸它们。内部类应该是private final,除非您需要让外部的人接触它们。

总之,有两个类和三个函数,它们同时做的太多了。将您的函数拆分为较小的函数,给它们起好的名称,并将所有功能的范围限制在尽可能小的范围内。也可以使用JavaDoc,这个类更容易阅读和使用。

票数 10
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/60900

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档