I remembered one more thing: I think we wanted to include template call stack and macro call stack into the MethodInvocationException message. There was no easy way to do it without installing a ContextAware MethodExceptionEventHandler because the stack info is only available from the Context.
I guess this would be covered by passing the Context to all event handlers. Alex On Wed, Nov 30, 2016 at 11:26 AM, Alex Fedotov <a...@kayak.com> wrote: > As far as event handlers and passing the RuntimeServices in > initialization. There is no problem with that, except that the interface is > optional. You end-up with the code like EventCartridge.initialize. > > May be the event handler interfaces like ReferenceInsertionEventHandler > should just derive from RuntimeServicesAware. This way you can just call it > unconditionally at creation time without any "instanceof" checks. > In Java 8 "setRuntimeServices" could be a default method on the interface > with a NOOP implementation. > > Alex > > > On Wed, Nov 30, 2016 at 4:56 AM, Claude Brisson <cla...@renegat.net> > wrote: > >> 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/license >> s/#submitting ) so that we can use it. >> >> >> >> Claude >> >> >
