4
\$\begingroup\$

Are there any possible speed improvements I can make to the following method. It takes a JDBC ResultSet and breaks it into several CSV files:

public static void convertToCSV(final ResultSet rs) throws SQLException, IOException {
    final DateFormat format = new SimpleDateFormat("yyyyMMdd,HH:mm:ss");
    format.setTimeZone(TimeZone.getTimeZone("Etc/UTC"));
    final StringBuilder sb = new StringBuilder();
    while (rs.next()) {
        final File file = new File(sb.append(rs.getString("FIELD1")).append(".csv").toString());
        if (!file.exists()) {
            file.createNewFile();
        }
        final BufferedWriter fw = new BufferedWriter(new FileWriter(file.getAbsoluteFile(), true));
        fw.write(rs.getString("FIELD2"));
        fw.write(',');
        fw.write(rs.getString("FIELD3"));
        fw.write(',');
        final String clobValue = rs.getString("FIELD4");
        if(clobValue==null)
            fw.write("null,");
        else{
            fw.write('\"');
            fw.write(clobValue);
            fw.write("\",");
        }
        final Date date = new Date(rs.getLong("FIELD5"));
        fw.write(format.format(date));
        fw.write('\n');
        fw.close();
        sb.setLength(0);
    }
    rs.close();
}
\$\endgroup\$
  • \$\begingroup\$ I assume that field 1 always contains a valid filename, and fields 2, 3, and 4 never contain quote characters or comma characters? \$\endgroup\$ – Eric Stein Oct 20 '15 at 18:46
  • \$\begingroup\$ @EricStein Correct. \$\endgroup\$ – John Roberts Oct 20 '15 at 18:47
  • \$\begingroup\$ Is there a specific reason for using Etc/UTC over just UTC? \$\endgroup\$ – lealand Oct 21 '15 at 3:38
  • \$\begingroup\$ @lealand I always thought Etc/UTC was just the standard way of writing it. \$\endgroup\$ – John Roberts Oct 21 '15 at 11:59
  • \$\begingroup\$ @JohnRoberts I've been looking into it over the past couple of days and I think you're correct. Apparently plain UTC is outdated now (don't have the citation on me though). My ears perk when I see Etc though because I think with Java that means a nautical time zone, which is the opposite of what you would think. \$\endgroup\$ – lealand Oct 24 '15 at 4:15
2
\$\begingroup\$

You may see a performance increase by adding multithreading support. For each row, create an instance of a custom Runable that takes the five values and add it to a ThreadPoolExecutor. You can futz around with the size of the pool. As with all performance enhancements, you'll need to test, test, test to make sure you're seeing a real benefit, and you'll really need to test on a production clone for it to have any meaning. It may not make a difference on your local machine but be a big help on production, or vice versa.

Your use of the StringBuilder as opposed to just concatenation is probably not doing you any good. Compilers will do that work, and the code is harder to read as-is. There are other things to pick at, but none of them are performance-related.

\$\endgroup\$
  • \$\begingroup\$ Hmm. I've always thought that multi-threading is something to be avoided when writing to disk, especially since I'm using a standard RAID. Might I not see a performance degradation from that? \$\endgroup\$ – John Roberts Oct 20 '15 at 18:55
  • \$\begingroup\$ @JohnRoberts You certainly might. I am not a performance expert. Hence the emphasis on 'may'. I believe it depends on the number of disks you have and the size of the clobs you're writing. Test, test, test! :-) \$\endgroup\$ – Eric Stein Oct 20 '15 at 19:22
  • \$\begingroup\$ My shoot-from-the-hip thought is that you shouldn't worry about performance degradation from multithreading. Just the other day I implemented multithreading to make use of my cores when processing multiple files and my runtime was cut by at least a third. You have at least thousand threads running on your OS, all doing their own thing. I would be surprised if at this point the hardware wasn't optimized for exactly this. \$\endgroup\$ – lealand Oct 21 '15 at 3:46
  • 2
    \$\begingroup\$ Do note that the parse method of SimpleDateFormat isn't thread safe. This doesn't make a difference in your code because it's instanced on each method invocation, but it you ever extract it to a constant, you'l run into to whole bunch of problems. \$\endgroup\$ – lealand Oct 21 '15 at 3:49
1
\$\begingroup\$

If you're on Java 7, you should use try-with-resources on the BufferedWriter instance for safe and efficient I/O management.

Since you only use your StringBuilder once to create the filename, you can also consider simple String concatenation:

File file = new File(rs.getString("FIELD1") + ".csv");

This is a trade-off between one StringBuilder instance, and slight improvement in readability. :)

Instead of calling fw.write() a number of times, you can also consider constructing the output String first. Then, if you happen to be on Java 8, you can consider using Collectors.joining(CharSequence) to join the Stream of output values you need as such:

// using Java 8's optional to slightly simplify the transformation
String clobValue = Optional.ofNullable(rs.getString("FIELD4"))
                            .map(v -> "\"" + v + "\"").orElse(null);
String date = format.format(new Date(rs.getLong("FIELD5")));
String output = Arrays.asList(rs.getString("FIELD2"), rs.getString("FIELD3"), 
                    clobValue, date).stream().collect(Collectors.joining(","));
fw.append(output).newLine();

Finally, I concur with @EricStein's answer that you may want to consider multi-threading if you really need to go up to 11. On this note, you may then want to apply a simple model class for the values from the ResultSet object so that you can handle the multi-threading from a List<FileContent> instead. For example:

public static List<FileContent> map(ResultSet rs) throws SQLException, IOException {
    List<FileContent> result = new ArrayList<>();
    while (rs.next()) {
        result.add(mapRow(rs));
    }
    return result;
}

public static void writeToFiles(List<FileContent> list) {
    // Multi-threading handling here, e.g. using CompletableFutures
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Even if not using Java 7+, all the close statements should be in a finally block. \$\endgroup\$ – lealand Oct 21 '15 at 3:37

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.