0

I want to download a file from an URL. The file size is 564.31MB. I have no clue what error is going on here. Also, I'm wondering if my code is the correct way to download a file from an URL. If there is a better way, please tell me in detail why it is better than this. Thanks.

import org.apache.commons.io.FilenameUtils;
import java.io.*;
import java.net.MalformedURLException;
import java.net.URL;

/**
 * Created by lukas on 6/30/16.
 */
public class Main {
    public static void main(String[] args){
        try {
            String link = "https://s.basketbuild.com/uploads/devs/dianlujitao/oneplus3/cm13/cm-13.0-20160621-UNOFFICIAL-oneplus3.zip";
            URL url = new URL(link);
            InputStream inputStream = new BufferedInputStream(url.openStream());
            ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
            int n=0;
            byte[] buf = new byte[1024];
            long duration = System.currentTimeMillis();
            while((n=inputStream.read(buf))!=-1){
                byteArrayOutputStream.write(buf, 0, n);
            }
            duration = System.currentTimeMillis()-duration;
            System.out.println("Finish in "+duration+"ms");

            inputStream.close();

            File dir = new File("Output path");
            if(!dir.exists())
                dir.mkdirs();

            String fileBaseName = FilenameUtils.getBaseName(link);
            String fileExtension = FilenameUtils.getExtension(link);
            System.out.println("Name: "+fileBaseName+'.'+fileExtension);
            File outputFile = new File(dir, fileBaseName+'.'+fileExtension);

            if(!outputFile.exists()){
                outputFile.createNewFile();
            }

            FileOutputStream fileOutputStream = new FileOutputStream(outputFile);
            fileOutputStream.write(byteArrayOutputStream.toByteArray());

            //release outputstream
            byteArrayOutputStream.close();
            fileOutputStream.close();

            System.out.println("Your download has been finished");


        } catch (MalformedURLException e) {
            e.printStackTrace();
            System.out.println("Something unexpected has happened!");
        } catch (IOException e) {
            e.printStackTrace();
            System.out.println("Something unexpected has happened!");
        }


    }
}

Here is what my console says:

Finish in 133506ms
Name: cm-13.0-20160621-UNOFFICIAL-oneplus3.zip
Exception in thread "main" java.lang.OutOfMemoryError: Java heap space
    at java.util.Arrays.copyOf(Arrays.java:3236)
    at java.io.ByteArrayOutputStream.toByteArray(ByteArrayOutputStream.java:191)
    at Main.main(Main.java:42)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:144)
Lukas Herman
  • 13
  • 1
  • 3
  • Possible duplicate of [How to deal with "java.lang.OutOfMemoryError: Java heap space" error (64MB heap size)](http://stackoverflow.com/questions/37335/how-to-deal-with-java-lang-outofmemoryerror-java-heap-space-error-64mb-heap) – Software Engineer Jul 09 '16 at 01:18
  • Welcome to SO. It's a good idea to search for possible answers before spending time writing a question. Usually just googling the main error message will do (in this case it's `java.lang.OutOfMemoryError: Java heap space`). – Software Engineer Jul 09 '16 at 01:19
  • And, that's 2 questions. It's best to keep them separate. Typically 'how to do this better' questions don't get a lot of answers because that's not what we're here for. Please read [ask] and [answer]. – Software Engineer Jul 09 '16 at 01:20
  • 2
    Why don't you write directly to the `FileOutputStream`, instead of the `ByteArrayOutputStream`? That way you could download a 100GB file without running out of memory. --- Also, you don't need to call `createNewFile()`. – Andreas Jul 09 '16 at 01:23

1 Answers1

7

  1. ByteArrayOutputStream allocates all its data in the heap memory. The correct way is to write directly to the file, and use a buffer for that so disk I/O is optimized:

    OutputStream os = new BufferedOutputStream(new FileOutputStream("myFile.txt"));

  2. System.currentTimeMillis() gives you the current date and time, which may change at any point. It's not meant to calculate durations, use SystemClock.nanoTime() for that.

Adrian Crețu
  • 878
  • 8
  • 17
  • Don't use a `BufferedOutputStream` here. Allocate a fixed size buffer, and read and write the data in chunks with that. – erickson Jul 09 '16 at 02:13
  • 1
    @erickson This îs what exactly BufferedOutputStream does. Your method does not guarantee the chunk îs full, reading data off the network can even returm one byte at a time. Unbuffered disk I/O is overkill. – Adrian Crețu Jul 09 '16 at 03:43
  • There's no need to guarantee the chunk is full. Single-byte network reads when downloading a file would be pathological. Redundant copying is overkill. – erickson Jul 09 '16 at 04:04
  • I think that's an opinion. Using an extra 8192 bytes of RAM buffering that, when full, are written synchronossly to disk, is way, way, way faster than writing an unknown N bytes (where 0 < N <= buffer.length) to disk for an unknown number of times (for the same total buffer size). The blocking I/O times do add up (because of the disk access overhead, etc), But again, this just depends on how you expect your data to be read. You don't have any guarantee that a server/proxy/whatever won't decide to return data in 64 bytes chunks, if the file is dynamic, etc, etc. – Adrian Crețu Jul 09 '16 at 10:15