2
\$\begingroup\$

I have implemented a Service, which executes three operations, in the following order:

  1. Downloading a compressed (.zip) CSV file
  2. Decompress the CSV file
  3. Reading the CSV file and inserting the data into a SQLite database

I'm more or less pleased with the performance of (1) and (2), but (3) is taking quite some time. Depending on which unit I debug on, and the size of the CSV file, it takes 45-120 seconds. I'd like this part to be a bit faster.

As stated, the first and second operations are currently fine in the aspect of performance, but I will add the code and appreciate any input that would lead to an increased performance. However, focus is on (3), since it's the slowest as of now.

(1) Downloading and (2) Decompressing

Below is the code that defines the AsyncTask which downloads and decompresses the CSV file. It's started from a service, and is provided with a URL which points to the location of the file. Also provided is the helper class which handles the decompress logic.

private class DownloadSaidAndDoneDataTask extends AsyncTask<String, String, String> {

    @Override
    protected void onPreExecute() {
        super.onPreExecute();           
    }

    @Override
    protected String doInBackground(String... params) {
        int count;
        try {
            URL url = new URL(params[0]);
            URLConnection connection = url.openConnection();
            connection.connect();
            int lenghtOfFile = connection.getContentLength();
            InputStream input = new BufferedInputStream(url.openStream(), 10 * 1024);

            File folder = new File(Environment.getExternalStorageDirectory().getPath() + "/Riksdagskollen");
            if (!folder.exists()) {
                folder.mkdir();
            }

            // Output stream to write file in SD card
            OutputStream output = new FileOutputStream(Environment.getExternalStorageDirectory().getPath() + "/Riksdagskollen/Sagtochgjort.zip");
            byte data[] = new byte[1024];
            long total = 0;
            while ((count = input.read(data)) != -1) {
                total += count;
                // Publish the progress which triggers onProgressUpdate method
                publishProgress("" + (int) ((total * 100) / lenghtOfFile));
                // Write data to file
                output.write(data, 0, count);

            }
            // Flush output
            output.flush();
            // Close streams
            output.close();
            input.close();

            //Unzip
            String zipFile = Environment.getExternalStorageDirectory() + "/Riksdagskollen/Sagtochgjort.zip";
            String unzipLocation = Environment.getExternalStorageDirectory() + "/Riksdagskollen/";
            Decompress d = new Decompress(zipFile, unzipLocation);
            d.unzip();

        } catch (Exception e) {
            e.printStackTrace();
        }
        return "";
    }

    @Override
    protected void onProgressUpdate(String... values) {
        super.onProgressUpdate(values);
        //Update progressbar
    }

    @Override
    protected void onPostExecute(String s) {
        super.onPostExecute(s);
        downloadSaidAndDoneDataTask = null;
        //Initiate database insert when download and decompress are done
        insertCsvDataIntoDatabase();
    }
}

Decompress.java

public class Decompress {
    private String _zipFile;
    private String _location;

    public Decompress(String zipFile, String location) {
        _zipFile = zipFile;
        _location = location;

        hanldeDirectory("");
    }

    public void unzip() {

        try {
            FileInputStream inputStream = new FileInputStream(_zipFile);
            ZipInputStream zipStream = new ZipInputStream(inputStream);
            ZipEntry zEntry = null;
            while ((zEntry = zipStream.getNextEntry()) != null) {
                Log.d("Unzip", "Unzipping " + zEntry.getName() + " at "
                        + _location);

                if (zEntry.isDirectory()) {
                    hanldeDirectory(zEntry.getName());
                } else {
                    FileOutputStream fout = new FileOutputStream(
                            this._location + "/" + zEntry.getName());
                    BufferedOutputStream bufout = new BufferedOutputStream(fout);
                    byte[] buffer = new byte[1024];
                    int read = 0;
                    while ((read = zipStream.read(buffer)) != -1) {
                        bufout.write(buffer, 0, read);
                    }

                    zipStream.closeEntry();
                    bufout.close();
                    fout.close();
                }
            }
            zipStream.close();
            Log.d("Unzip", "Unzipping complete. path :  " + _location);
        } catch (Exception e) {
            Log.d("Unzip", "Unzipping failed");
            e.printStackTrace();
        }

    }

    public void hanldeDirectory(String dir) {
        File f = new File(this._location + dir);
        if (!f.isDirectory()) {
            f.mkdirs();
        }
    }
}

(3) Inserting

This operation is my main concern. I've managed to reduce the execution time from 25 minutes to 1-2 minutes by handling the inserts as a single transaction. If possible, I'd like it to be even faster.

private class InsertSaidAndDoneDataIntoDataBase extends AsyncTask<String, String, String> {
    @Override
    protected void onPreExecute() {
        super.onPreExecute();
    }

