Velocity
  1. Velocity
  2. VELOCITY-662

EventHandler.referenceInsert performance bottleneck

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Engine
    • Labels:
      None

      Description

      The times resulting from SimpleNode.literal, StrBuilder.append (the top two) and EventHandlerUtil.referenceInsert are due to the following line in ASTReference.render:

      value = EventHandlerUtil.referenceInsert(rsvc, context, literal(), value);

      Which accounts for nearly 25% of the time, Ouch! Not to mention the many number of StrBuilder objects created. (StrBuilder is called from within literal()) We are always taking the hit for this call even if there is no event handler installed. I havn't looked into it, but will probably just put a conditional around it.

        Activity

        Hide
        Byron Foster added a comment -

        I didn't do as good a job as I wanted. I removed the call to literal(), which was the bulk of the problem. However, ASTReference still calls the referenceInsert handling code each time. I can tell by the comments in EventHandlerUtil.referenceInsert that I'm not the first one to notice the performance problems here.

        There are allot of hoops that have to be jumped through in servicing the event request, unnecessarily. If the event dispatching was any place else it might not be a big deal, but this location is highly performance critical. It gets even worse if a context EventCatridge is installed. Then, every time dispatching takes place the EventCatridge.initialize method gets called which processes all events. I looked at changes I could make, but it seem that it would require changes in the API.

        On a side note, this really isn't an event, but an interceptor. But its structured like an event which makes it less useful, in my opinion. As an interceptor it seems the latest installed handler would be responsible to dispatch to the next in the chain. That way the latest installed handler could decide if calling the next handler was appropriate, or call the next handler and modify its return value, etc... With this setup I believe the code simplifies, and dispatching has less overhead. Probably words for another issue.

        Show
        Byron Foster added a comment - I didn't do as good a job as I wanted. I removed the call to literal(), which was the bulk of the problem. However, ASTReference still calls the referenceInsert handling code each time. I can tell by the comments in EventHandlerUtil.referenceInsert that I'm not the first one to notice the performance problems here. There are allot of hoops that have to be jumped through in servicing the event request, unnecessarily. If the event dispatching was any place else it might not be a big deal, but this location is highly performance critical. It gets even worse if a context EventCatridge is installed. Then, every time dispatching takes place the EventCatridge.initialize method gets called which processes all events. I looked at changes I could make, but it seem that it would require changes in the API. On a side note, this really isn't an event, but an interceptor. But its structured like an event which makes it less useful, in my opinion. As an interceptor it seems the latest installed handler would be responsible to dispatch to the next in the chain. That way the latest installed handler could decide if calling the next handler was appropriate, or call the next handler and modify its return value, etc... With this setup I believe the code simplifies, and dispatching has less overhead. Probably words for another issue.

          People

          • Assignee:
            Unassigned
            Reporter:
            Byron Foster
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development