asterisk-java
Actions and Events make heavy use of uncached reflection lookups
Under a heavy volume of asterisk events, there is an observable performance impact stemming from heavy reflection use:
- AbstractBuilder.setAttributes will always look up all setters for a type on every event construction. (18% of event build time)
- Setter names in AbstractBuilder are further transformed (e.g. check for src, clazz), and run through ReflectionUtil.StripIllegalChars on each invocation. This field name transformation result is not cached. (15% of event build time)
- ManagerConnectionImpl will invoke ManagerEvent.toString which by default will always look up all getters for a type on every invocation. (26% of event dispatch time)
Interesting observations.
Event handling overhead is certainly an issue.
We have normally taken the approach of trying to reduce the no. of events emitted.
Are you able to raise a PR with caching?
Need to be cautious of not introducing leaks.
It's probably reasonable to use lazy initialisation of the reflection and then hold out for the life of the app
Need to be cautious of not introducing leaks.
This was actually why I didn't add a PR - sorry about that. Since ReflectionUtils is static, adding a class-keyed static map alongside to cache seems like it'd leave a ClassLoader reference leak that would affect apps that did hot-deploy style restarts in a continuously running app container. It works locally, but it'd likely cause issues for other consumers. Spring's ReflectionUtils for example gets around this by using their own concurrency-safe implementation of a soft reference map, but I didn't want to add additional dependencies.
We have normally taken the approach of trying to reduce the no. of events emitted.
Yes, this probably isn't an issue for most applications - it indeed takes a lot of events to add up. I observed ~750ms total of these reflection operations across a test sequence that builds and dispatches ~1,800 events. Of course that depends heavily on event complexity, field count, server spec, and countless other factors, so I don't know how relevant that number is either.
@cdupl I think having a new dependency in sake of performance is ok not sure what @piotrooo think about this as he is the one that has follow the project, do you have experience on Spring's ReflectionUtils? if yes can you make a PR so we can review this option?
It depends. If you are thinking about add whole spring-core to the project I'm not sure it is a good option. asterisk-java should be a framework agnostic, to take care about our vanilla Java users.
