Re: FreeMarker, ANTLR & New FreeMarker Template Notations [was: JetBrains / IntelliJ IDEA v8 to Support FreeMarker!]

On Thu, Apr 3, 2008 at 9:12 AM, Attila Szegedi <[email protected]> wrote:
>
>  On 2008.04.03., at 2:09, Randall R Schulz wrote:
>  > On Wednesday 02 April 2008 15:41, Jonathan Revusky wrote:
>  >>
>
> >> JavaCC pulls
>  >> tokens off the token source on an as-needed basis. In JavaCC you can
>  >> even do something like token_source.setState(...) from within the
>  >> expression parser.
>  >
>  > I consider that a far more defensible design, personally.
>
>  Although in the particular case of the FreeMarker grammar, we ended up
>  with a generated parser that'll pretokenize the whole input file,

Well, that issue doesn't have much to do with this, because this is
about a case where of  a 5 megabyte file (no FM instructions in there
even) that's just getting puled in as plain text tokens, whitespace
and printable characters and so on. A sequence of plain text is just
getting continually appended to a StringBuffer and then all 5 megs is
put in a single TextBlock object and that's the whole parsed template,
a single plain text node. It's running out of memory as it slurps in
the tokens and appends them to the StringBuffer. Probably the main
problem is the llinked list of Token objects that JavaCC is
maintaining internally. And that is what is eating up all the RAM.  It
isn't lookahead per se, because it builds this linked list of Token
objects as it reads in tokens on an as-needed basis. And then it does
end up with that whole linked list of tokens sitting in memory. I
guess that, since there is no real need for all the tokens.  At least,
not the ones in the middle. The first token and the last one are
needed for location start/end location info.

Actually, come to think of it, I think I see now what the issue is.
TextBlock PCData() :
{
    StringBuffer buf = new StringBuffer();
    Token t, start=null;
}
{
    (
      LOOKAHEAD(<WHITESPACE>|<PRINTABLE_CHARS>|<FALSE_ALERT>)
      (
         t=<WHITESPACE>
             |
         t=<PRINTABLE_CHARS>
         |
         t=<FALSE_ALERT>
      )
      {
         buf.append(t.image);
         if (start == null) start = t;
      }
    )+
    {
         if (stripText && contentNesting == 1)
             return TextBlock.EMPTY_BLOCK;

         TextBlock result = new TextBlock(buf.toString(), false);
         result.setLocation(template, start, t);
         return result;
    }
}

The basic problem is that we are hanging on to a reference to the
start token (the reason is to get location info). So, all we need to
do is set start.next to null and then all the chained token.next and
token.next.next becom candidates for garbage collection and the memory
problem should disappear. Of course, I suppose it would help the
garbage collector a bit (possibly) to just set token.next to null
since we don't need it.

So if we write:


TextBlock PCData() :
{
    StringBuilder buf = new StringBuilder();
    Token t=null, start=null, prevToken = null;
}
{
    (
      LOOKAHEAD(<WHITESPACE>|<PRINTABLE_CHARS>|<FALSE_ALERT>)
      (
         {prevToken = t;}
         t=<WHITESPACE>
         |
         t=<PRINTABLE_CHARS>
         |
         t=<FALSE_ALERT>
      )
      {
         buf.append(t.image);
         if (start == null) start = t;
         {if (prevToken != null) prevToken.next = null;} // It's
actually just crucial to do it in the starting token, because then
it's all garbage collectable.
      }
    )+
    {
         if (stripText && contentNesting == 1)
             return TextBlock.EMPTY_BLOCK;
         TextBlock result = new TextBlock(buf.toString(), false);
         result.setLocation(template, start, t);
         return result;
    }
}

That should do it. I haven't tested this though. I know it's easy, but
I didn't get any sleep last night...


>  thanks to certain lookaheads; see <http://sourceforge.net/tracker/index.php?func=detail&aid=1851842&group_id=794&atid=100794
>   >. I have no idea whether we can fix this, or is it now an inherent
>  consequence of our grammar...

Nah, I think it's just the above, that the method is hanging on to
that first token in the chain and it doesn't really need it, except
for the location info, but that means it's hanging onto the entire
chain, in this case, like literally a million tokens. I mean, in a
normal template file, this memory usage bug would never show up. You
need megabytes of uninterrupted plain text in there. Still, it's
possible in some usages, I guess. It's a pretty major bug fix.

Anyway, I am pretty sure this fixes it and it really has nothing to do
with our overall grammar or lookaheads or the tricky business of
chanigng the the lexer state from the parser.

Strange, when this was reported, I looked at it and could not fathom
why a 5 meg text file needed 192 megs to be parsed.  Now I see why,
when I haven't slept for nearly 24 hours. I don't understand why I
didn't see it before. The problem is that it's not an array with
contiguous text. It's like a linked list chain of a million tokens
that are being retained in memory. So, if the memory overhead of each
Token object in the chain is about 190 bytes or so (which still seems
a bit excessive, but within the bounds of the believable) then that
explains it. Mystery resolved.

JR


>
>  Attila.
>
>
>
>
>  -------------------------------------------------------------------------
>  Check out the new SourceForge.net Marketplace.
>  It's the best place to buy or sell services for
>  just about anything Open Source.
>  http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
>  _______________________________________________
>  FreeMarker-user mailing list
>  [email protected]
>  https://lists.sourceforge.net/lists/listinfo/freemarker-user
>

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace