There are some open questions though:

There is a very useful method Template.merge(Context, Writer, List
macroLibraries).
Should these macroLibraries be treated as global or local? I believe they
are treated as local now.

Alex


On Fri, Mar 6, 2015 at 12:08 PM, Alex Fedotov <a...@kayak.com> wrote:

> Sure it can be done.
>
> I would still keep the macros in templates, including the global templates
> (i.e. library files). I would maintain the list of global Template objects.
> Depending on the settings, during the macro lookup iterate over the global
> Template objects first, if not found there do the normal lookup in the
> calling template or in the Template list (macroLibraries).
>
> Alex
>
>
> On Fri, Mar 6, 2015 at 11:54 AM, Nathan Bubna <nbu...@gmail.com> wrote:
>
>> inline...
>>
>> On Fri, Mar 6, 2015 at 7:48 AM, Alex Fedotov <a...@kayak.com> wrote:
>>
>> > One more question. One of the other areas we making changes is around
>> how
>> > macros are called. Right now the lookup process is fairly rigid and can
>> be
>> > inefficient as well. Consider a couple of examples:
>> >
>> > 1. Scenario one - overload macro in the template This works now because
>> the
>> > lookup checks the namespace where the call is made first.
>> >
>> > macros.vtl
>> > ========
>> > #macro(B)
>> > #end
>> >
>> > #macro(A)
>> >     #B()
>> > #end
>> > ========
>> >
>> > template.vtl
>> > =========
>> > #parse('macros.vtl')
>> > #macro(A)
>> >    ## overloaded macro A
>> > #end
>> > #A() ## this calls local macro A which is expected.
>> > =========
>> >
>> > 2. Scenario two - overload macro B - this does not work now.
>> >
>> > template.vtl
>> > =========
>> > #parse('macros.vtl')
>> > #macro(B)
>> >    ## overloaded macro B
>> > #end
>> > #A() ## this calls macro A which is then does NOT call overloaded B,
>> since
>> > the look-up would prefer the B from the same module where A is.
>> > =========
>> >
>> > We have created a custom directive #call which is similar to the
>> > RuntimeMacro, but allows more control over the lookup process. This also
>> > allows stuff like:
>> > #call('self::macroName')
>> > #call('nameSpace::macroName')
>> >
>> > The way every #parse(d) template is appended to the list of the
>> > "macroLibraries" can be very bad for macro look-up speed:
>> > #foreach( $item in $items )
>> >     #parse('item.vtl')
>> > #end
>> >
>> > You end-up with a lot of "item.vtl" references added to the list which
>> then
>> > cause a lot of duplicate macro lookups in namespaces.
>> > We have fixes that avoid adding duplicate sequences of template names to
>> > the macroLibraries list.
>> >
>> > Do you guys have any plans to address this area?
>> >
>>
>> So far as i know, no one has plans to work on these things.
>>
>>
>> > We are also inclined to always use namespaces for macros, always allow
>> > local macros to replace globals and keep the macros as
>> ConcurrentHashMap in
>> > the Template object.
>> >
>>
>> Keeping macros in the Template would be good, but i'd be hesitant to
>> accept
>> a patch that permanently allows local macros to replace global macros.
>> There are a LOT of Velocity users out there who use it to allow
>> third-parties to write templates. Those Velocity users tend to like the
>> ability to limit what their untrusted users can do with templates.
>>
>>
>> > This way VelocimacroManager can be dropped all together, there is no
>> issue
>> > with reloading templates while maintaining the VelocimacroManager in
>> sync,
>> > less synchronization problems, etc.
>> > The flag "isFromLib" for macros would also not be needed, so all the
>> hair
>> > around maintaining it during the "init" phase could be removed as well.
>> >
>>
>> That sounds good. Can you do that while keeping the option to block local
>> macros from overriding global ones?
>>
>>
>>
>> >
>> > Alex
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > On Fri, Mar 6, 2015 at 2:48 AM, Claude Brisson <cla...@renegat.net>
>> wrote:
>> >
>> > > Hi.
>> > >
>> > > We'll publish the 2.0 one day, it's slowly moving...
>> > >
>> > > Thank you for your proposal. Of course, it is interesting for the
>> project
>> > > if you can share your 1.7 bugfixes as long as your synchronization
>> work
>> > on
>> > > the 2.0 branch.
>> > >
>> > > The process is rather easy: patches are to be submitted on JIRA :
>> > > https://issues.apache.org/jira/browse/VELOCITY/
>> > >
>> > > Very small patches (too small to be considered intellectual property)
>> > > don't need anything else. For more consequent patches, some people say
>> > you
>> > > should at least add a sentence like "feel free to use this patch under
>> > the
>> > > Apache License" in the issue.
>> > >
>> > >
>> > >   Claude
>> > >
>> > >
>> > >
>> > > On 04/03/2015 17:48, Alex Fedotov wrote:
>> > >
>> > >> Hello Guys,
>> > >>
>> > >> We use Velocity quite a bit and are considering moving forward and
>> > >> deploying the trunk build in production even though there never was a
>> > 2.0
>> > >> release. We currently use a custom 1.7 build with a couple of bugs
>> fixed
>> > >> by
>> > >> us.
>> > >>
>> > >> One area where we are going to make some changes before it can go in
>> > >> production is the synchronization in the VelocimacroFactory and
>> > >> VelicimacroManager classes. It appears to be a mix of
>> ConcurrentHashMaps
>> > >> and global synchronized blocks. Our code is highly threaded, loads
>> > quite a
>> > >> bit of templates dynamically at run-time and having these global
>> locks
>> > >> would likely present a synchronization hotspot.
>> > >>
>> > >> Our plan is to see if we can cleanup the synchronization in this area
>> > keep
>> > >> it robust, but remove the global synchronized blocks.
>> > >>
>> > >> If you guys have any interest in accepting our contribution back
>> please
>> > >> let
>> > >> me know what the process should be.
>> > >>
>> > >> Thank you,
>> > >> Alex
>> > >>
>> > >>
>> > >
>> >
>>
>
>

Reply via email to