11
\$\begingroup\$

While reviewing Sending templatized e-mail to a million contacts, I wrote this implementation to illustrate an alternate approach. It is designed to be the fastest possible way to generate templated text repeatedly. Is it?

I used the in-memory Java compiler featured in this Stack Overflow answer.

I think that the stringLiteral() function and the try-catch block that performs the compilation are rather ugly.

Template.java

import java.io.IOException;
import java.io.Writer;
import java.util.Map;

public interface Template {
    public void write(Writer out, Map<String, String> params) throws IOException;
}

TemplateCompiler.java

import java.io.*;
import java.lang.reflect.Constructor;
import java.util.HashMap;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.tools.*;
import org.mdkt.compiler.InMemoryJavaCompiler;

public class TemplateCompiler {
    private static final Pattern SUBST_PAT = Pattern.compile(
        "(?<LITERAL>.*?)(?:\\{\\{(?<SUBST>[^}]*)\\}\\})"
    );

    /**
     * Instantiates a <code>Template</code> that performs simple
     * text substitutions for <code>{{PLACEHOLDERS}}</code>.
     */
    public static Template compile(String templateText) {
        int rest = 0;
        StringBuilder script = new StringBuilder(
            "import java.io.IOException;\n" +
            "import java.io.Writer;\n" +
            "import java.util.Map;\n" +
            "public class C implements Template {\n" +
            "    public void write(Writer out, Map<String, String> params) throws IOException {\n"
        );
        for (Matcher m = SUBST_PAT.matcher(templateText); m.find(); rest = m.end()) {
            script.append("out.write(")
                  .append(stringLiteral(m.group("LITERAL")))
                  .append(");\nout.write(params.get(")
                  .append(stringLiteral(m.group("SUBST")))
                  .append("));\n");
        }
        script.append("out.write(")
              .append(stringLiteral(templateText.substring(rest)))
              .append(");\n");

        script.append("out.flush();\n")
              .append("}}");

        try {
            @SuppressWarnings("unchecked")
            Class <? extends Template> c = (Class <? extends Template>)InMemoryJavaCompiler.compile("C", script.toString());
            Constructor<? extends Template> ctr = c.getConstructor();
            return ctr.newInstance();
        } catch (Exception e) {
            e.printStackTrace();
            return null;
        }
    }

    private static final Pattern UNSAFE_CHARS = Pattern.compile("[^A-Za-z0-9 ]");

    private static String stringLiteral(String s) {
        StringBuffer result = new StringBuffer("\"");
        Matcher matcher = UNSAFE_CHARS.matcher(s);
        while (matcher.find()) {
            char c = matcher.group().charAt(0);
            switch (c) {
              // JLS SE7 3.10.5: 
              // It is a compile-time error for a line terminator to appear
              case '\r':
                matcher.appendReplacement(result, "\\r");
                break;
              case '\n':
                matcher.appendReplacement(result, "\\n");
                break;
              default:
                String.format("\\\\u%04x", (int)c);
            }
        }
        matcher.appendTail(result);
        result.append("\"");
        return result.toString();
    }
}

Sample usage

