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

Reply via email to