Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise ResolvableType.as for introspection performance (limiting serializability) [SPR-17070] #21608

Closed
spring-projects-issues opened this issue Jul 20, 2018 · 5 comments
Assignees
Labels
in: core type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jul 20, 2018

Dave Syer opened SPR-17070 and commented

After #21508? we now have some caching in GenericApplicationListenerAdapter which helps a bit. Unfortunately I still see the app losing 2.5% of startup time to GenericApplicationListenerAdapter while the multicaster checks each listener for supported event types. The culprit is ResolvableType.as() (where it looks for all interfaces).

I tried this and it works and is much faster:

@Nullable
static ResolvableType resolveDeclaredEventType(Class<?> listenerType) {
     ResolvableType eventType = eventTypeCache.get(listenerType);
     if (eventType == null) {
          Method method = null;
          for (Method candidate : ReflectionUtils.getUniqueDeclaredMethods(listenerType)) {
               if (candidate.getName().equals("onApplicationEvent")) {
                        method = candidate;
                        break;
               }
          }
          if (method !=null) {
               eventType = ResolvableType.forMethodParameter(method, 0);
          } else {
               eventType = ResolvableType.forClass(listenerType).as(ApplicationListener.class).getGeneric();
          }
          eventTypeCache.put(listenerType, eventType);
     }
     return (eventType != ResolvableType.NONE ? eventType : null);
}

Whether or not that is general enough is beyond me, but it has a big effect on the tiny apps I have been benchmarking.


Affects: 5.1 RC1

Reference URL: #21508

Issue Links:

  • #21508 Resolved ApplicationListener event type should get cached
  • #21609 Avoid repeated factory class introspection in AbstractAutowireCapableBeanFactory

Referenced from: commits cfbacfd

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 20, 2018

Juergen Hoeller commented

I'm rather keen on optimizing ResolvableType.as here. There is no need for that interface initialization to be so expensive... and we're using the same generic resolution code in other places as well.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 20, 2018

Dave Syer commented

Indeed that would have benefits in many other places. Good luck!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 20, 2018

Andy Clement commented

Just my 2c as I see the code above - I was exploring other uses of getUniqueDeclaredMethods() yesterday. I found a few places ask for them all and then immediately ignoring most of them. So I was fiddling with creating a variant of getUniqueDeclaredMethods() that took a methodfilter and just passed it down into the doWithMethods() that getUniqueDeclaredMethods() calls. In the above case (for example) it would be like:

for (Method candidate: ReflectionUtils.getFilteredUniqueDeclaredMethods(listenerType, m->m.getName().equals("onApplicationEvent")) {

(I suppose here you could possibly even be quicker since you want to exit it on the first one if ReflectionUtils returned a stream of methods and you could stop on the first that met the condition). Just an observation...

 

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 22, 2018

Juergen Hoeller commented

It turns out that ResolvableType.as is expensive because of getInterfaces() and getSuperType creating serializable proxies, despite only being used for an introspection attempt. Since ResolvableType serializability is generally a niche feature and only really relevant for directly reflected types as a starting point (that is, in particular not necessary for derived handles that will never be stored on their own), I've streamlined the interface/superclass introspection algorithm to use straight forType wrapping (like in other places already). This makes the ApplicationListener introspection case with unchanged client code about 40% faster and brings it to the same performance as the direct method parameter introspection that you sketched.

If some of our portfolio projects happen to rely on serializability of derived ResolvableType instances (like we did ourselves in the case of Provider serialization for collection types), it should be easy enough to revise those spots accordingly: re-obtaining the derived handles from a primary ResolvableType instance, or - even better - marking the primary ResolvableType field as transient and lazily reobtaining it after deserialization. Aside from faster introspection, this also reduces the size of the serialized representation in such spots.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 22, 2018

Dave Syer commented

Excellent! This will have a big effect on performance for a large class of apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants