Migrated existing code from reflection and it runs twice as slow

Question

I've been given a piece of code and asked to make it "not use reflection" because it's too slow. The code currently uses reflection to get the values of the 205 fields, append it all to a StringBuilder and return it. It looks something like this:

public String reflection()
    {
        StringBuilder sb = new StringBuilder();

        sb.append("ExecutionEvent(");

        // This will be very slow. We need to hard code the string
        // encoding for this event type
        // For now I am using reflection to quickly flesh out the framework
        // code.
        // TODO: fix this to not use reflection
        String delim = "";
        for (Field f : this.getClass().getFields())
        {
            try
            {
                Object o = f.get(this);

                if (f.getType() == String.class)
                {
                    // We wrap values with double quotes so cannot contain a double quote within a value.
                    // Change to single quote.
                    String value = o == null ? "" : o.toString().replace("\"", "'");

                    // value = value.replace("\'", "\\'");
                    // value = value.replaceAll("\\\\", "");
                    sb.append(delim);
                    sb.append("\"");
                    sb.append(value);
                    sb.append("\"");
                }
                else
                {
                    sb.append(delim);
                    String value = o.toString();
                    sb.append(value);
                }

                delim = ",";
            }
            catch (Exception ex)
            {
                logger.error("Cannot get value for field. {}", ex.getMessage());
            }

        }

        if (sb.toString().contains("\\"))
        {
            sb.replace(sb.indexOf("\\"), sb.indexOf("\\") + 1, "");
        }

        sb.append(")");

        return sb.toString();
    }

I've changed this code to this:

    public String noReflection() {

        StringBuilder sb = new StringBuilder();
        sb.append("ExecutionEvent(");

        String delim = "";
        for (Object o : getFields()) {

            if(o instanceof String || o == null){
                o = o == null ? "" : ((String) o).replace("\"", "");
                sb.append(String.format("%s\"%s\"", delim, o));
            }
            else
                sb.append(String.format("%s%s", delim, o.toString()));

            delim = ",";
        }

        if (sb.toString().contains("\\")) {
            sb.replace(sb.indexOf("\\"), sb.indexOf("\\") + 1, "");
        }

        sb.append(")");


        return sb.toString();
    }

The getFields() method in the for loop refers to another method I created:

private List<Object> getFields(){

        List<Object> values = new ArrayList<Object>();

        values.add(announcementDate);
        //Another ~200 values.add(fieldName);
        values.add(whenIssuedIndicator);

        return values;
    }

I've ran both methods side by side and they are the only things that I change. I've used the benchmark of the existing code to measure the speed:

try ( ResultSet resultSet = stmt.executeQuery(sqls.get(8));) {
                        StopWatch timer = new StopWatch();
                        timer.start();
                        while (resultSet.next()) {

                            processRow(resultSet, runDate);
                            count++;

                            if (count % 100000 == 0) {
                                logger.info(feedName+ " | "+ new Object() {}.getClass().getEnclosingMethod().getName()+ " | OceanOrderFeed processed {} orders",
                                decimalFormat.format(count));                                                               
                            }
                        }
                        timer.stop();

                        int hours = (int) (timer.getTotalTimeSeconds() / 3600);
                        int minutes = (int) ((timer.getTotalTimeSeconds() - (hours * 3600)) / 60);
                        int seconds = (int) ((timer.getTotalTimeSeconds() - (hours * 3600) - (minutes * 60)));

                        logger.info(
                                feedName+ " | "+ new Object() {}.getClass().getEnclosingMethod().getName()
                                        + " | OceanOrderFeed completed. Processed {} orders in {} Hours : {} Minutes : {} Seconds",
                                decimalFormat.format(count), hours, minutes, seconds);
                    }

The benchmark gives me the following results:

No reflection
Processed 1,000,000 orders in 0 Hours : 8 Minutes : 49 Seconds
Processed 1,000,000 orders in 0 Hours : 9 Minutes : 3 Seconds
Processed 1,000,000 orders in 0 Hours : 10 Minutes : 11 Seconds

Reflection
Processed 1,000,000 orders in 0 Hours : 4 Minutes : 46 Seconds
Processed 1,000,000 orders in 0 Hours : 4 Minutes : 27 Seconds
Processed 1,000,000 orders in 0 Hours : 4 Minutes : 34 Seconds

Using reflection is faster across the board. Is there a problem in the code I have posted that's causing something that should be faster to be slower?

See [tips for writing micro-benchmarks](http://stackoverflow.com/a/513259/1089967) — lance-java, Oct 19 '16 at 13:16
Just a guess: try to set a capacity for `ArrayList` or to replace it with an array. e.g. `List — default locale, Oct 19 '16 at 13:17
This might be irrelevant but I do not like the way you are measuring time. It probably will not make a difference now, but in the future you should only use `getTotalTimeSeconds()` once per time check. — nick zoum, Oct 19 '16 at 13:27
*to make it "not use reflection" because it's too slow* Why was reflection assumed to be why the code is slow? Have you benchmarked and/or profiled the application to actually find out where it's slow? — Andrew Henle, Oct 19 '16 at 15:55
It wasn't that the code was slow, it was that reflection is slow generally speaking. — Nanor, Oct 19 '16 at 15:58

score 0 · Accepted Answer · edited May 23 '17 at 12:10

The issue was that I was using String.format() in my loops when I should have been using StringBuilder.append() as was in the original code.

The reason is that in the current implementation String.format first parses the input with regular expressions and then fills in the parameters. Concatenation with plus, on the other hand, gets optimized by javac (not by the JIT) and uses StringBuilder.append directly.

Migrated existing code from reflection and it runs twice as slow

1 Answers1