Optimize the parsing of large templates

[honglooker]

We were getting performance issues for large templates with nested custom tags and blocks.

In particular, items of the form:

{% block data %}
    {% simple data %}

    <large string>
{% endblock %}

Would parse a lot slower in liqp than shopify's ruby implementation, roughly linearly with the size of the string addendum.

We've noticed that the SLL parser is blazing fast, but liqp falls back to the LL parser a lot.

A case like

    {% block %}
      {% simple %}
    {% endblock %}

will use the LL parser because there isn't a matching end for simple, whereas this works:

   {% block %}
      {% simple %}{% endsimple %}
    {% endblock %}

We augment the parser to work more dynamically -- with more information about the starts/ends of tags and blocks, we can invoke the SLL parser, leading to dramatic performance improvements.

Note that Liqp@HEAD does not differentiate between tags and blocks.

This PR significantly changes the API, since we now differentiate between blocks and tags. However, since the tags and blocks are now passed to the lexer for an optimized parser (SLL), we believe the changes are worth it.

Also added some error handling for mismatched tags and broken templates. Removed a few edges around open ({{) and close (}}) tags.

Closes #170.

[msangel]

Thanks! I have looked in it, then looked again. Looks nice(even if I'm not quite clear what's happening there). Since old tests are not falling and the general idea to differentiate tags and blocks seems good, I`m fine with that. @bkiers probably need your attention anyway here as all I see is rocket science for me...

[msangel]

So my observations: Block extends Tag and even Block implementation seems empty, attempt to use a tag like/instead of a block will cause runtime exception like:

Exception in thread "main" liqp.exceptions.LiquidException: parser error "mismatched input '{%' expecting " on line 2, index 0 in this sample:

Tag.registerTag(new Tag("loop"){
@Override
public Object render(TemplateContext context, LNode... nodes) {
int n = super.asNumber(nodes[0].render(context)).intValue();
LNode block = nodes[1];
StringBuilder builder = new StringBuilder();
while(n-- > 0) {
builder.append(super.asString(block.render(context)));
}
return builder.toString();
}
});

String source = "{% loop 5 %}looping!\n{% endloop %}"; Template template = Template.parse(source); String rendered = template.render(); System.out.println(rendered);

While this will work in sample above: `Tag.registerTag(new Block("loop"){....`
And this is because we do differentiate blocks and tags on parser level, literally in `Template#partitionBlocksAndTags` method:
```java
Tag t = tags.get(name);
if (t instanceof Block) {
    blockNames.add(name);
} else {
    tagNames.add(name);
}

So my question is: if we are about breaking some compatibility and introducing different concepts of tags and blocks, maybe we need to show this difference on a higher level, like:

• make Block and Tag as independent successors of some another abstraction so block will not be a tag anymore.
• made some Block.registerBlock for blocks. And keep Tag.registerTag for tags only. Or at least rename the method, because it looks odd that you cannot register Tag with registerTag just because it is used as block.

This way we will be closer to API and the idea of ruby's implementation where (from original https://github.com/bkiers/Liqp/issues/170):

in ruby you can override the Tag class or the Block class to get a custom tag or block.

Yes, this will break the API even much stronger. But the usage of it will be fair and logical. Any thoughts?

@bkiers @honglooker

[bkiers]

@honglooker thank you for your time and effort!

@bkiers probably need your attention anyway here as all I see is rocket science for me...

Grammar changes look good to me! 👍

in ruby you can override the Tag class or the Block class to get a custom tag or block.

Yes, this will break the API even much stronger. But the usage of it will be fair and logical.

Agreed: if the API is going to break, mind as well do it such a way that it is more in line with Ruby's implementation (so make tags and blocks really 2 different things, and not let a Block extend a Tag).

[bkiers]

Oh, and perhaps also change the example block/tag code in the README

[msangel]

One step at a time. Will prepare interface changes in ongoing commits.

[honglooker]

Thanks all!

[msangel]

Another price for this - we need t know finite list of blocks and tags before parsing, so this sample will no longer works:

    @Test
    public void testCustomTagRegistration() {
        Template template = Template.parse("{% custom_tag %}")
                .with(new Tag("custom_tag") {
                    @Override
                    public Object render(TemplateContext context, LNode... nodes) {
                        return "xxx";
                    }
                });
        assertEquals("xxx", template.render());
    }

and so @honglooker removed the .with at all.

Well. Fixing documentation...

[msangel]

Viable proposal:

@Test
    public void testCustomTagRegistration() {
        Template template = Template.parse("{% custom_tag %}", new ParseSettings.Builder()
                        .with(new Tag("custom_tag") {
                            @Override
                            public Object render(TemplateContext context, LNode... nodes) {
                                return "xxx";
                            }
                        })
                        .build());
        assertEquals("xxx", template.render());
    }

[msangel]

Released as 0.8.3 (without other dependencies upgrade) and 0.8.3.1 (with dependencies upgrade).