    @Override
    protected String doInBackground(String... params) {
        String csvFile = Environment.getExternalStorageDirectory().getPath() + "/Riksdagskollen/Sagtochgjort.csv";
        BufferedReader br = null;
        BufferedReader brCount = null;
        String line;
        String cvsSplitBy = ",";
        long total = 0;

        try {
            br = new BufferedReader(new FileReader(csvFile));
            brCount = new BufferedReader(new FileReader(csvFile));
            dataSource.open();
            dataSource.beginTransaction();

            long lenghtOfFile = 0;
            while (brCount.readLine() != null) {
                lenghtOfFile++;
            }

            while ((line = br.readLine()) != null && !canceledDatabaseTransaction) {
                total++;

                String[] data = line.split(cvsSplitBy);
                String personId = data[0];
                String documentType = data[4];
                String subType = data[5];
                String session = data[6];
                String documentId = data[7];
                String term = data[8];
                String authority = data[9];
                String date = data[10];
                String speaker = data[11];
                String speakerTime = data[15];
                String numOfChars = data[16];
                String personActivities = data[17];
                dataSource.createSaidAndDoneEntry(personId, documentType, subType, session, documentId, term, authority, date,
                        speaker, speakerTime, numOfChars, personActivities);

                publishProgress("" + (int) ((total * 100) / lenghtOfFile));

            }
            dataSource.setTransactionSuccessful();


        } catch (IOException e) {
            e.printStackTrace();
        } finally {
            try {
                dataSource.endTransaction();
                dataSource.close();
            } catch (SQLException e) {
                e.printStackTrace();
            }
            if (br != null) {
                try {
                    br.close();
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
            if (brCount != null) {
                try {
                    br.close();
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
        }
        return "";
    }

    @Override
    protected void onProgressUpdate(String... values) {
        super.onProgressUpdate(values);
        //Update progress dialog
    }

    @Override
    protected void onPostExecute(String s) {
        super.onPostExecute(s);           
        insertSaidAndDoneDataIntoDataBase = null;
    }
}

Edit

Below is the code for the createSaidAndDoneEntry method. database is the reference from getReadableDatabase() which I call in a separate method prior to any database calls.

public long createSaidAndDoneEntry(String personId, String documentType, String subType,
                                   String session, String documentId, String term,
                                   String authority, String date, String speaker,
                                   String speakerTime, String numberOfChars,
                                   String personActivities) {

    ContentValues values = new ContentValues();
    values.put(DBHelper.COLUMN_SAIDANDDONE_PERSON_ID, personId);
    values.put(DBHelper.COLUMN_SAIDANDDONE_DOCUMENT_TYPE, documentType);
    values.put(DBHelper.COLUMN_SAIDANDDONE_SUB_TYPE, subType);
    values.put(DBHelper.COLUMN_SAIDANDDONE_SESSION, session);
    values.put(DBHelper.COLUMN_SAIDANDDONE_DOCUMENT_ID, documentId);
    values.put(DBHelper.COLUMN_SAIDANDDONE_TERM, term);
    values.put(DBHelper.COLUMN_SAIDANDDONE_AUTHORITY, authority);
    values.put(DBHelper.COLUMN_SAIDANDDONE_DATE, date);
    values.put(DBHelper.COLUMN_SAIDANDDONE_SPEAKER, speaker);
    values.put(DBHelper.COLUMN_SAIDANDDONE_SPEAKER_TIME, speakerTime);
    values.put(DBHelper.COLUMN_SAIDANDDONE_NUMBER_OF_CHARACTERS, numberOfChars);
    values.put(DBHelper.COLUMN_SAIDANDDONE_PERSON_ACTIVITIES, personActivities);

    return database.insert(DBHelper.TABLE_NAME_SAIDANDDONE, null, values);
}  
\$\endgroup\$
  • \$\begingroup\$ Welcome to CodeReview, Marcus. This is a well written first post! I hope you get some fine answers. \$\endgroup\$ – Legato Apr 22 '15 at 22:41
1
\$\begingroup\$

I guess, you could save quite some time by making the operations overlap: Feed the download stream into the ZipInputStream and feed the output into the database. The timing could be something like max(a, b, c) instead of a+b+c, but it could easily lead to a messy code, so let's forget it for now.

I'll only look at the third part, i.e., InsertSaidAndDoneDataIntoDataBase.

@Override
protected void onPreExecute() {
    super.onPreExecute();
}

This does nothing, so leave it out.

@Override
protected String doInBackground(String... params) {
    String csvFile = Environment.getExternalStorageDirectory().getPath() + "/Riksdagskollen/Sagtochgjort.csv";

The filename surely should be a constant. Moreover, during the decompression you may want to look if such an entry exists.

BufferedReader br = null;
BufferedReader brCount = null;
String line;
String cvsSplitBy = ",";

The first two have to be declared outside of the try-finally block, but the latter two don't. Always minimize the scope.

Also consider something smarter than classical try-finally, maybe Lombok's @Cleanup or Guava's Closer.

I'd also use a single BufferedReader variable only. After counting the lines, I'd close it and reassign a new BufferedReader to the variable. Re-reading a file is pretty wasteful, but I guess, you can't simply read it all into memory (as ArrayList<String>)?

long lenghtOfFile = 0;

But it's not the file length.

dataSource.createSaidAndDoneEntry

I'm afraid, this is the time-consuming part. And you're keeping the dataSource code secret.

Some databases can read CSV directly, I'd check this first. Otherwise, I'm not sure about Android, but normally you should

\$\endgroup\$
  • \$\begingroup\$ Hi @maaartinus, thank you for your valuable input. I've added the code for dataSource.createSaidAndDoneEntry. I'll also have a look at your suggestions regarding PreparedStatement and addBatch. In the meantime, please have a look at my edit. Thanks. \$\endgroup\$ – Marcus Apr 23 '15 at 7:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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