3
\$\begingroup\$

I am trying to optimize my code when downloading around 50 pdf files from a website. All of the pdf are big in size (average 25mb). Is there any way to optimize further to download the pdf faster?

for (String pdfUrl : pdfUrls) {
    URLConnection request;
    request = new URL(pdfUrl).openConnection();
    InputStream in = request.getInputStream();
    File downloadedFile = File.createTempFile("download-pdf-" + System.currentTimeMillis(), ".pdf");
    FileOutputStream out = new FileOutputStream(downloadedFile);
    byte[] buffer = new byte[10240];
    int len = 0;
    int bytesBuffered = 0;
    while ((len = in.read(buffer)) != -1) {
        out.write(buffer, 0, len);
        bytesBuffered += len;
        if (bytesBuffered > 1024 * 1024) {
            out.flush();
        }
    }
    in.close();
    out.close();
}
\$\endgroup\$
1
  • \$\begingroup\$ @DebmalyaBiswas, In general it's considered bad to edit the code after answers has been given, as it might invalidate questions. We're gonna let it slide this time, as that doesn't seem to be the main issue with your code. \$\endgroup\$
    – holroy
    Jan 13, 2016 at 4:39

1 Answer 1

5
\$\begingroup\$

Correctness

Files over 10240 bytes will fail to download because the streams are closed after one iteration of the while loop(Edit: Was paste error by OP). As of java 7, it is highly preferable to use a try-with-resources block when working with AutoCloseable objects.

Implementation

Wrapping your FileOutputStream in a BufferedOutputStream would be preferable to manually flushing.

Performance

You may see some performance gain by running each file download on its own thread. You'd need to take your existing code and move much of it into a Runnable or Callable. Use an Executor to perform some or all of the downloads concurrently.

If you made these modifications, the code might look more like:

import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.net.URLConnection;
import java.util.Objects;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public final class FileDownloader {

    // or newFixedThreadPool() to control the number of threads.
    private final ExecutorService executor = Executors.newCachedThreadPool();

    public FileDownloader() {
        super();
    }

    public void downloadPdf(final URL targetUrl) throws IOException {
        final File destination = File.createTempFile("download-pdf-" + System.currentTimeMillis(), ".pdf");
        this.executor.submit(new DownloadCallable(targetUrl, destination));
    }

    private static final class DownloadCallable implements Callable<File> {

        private static final int BUFFER_SIZE = 1024 * 1024;

        private final URL targetUrl;
        private final File destination;

        public DownloadCallable(final URL targetUrl, final File destination) {
            this.targetUrl = targetUrl;
            Objects.requireNonNull(targetUrl);

            this.destination = destination;
            Objects.requireNonNull(destination);
        }

        @Override
        public File call() throws IOException {
            final URLConnection request = this.targetUrl.openConnection();
            try (final InputStream inputStream = request.getInputStream();
                    final FileOutputStream fileStream = new FileOutputStream(this.destination);
                    final BufferedOutputStream outputStream = new BufferedOutputStream(fileStream, BUFFER_SIZE);) {

                final byte[] data = new byte[10240];
                int bytesRead = 0;
                while ((bytesRead = inputStream.read(data)) != -1) {
                    outputStream.write(data, 0, bytesRead);
                }
            }
            return this.destination;
        }
    }
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Keep in mind that System.currentTimeMillis() might not be unique enough when requesting multiple downloads at once. \$\endgroup\$
    – Marvin
    Jan 12, 2016 at 20:52
  • \$\begingroup\$ @Marvin Yes, that's a very good point. A simple counter in FileDownloader would probably be a better idea. \$\endgroup\$
    – Eric Stein
    Jan 13, 2016 at 1:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.