我正在用Java制作一个下载管理器。我的代码目前正在工作,下载速度为430-450 kb/s。同样的文件,当用下载方式下载时,可以获得440-460的速度,而当使用互联网下载管理器时,它会给出500+ kb/s。
我正在寻找我的代码中的性能缺陷( CPU、内存和网络速度的低效使用)。
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");
}
}
}
}我所做的是:
有两个构造函数,一个用于创建新下载,另一个用于恢复下载。开始下载启动多个下载线程,然后继续保存下载状态,直到下载完成。
发布于 2014-09-06 18:48:21
JavaDoc,JavaDoc,JavaDoc。
// States:
// 1 = downloading
// 2 = completed
// 0 = paused
// -1 = failed
AtomicInteger currentState = new AtomicInteger(1);这就是为什么存在Enum的原因。您在这里使用的是一个名为“神奇整数”,由于许多原因,例如可读性、可用性和文档噩梦,都是不好的。
如果您有有意义的数值,请使用Enum!
public Downloader(String downloadDirectory, URI uri, CloseableHttpClient client,
long[] initialState, long byteDone, int segments, long sizeOfEachSegment) {为什么构造函数接受这么多值?为什么构造函数接受表示已经下载的字节的值?这就是为什么你需要JavaDoc。
此外,构造函数不应该接受这么多的参数,这是令人困惑的。添加合理的默认值和getter/setter来更改这些选项。
} catch (FileNotFoundException ex) {
System.out.println("failed to create file");
}记录stdout的异常有多种原因,这取决于应用程序的启动方式--可能没有stdout,或者客户端不希望程序打印为stdout。此外,您提供的消息对于调试目的也是无用的。使用记录器记录详细消息和异常。
为什么默认情况下登录到stdout很糟糕?我正在与一家大型软件公司的图书馆合作,该公司确实向stdout打印消息,类似于:
发生异常,对象为空。发生异常,对象为空。发生异常,对象为空。发生异常,对象为空。
每次被使用它都会这样做。它是噪音,更糟糕,它是无用的噪音。我第一次花了大约10分钟,直到我意识到这既不是致命的消息(操作完成成功),也不是来自我的代码。
public class Segment implements Runnable {这个类需要是public还是非final?
在有效的Java第17项中,据说每个未明确设计为子类的类都应该是final。
现在,我并没有那么严格,但我确实相信,每一个“一击”类,每个只有单一、受限和范围使用范围的类都应该是final。尤其是可以内联但为可读性而提取的类。让这些类private final发出明确的信息:“这只在这里使用,如果我幸运的话,once...maybe会使用两次”。
avgSpeed[0] += speed;
avgSpeed[1]++;这是不好的,这应该是两个变量。
总的来说,这个类的设计很奇怪。
class Downloader
Downloader(...)
startDownload() // Should be called start() or begin().
class Segment // Should be private final and called SegmentDownloader这引起了多方面的思考:
startDownload()的多个调用?segments和sizeOfEachSegment?好吧,我试着回答我提出的一些问题。
应该有一种向客户端通知完成的方法,最简单的方法是提供属性isFinished和/或允许为完成事件注册侦听器。
目前,这个类根本不处理对startDownload()的多个调用,它只是从那里走下坡路。应该在startDownload的开头进行状态检查,如果下载正在进行,或者(如果愿意)下载完成,那么让我们的方法失败。
失败从来没有以一种有用的方式报告。从stdout打印某些东西只在编写代码时用于调试目的。使用适当的记录器并在必要时抛出异常。例如,startDownload应该测试目标文件是否存在,目标位置是否可以被写入,如果失败,它应该抛出一个异常。
你有很多变数。您确定在类范围内需要所有这些内容吗?你能限制其中一些的范围吗?
构造函数接受太多的参数,也就是混乱的参数。您应该删除它们中的大多数,而提供getter/setter,以便在初始化后可以更改它们,如有必要,如下所示:
Downloader downloader = new Donwloader(fromThisAddress, targetLocation);
downloader.setSegmentCount(5);
downloader.start();另外,为什么允许我设置Downloader的某种内部状态,比如HTTP和初始状态?
所有变量都是包私有的,这很糟糕。始终将所有事情的范围限制在尽可能小的范围内,只有在必要时才能扩展它。局部变量应该始终是private,除非它们显式地需要从外部被触摸,或者您计划让一个扩展类去触摸它们。内部类应该是private final,除非您需要让外部的人接触它们。
总之,有两个类和三个函数,它们同时做的太多了。将您的函数拆分为较小的函数,给它们起好的名称,并将所有功能的范围限制在尽可能小的范围内。也可以使用JavaDoc,这个类更容易阅读和使用。
https://codereview.stackexchange.com/questions/60900
复制相似问题