Thanks for the details.

I agree that the Context could be passed along with other standard arguments. It has been done this way for backward compatibility, but since we go 2.0, we can improve it. And even keep B.C. by introducing new interfaces while deprecating the old ones.

It's about the fourth or fifth time that I prepare a release candidate, but I think it's worth. Hopefully I'm now pretty used to it...

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

[...]

It seems like there is a some unnecessary work being done initializing
event cartridges, etc. and all of that just for the purpose of setting
RuntimeServices and Context references on them.

What's wrong with setting RuntimeServices at initialization?

As far as creation of executors:
I understand that it was convenient to create the method
"iterateOverEventHandlers" and apply it for event handlers with different
parameters, but the cost of that is creation of the new executor instance
for every invocation.
It would be more efficient to just have a utility that returns a combined
list of handlers (if needed) instead of creating one iterator for each list
(application and context). Then the callback invocation code could just
walk the list and call the handlers.

On the assumption that you have implemented it, did you observe any real performance gain with this change alone? Because once again, since JDK 1.5 or 1.6, the allocation of small objects doesn't cost much more than a function call. For instance, see :
  http://www.ibm.com/developerworks/library/j-jtp09275/
(and yet, this article dates back to 2005...)


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.

Oh, I never noticed that! That must be one of the very few parser node classes I didn't review... I agree it looks like very inefficient. I guess it has be done this way so that the node rendering is all or nothing, in case there's a throw exception. But considering the allocation impact, it cannot stand.

I can dig up a few other things we fixed in our version if you guys are
interested.

We are of course interested. But should you submit any code, you have to also submit a license agreement (see https://www.apache.org/licenses/#submitting ) so that we can use it.



   Claude


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

Reply via email to