You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Claude Brisson <cl...@renegat.net> on 2016/12/11 12:18:58 UTC

Re: 2.0 question

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-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: 2.0 question

Posted by Claude Brisson <cl...@renegat.net>.
It is not so hard to differenciate the cases. You just have to look to 
the callstack, and it will exactly give you the parents chain.

If your ASTReference is directly in ASTprocess, or inside an ASTBlock, 
it is being rendered. If your ASTReference is somewhere underneath an 
ASTExpression inside an ASTIfStatement or ASTElseIfStatement, without 
any ASTBlock in between, it is being tested.

   Claude


On 15/12/2016 19:31, Alex Fedotov wrote:
> One more item that comes to mind with event handlers:
>
> In the current interface they do not receive the AST node, just some fields
> from the node.
> In ReferenceInsertionEventHandler it makes very hard to differentiate
> between the insert into the template output as opposed to some string
> literal.
>
> For example:
>
> <div #if("$!blah" != "") attribute="$!blah">
>
> In this case the ReferenceInsertionEventHandler is called two times. Once
> in the context of beining inserted into the ASTStringLiteral "$!blah" and
> the second time when inserted into the template (attribute="$!blah").
> Currently it's difficult to differentiate these cases. If the node itself
> would be passed to the handler one could navigate through the parent chain
> and detect if the insertion is inside of the ASTStringLiteral.
>
>
>
>
>
>
>
> On Wed, Dec 14, 2016 at 4:08 AM, Claude Brisson <cl...@renegat.net> wrote:
>
>> It's the VelocityView from the tools subproject, which uses a pool of
>> VelocityWriters. The engine itself doesn't know which writer is given as
>> argument.
>>
>> Yes, it's *possible* to do it. But it also implies breaking backward
>> compatibility for the o.a.v.io.Filter API, all for the sake of
>> ASTStringLiteral's interpolation performance for big interpolated templates
>> (there is no other concerned case in standard use).
>>
>> So the trade-off is not worth. Plus, even if we try to be as fast as we
>> can with the current code paradigms, the whole engine would have to be
>> rewritten using nio buffers and file mapping to address a real performance
>> gain. This is not the purpose of the 2.0 version.
>>
>>    Claude
>>
>>
>>
>> On 13/12/2016 16:01, Alex Fedotov wrote:
>>
>>> I guess it depends what Writer is used.
>>> StringBuilderWriter.append(CharSequence) is overloaded.
>>>
>>> I thought that Velocity is using a pool of VelocityWriter instances, not
>>> an
>>> OutputStreamWriter. If that is so you can overload  append(CharSequence)
>>> as
>>> needed in VelocityWriter.
>>>
>>> JDK default implementation is very poor (creating potentially two copies
>>> of
>>> data in subSequence and toString).
>>>
>>>       public Writer append(CharSequence csq, int start, int end) throws
>>> IOException {
>>>           CharSequence cs = (csq == null ? "null" : csq);
>>>           write(cs.subSequence(start, end).toString());
>>>           return this;
>>>       }
>>>
>>>
>>> On Tue, Dec 13, 2016 at 8:48 AM, Claude Brisson <cl...@renegat.net>
>>> wrote:
>>>
>>> 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 <cl...@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 <cl...@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 <claude@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-unsubscribe@velocity.apache.org
>>>>>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ------------------------------------------------------------
>>>>>>>> ---------
>>>>>>>>
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>>>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>>>
>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>
>>>>
>>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>> For additional commands, e-mail: dev-help@velocity.apache.org
>>
>>


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


Re: 2.0 question

Posted by Alex Fedotov <al...@kayak.com>.
One more item that comes to mind with event handlers:

In the current interface they do not receive the AST node, just some fields
from the node.
In ReferenceInsertionEventHandler it makes very hard to differentiate
between the insert into the template output as opposed to some string
literal.

For example:

<div #if("$!blah" != "") attribute="$!blah">

In this case the ReferenceInsertionEventHandler is called two times. Once
in the context of beining inserted into the ASTStringLiteral "$!blah" and
the second time when inserted into the template (attribute="$!blah").
Currently it's difficult to differentiate these cases. If the node itself
would be passed to the handler one could navigate through the parent chain
and detect if the insertion is inside of the ASTStringLiteral.







On Wed, Dec 14, 2016 at 4:08 AM, Claude Brisson <cl...@renegat.net> wrote:

