Reducing useless synchronizations by using a StringBuilderWriter was easy.

But about using CharSequences instead of Strings, after a quick look, it doesn't look so promising: did you know that Writer.append(CharSequence) does call Writer.write(sequence.toString()), which will itself use string.getChars(), at leats in its default version?!

The most frightening about this code in java.io.Writer is that whenever you pass big strings to it, it will each time allocate big buffers instead of, for instance, copy small portions to output inside a loop. Time needed for memory copy is often negligible, whereas time needed to allocate big chunks of memory is rather annoying. Twice as annoying when done twice.

There is no point in trying to avoid the toString() call in Velocity if the i/o lower layer will call it anyway.

  Claude


On 12/12/2016 18:15, Alex Fedotov wrote:
I don't know if something like this is in scope fro 2.0, but for more
advanced manipulation of AST it would be helpful if creation of all AST
nodes would be delegated to some kind of configurable factory class.

This way if I wanted to replace ASTStringLiteral with a subclass of
ASTStringLiteral for optimization purposes (say use a chunked buffer, or
something similar) I could then setup my own factory and create an instance
of a subclass.

Alex


On Mon, Dec 12, 2016 at 11:52 AM, Claude Brisson <cla...@renegat.net> wrote:

Yes, I also thought of that. But a particular care must be taken in
upstream code that may expect to encounter String values and may otherwise
call toString(), voiding the benefice. Worth looking at, though ; it must
not be too difficult to have this code take CharSequence values into
account.

   Claude



On 12/12/2016 15:58, Alex Fedotov wrote:

I am not sure if it's possible to assume that ASTStringLiteral.value()
returns a CharSequence and not a String. If that was the case you could
just return the StringBuilder directly (StringBuilder implements
CharSequence) without calling StringBuilder.toString(). This would avoid
making one more copy of the data.

Same may apply in other places.

Alex


On Sun, Dec 11, 2016 at 12:28 PM, Claude Brisson <cla...@renegat.net>
wrote:

Thanks. I'll review those StringBuffer uses.
For the ASTStringLiteral, I see there's already a StringBuilderWriter in
commons-io that I may borrow...



On 11/12/2016 17:07, Alex Fedotov wrote:

Great thank you.
Actually I would recommend removing the StringWriter. StringWriter is
using
StringBuffer inside which is synchronized.
In case of ASTStringLiteral there is no need to synchronize anything.
There
must be a Writer implementation based on StringBuilder instead.

I have a run a search on the latest 2.0 code and it seems like there is
a
number of places where StringBuffer is used.
Unless synchronization is required those usages should really be
replaced
with StringBuilder(s).


============================================================
=================

As far our usage of large string literal it was in a legacy framework
that
we had.
It was creating a system of macros to render "tiles" on a page.

Similar to this syntax (not real):

@#PageSection(param1, param2....)
     @#SubSection("tileName1")
        some html
     #end
     @#SubSection("tileName2")
        some other html
     #end
     #SubSection("tileName3", "/blah/blah/section.vtl")
#end

Velocity macros with body allow only one body block (special parameter
really).
We needed multiple named body blocks which we could render in the
correct
spots inside of the macro.

We are no longer using this framework, but that is where we had large
string literals appearing.
I think we replaced the StringBuffer with some unsynchronized chunked
implementation to avoid allocation of very large strings and unnecessary
synchronzation.

Alex





On Sun, Dec 11, 2016 at 7:18 AM, Claude Brisson <cla...@renegat.net>
wrote:

FYI, we got rid of the Executor pattern in the events API, and we now

always provide the current Context when calling handlers.


On 29/11/2016 23:25, Alex Fedotov wrote:
[...]

We have run into some other inefficient places. For example

ASTStringLiteral is buffering the entire content in the StringWriter.
It
does not work very well for large templates.
That code creates a writer which buffers up the whole thing, then
does a
toString() on it creating another copy. Then in some cases calls
substring() that creates yet another copy.

The  substring belonged to an old workaround, it has been removed.

The StringWriter buffering is only done when interpolation is needed
(that
is, when the string literal is enclosed in double quotes and has some $
or
# inside). There could be some tricky ways of introducing some kind of
lazy
evaluation, that would resort to using the provided final writer when
the
value is meant to be rendered or to a buffered writer otherwise. But
I'm
not sure it is worth it, because it looks rather fragile and
convoluted.
Plus, you can generally avoid having big string literals. I don't know
about your use case, but why can't something as:

       #set($foo = "#parse('big.vtl')") $foo

be rewritten as:

       #parse('big.vtl')

to avoid the buffering?


     Claude




---------------------------------------------------------------------
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




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org

Reply via email to