Template t = TemplateCompiler.compile(
    "Dear {{USER_NAME}},\n\n" +
    "According to our records, your phone number is {{USER_PHONE}} and " +
    "your e-mail address is {{USER_EMAIL}}.  If this is incorrect, please " +
    "go to {{LOGIN_URL}} and update your contact information."
);
for (Contact c : contacts) {
    Map<String, String> params = new HashMap<>();
    params.put("USER_NAME", c.getUserName());
    params.put("USER_EMAIL", c.getEmail());
    params.put("USER_PHONE", c.getPhone());
    params.put("LOGIN_URL", c.getLoginUrl());
    StringWriter sw = new StringWriter();
    t.write(sw, params);
    sw.toString();
}
\$\endgroup\$
  • \$\begingroup\$ Just a comment because it is not part of your code: One thing about templatized E-mails that I see fairly often is that some replacement marker was not replaced appropriately. Maybe you should add a check if all {{}} have been set/replaced. One step further would be to test whether there are some malformed instances like {{}. On an easier level this could be checked with a (unit)test dataset against your expectation which could even have caught the errors @rofl reported. \$\endgroup\$ – Nobody Aug 31 '15 at 12:41
8
\$\begingroup\$

The compile concept produced incorrect results for me. When I run your code the template does not produce the correct results. For the input parameters:

final Map<String,String> parms = new HashMap<>();
Stream.of("USER_NAME", "USER_PHONE", "USER_EMAIL", "LOGIN_URL")
      .forEach(tag -> parms.put(tag, tag));

I would expect the input String:

"Dear {{USER_NAME}},\n\n" +
"According to our records, your phone number is {{USER_PHONE}} and " +
"your e-mail address is {{USER_EMAIL}}.  If this is incorrect, please " +
"go to {{LOGIN_URL}} and update your contact information."

to produce:

Dear USER_NAME,

According to our records, your phone number is USER_PHONE and your e-mail address is USER_EMAIL.  If this is incorrect, please go to LOGIN_URL and update your contact information.

But, instead, it produces:

Dear USER_NAMEAccording to our records, your phone number is USER_PHONE and your e-mail address is USER_EMAIL.  If this is incorrect, please go to LOGIN_URL and update your contact information.

I have looked through the code, and I am not sure why it is dropping the newlines, and the comma-punctuation after "USER_NAME".

I looked through the TemplateCompile code, and while I like that you use a Pattern/Matcher to parse the template, the actual loop structure is really complicated. You shoe-horn the process in to a for-loop, when a while-loop would be much better. Additionally, you use a complicated double-matching named-group regular expression, when a single-matching one would be more than adequate.

I particularly dislike the rest variable, and how it is used.

I wonder if this complicated regex logic is the cause of the broken output?

I wrote a "competing" code block, and I also chose regex to parse the template, but my loop is very different:

private static final Pattern token = Pattern.compile("\\{\\{(\\w+)\\}\\}");

public static Template compile(String text) {

    Matcher mat = token.matcher(text);

    int last = 0;

    while (mat.find()) {
        // the non-token text is from the last match end,
        // to this match start
        final String constant = text.substring(last, mat.start());
        // this token's key is the regex group
        final String key = mat.group(1);

        // do stuff with the text and subsequent token
        ....

        last = mat.end();
    }
    final String tail = text.substring(last);
    if (!tail.isEmpty()) {
        // do something with trailing text after last token.
        ....
    }

}

A while loop on the Matcher.find() result is the natural loop constraint.

Instead of compiling the code down, I used an array of text injectors to perform the write. Some injectors inject a constant value, others inject a lookup value from the Parameters. I was able to reduce your class down to much simpler constructs, with no code abstraction and compilation, etc. From a readability and maintenance perspective I believe it is clearly better:

public class MonkeyFix implements Template {

    @FunctionalInterface
    private interface Injector {
        String get(Map<String,String> params);
    }

    private static final Pattern token = Pattern.compile("\\{\\{(\\w+)\\}\\}");

    public static Template compile(final String text) {
        final Matcher mat = token.matcher(text);
        final List<Injector> sequence = new ArrayList<>();
        int last = 0;

        while (mat.find()) {
            final String constant = text.substring(last, mat.start());
            final String key = mat.group(1);

            sequence.add(params -> constant);
            sequence.add(params -> params.get(key));

            last = mat.end();
        }

        final String tail = text.substring(last);
        if (!tail.isEmpty()) {
            sequence.add(params -> tail);
        }

        return new MonkeyFix(sequence.toArray(new Injector[sequence.size()]));
    }

    private final Injector[] sequence;

    public MonkeyFix(Injector[] sequence) {
        this.sequence = sequence;
    }

    @Override
    public void write(Writer out, Map<String, String> params) throws IOException {
        for (Injector lu : sequence) {
            out.write(lu.get(params));
        }
    }

}

How about the performance, though?

I pout the code through my MicroBench suite, using the following code (I had to use a different validation string for your code, I called that one wrong ... ;-) :

public class TemplateMain {

    private static final String text = 
            "Dear {{USER_NAME}},\n\n" +
            "According to our records, your phone number is {{USER_PHONE}} and " +
            "your e-mail address is {{USER_EMAIL}}.  If this is incorrect, please " +
            "go to {{LOGIN_URL}} and update your contact information.";

    private static final Template inmemcomp = TemplateCompiler.compile(text);

    private static final Template monkeyfix = MonkeyFix.compile(text);

    private static final String inMemFunc(Template t, Map<String, String> params) {
        StringWriter sw = new StringWriter();
        try {
            t.write(sw, params);
        } catch (IOException e) {
            e.printStackTrace();
        }
        return sw.toString();
    }

    public static void main(String[] args) {
        UUtils.setStandaloneLogging(Level.INFO);
        UBench bench = new UBench("Templating");
        final String expect = "Dear USER_NAME,\n\n" +
                "According to our records, your phone number is USER_PHONE and " +
                "your e-mail address is USER_EMAIL.  If this is incorrect, please " +
                "go to LOGIN_URL and update your contact information.";
        final String wrong = "Dear USER_NAMEAccording to our records, your phone number is USER_PHONE and your e-mail address is USER_EMAIL.  If this is incorrect, please go to LOGIN_URL and update your contact information.";

        System.out.println(expect);
        System.out.println(wrong);

        final Map<String,String> parms = new HashMap<>();
        Stream.of("USER_NAME", "USER_PHONE", "USER_EMAIL", "LOGIN_URL").forEach(tag -> parms.put(tag, tag));

        bench.addTask("InMemCompile", () -> inMemFunc(inmemcomp, parms), got -> wrong.equals(got));
        bench.addTask("MonkeyFix", () -> inMemFunc(monkeyfix, parms), got -> expect.equals(got));

        bench.press(10000).report();

    }
}

The results are inconclusive on my computer, sometimes your code wins, sometimes mine does. Regardless, they are both "fast enough", and the differences are marginal.

\$\endgroup\$
3
\$\begingroup\$

@rolfl has uncovered some embarrassing bugs in the output.

Parts of the literal strings were being dropped due to a missing Pattern.DOTALL flag:

private static final Pattern SUBST_PAT = Pattern.compile(
    "(?<LITERAL>.*?)(?:\\{\\{(?<SUBST>[^}]*)\\}\\})", Pattern.DOTALL
);

In stringLiteral(), all three cases were wrong:

        switch (c) {
          // JLS SE7 3.10.5: 
          // It is a compile-time error for a line terminator to appear
          case '\r':
            matcher.appendReplacement(result, "\\\\r");
            break;
          case '\n':
            matcher.appendReplacement(result, "\\\\n");
            break;
          default:
            matcher.appendReplacement(result, String.format("\\\\u%04x", (int)c));
        }
\$\endgroup\$
2
\$\begingroup\$
StringBuilder script = new StringBuilder(
    "import java.io.IOException;\n" +
    "import java.io.Writer;\n" +
    "import java.util.Map;\n" +
    "public class C implements Template {\n" +
    "    public void write(Writer out, Map<String, String> params) throws IOException {\n"
);

Perhaps skip the String concatenation and use the append() methods too?

// 1024 size is an arbitrary choice, 
// the code below + example template stands at under 400 chars already
// each substitution will use an additional 60 bytes or so... 
// multiply accordingly for 4 fields, 
// and finally setting aside more spaces for the actual replacements
StringBuilder script = new StringBuilder(1024);
script.append("import java.io.IOException;")
    .append("import java.io.Writer;")
    .append("import java.util.Map;")
    .append("public class C implements Template{")
    .append("public void write(Writer out,Map<String, String> params) throws IOException{");

Also,

result.append('"');
// instead of
result.append("\"");

Have you actually tested the speed of this, using something like say jmh? I'll also be interested to know how this compares with something written and compiled with Nashorn (on Java 8)...

\$\endgroup\$
  • 1
    \$\begingroup\$ .append() should be worse than +, since concatenation using + happens at compile time (Java compile time, not template compile time), whereas .append() happens at runtime. \$\endgroup\$ – 200_success Aug 31 '15 at 4:12
  • 2
    \$\begingroup\$ JavaScript using Nashorn was much much worse than Java, by orders of magnitude. \$\endgroup\$ – 200_success Aug 31 '15 at 4:14
  • \$\begingroup\$ @200_success ah ok... \$\endgroup\$ – h.j.k. Aug 31 '15 at 4:15

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.