I don't honestly know much about JDK licensing, so i'll leave that to you
or others. In general, i think the overhead from StringWriter is probably
pretty minimal. Before worrying about it, i'd want to see some profiling
that showed it was a problem. Our dev resources these days are limited and
things are pretty stable, so i'm wary of taking on optimizations that
increase our complexity.


On Mon, Dec 14, 2020 at 8:38 AM Christopher Schultz <
ch...@christopherschultz.net> wrote:

> Nathan,
>
> On 12/14/20 11:15, Nathan Bubna wrote:
> > I'm pretty sure we don't have a setting like that.
>
> Yep, I just got into the code and was writing a patch for it:
>
> L265
>      protected void mergeTemplate(Template template, Context context,
>                                   HttpServletResponse response)
>          throws IOException
>      {
>          //
>          // this section is based on Tim Colson's "two pass render"
>          //
>          // Render the screen content
>          StringWriter sw = new StringWriter();
>          template.merge(context, sw);
>          // Add the resulting content to the context
>          context.put(KEY_SCREEN_CONTENT, sw.toString());
>
>
> I have a LimitedStringWriter class that can be swapped-in for
> StringWriter, but I'm having trouble building the JAR files due to an
> unrelated build issue. I'll post about that separately.
>
> If I'm going to implement a limited StringWriter class, I'm wondering if
> we shouldn't also take the opportunity to improve performance by using a
> StringBuilder internally instead of a StringBuffer (which is what
> java.io.StringWriter uses internally). Since this isn't expected to be
> multi-threaded, the overhead of synchronization on StringWriter's
> StringBuffer can be avoided. That would essentially require writing a
> complete clone of StringWriter, which, while not complicated, might
> possibly violate JDK licensing since I have recently read the code for
> StringWriter in order to build the subclass. :(
>
> I didn't see anything in commons-lang or similar existing dependencies
> that might fit the billing, so I have left it alone for now.
>
> -chris
>
> > On Mon, Dec 14, 2020 at 8:01 AM Christopher Schultz <
> > ch...@christopherschultz.net> wrote:
> >
> >> All,
> >>
> >> We recently suffered an OOME in production because we had a huge SQL
> >> query return a log of data which, in turn, generated even more HTML
> >> output for a particular screen.
> >>
> >> We got an OOME with ASTNode.render trying to call StringWriter.write
> >> (which calls StringBuffer's auto-re-sizing) because the output string
> >> simply grew too much.
> >>
> >> We are of course fixing the SQL query to make sure we limit the amount
> >> of data that can come back, but we'd also like to make sure that
> >> Velocity doesn't trigger an OOME like this if possible.
> >>
> >> Is there a setting for limiting the output buffer size? I haven't yet
> >> found one in the documentation, so I'm starting to look in the code.
> >>
> >> Thanks,
> >> -chris
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
> >> For additional commands, e-mail: dev-h...@velocity.apache.org
> >>
> >>
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
> For additional commands, e-mail: dev-h...@velocity.apache.org
>
>

Reply via email to