> It's the VelocityView from the tools subproject, which uses a pool of
> VelocityWriters. The engine itself doesn't know which writer is given as
> argument.
>
> Yes, it's *possible* to do it. But it also implies breaking backward
> compatibility for the o.a.v.io.Filter API, all for the sake of
> ASTStringLiteral's interpolation performance for big interpolated templates
> (there is no other concerned case in standard use).
>
> So the trade-off is not worth. Plus, even if we try to be as fast as we
> can with the current code paradigms, the whole engine would have to be
> rewritten using nio buffers and file mapping to address a real performance
> gain. This is not the purpose of the 2.0 version.
>
>   Claude
>
>
>
> On 13/12/2016 16:01, Alex Fedotov wrote:
>
>> I guess it depends what Writer is used.
>> StringBuilderWriter.append(CharSequence) is overloaded.
>>
>> I thought that Velocity is using a pool of VelocityWriter instances, not
>> an
>> OutputStreamWriter. If that is so you can overload  append(CharSequence)
>> as
>> needed in VelocityWriter.
>>
>> JDK default implementation is very poor (creating potentially two copies
>> of
>> data in subSequence and toString).
>>
>>      public Writer append(CharSequence csq, int start, int end) throws
>> IOException {
>>          CharSequence cs = (csq == null ? "null" : csq);
>>          write(cs.subSequence(start, end).toString());
>>          return this;
>>      }
>>
>>
>> On Tue, Dec 13, 2016 at 8:48 AM, Claude Brisson <cl...@renegat.net>
>> wrote:
>>
>> 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 <cl...@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 <cl...@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 <claude@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-unsubscribe@velocity.apache.org
>>>>>>>>
>>>>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ------------------------------------------------------------
>>>>>>> ---------
>>>>>>>
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>>
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>
>>>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>

Re: 2.0 question

Posted by Claude Brisson <cl...@renegat.net>.
It's the VelocityView from the tools subproject, which uses a pool of 
VelocityWriters. The engine itself doesn't know which writer is given as 
argument.

Yes, it's *possible* to do it. But it also implies breaking backward 
compatibility for the o.a.v.io.Filter API, all for the sake of 
ASTStringLiteral's interpolation performance for big interpolated 
templates (there is no other concerned case in standard use).

So the trade-off is not worth. Plus, even if we try to be as fast as we 
can with the current code paradigms, the whole engine would have to be 
rewritten using nio buffers and file mapping to address a real 
performance gain. This is not the purpose of the 2.0 version.

   Claude


On 13/12/2016 16:01, Alex Fedotov wrote:
> I guess it depends what Writer is used.
> StringBuilderWriter.append(CharSequence) is overloaded.
>
> I thought that Velocity is using a pool of VelocityWriter instances, not an
> OutputStreamWriter. If that is so you can overload  append(CharSequence) as
> needed in VelocityWriter.
>
> JDK default implementation is very poor (creating potentially two copies of
> data in subSequence and toString).
>
>      public Writer append(CharSequence csq, int start, int end) throws
> IOException {
>          CharSequence cs = (csq == null ? "null" : csq);
>          write(cs.subSequence(start, end).toString());
>          return this;
>      }
>
>
> On Tue, Dec 13, 2016 at 8:48 AM, Claude Brisson <cl...@renegat.net> wrote:
>
>> 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 <cl...@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 <cl...@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 <cl...@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-unsubscribe@velocity.apache.org
>>>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>>>
>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>
>>>>
>>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>> For additional commands, e-mail: dev-help@velocity.apache.org
>>
>>


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


Re: 2.0 question

Posted by Alex Fedotov <al...@kayak.com>.
I guess it depends what Writer is used.
StringBuilderWriter.append(CharSequence) is overloaded.

I thought that Velocity is using a pool of VelocityWriter instances, not an
OutputStreamWriter. If that is so you can overload  append(CharSequence) as
needed in VelocityWriter.

JDK default implementation is very poor (creating potentially two copies of
data in subSequence and toString).

    public Writer append(CharSequence csq, int start, int end) throws
IOException {
        CharSequence cs = (csq == null ? "null" : csq);
        write(cs.subSequence(start, end).toString());
        return this;
    }


On Tue, Dec 13, 2016 at 8:48 AM, Claude Brisson <cl...@renegat.net> wrote:

> 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 <cl...@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 <cl...@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 <cl...@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-unsubscribe@velocity.apache.org
>>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>>
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>
>>>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>

Re: 2.0 question

Posted by Claude Brisson <cl...@renegat.net>.
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 <cl...@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 <cl...@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 <cl...@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-unsubscribe@velocity.apache.org
>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>
>>>>
>>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>> For additional commands, e-mail: dev-help@velocity.apache.org
>>
>>


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


Re: 2.0 question

Posted by Alex Fedotov <al...@kayak.com>.
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 <cl...@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 <cl...@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 <cl...@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-unsubscribe@velocity.apache.org
>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>
>>>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>

Re: 2.0 question

Posted by Claude Brisson <cl...@renegat.net>.
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 <cl...@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 <cl...@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-unsubscribe@velocity.apache.org
>> For additional commands, e-mail: dev-help@velocity.apache.org
>>
>>


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


Re: 2.0 question

Posted by Alex Fedotov <al...@kayak.com>.
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 <cl...@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 <cl...@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-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>

Re: 2.0 question

Posted by Claude Brisson <cl...@renegat.net>.
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 <cl...@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-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: 2.0 question

Posted by Alex Fedotov <al...@kayak.com>.
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 <cl...@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
>
>
>