SpringConfigurator uses ListableBeanFactory method which is documented to be slow for EVERY new WebSocket connection [SPR-10605]

[spring-projects-issues]

Nick Williams opened SPR-10605 and commented

When using the server endpoint configurator org.springframework.web.socket.server.endpoint.SpringConfigurator to manage creation of WebSocket server endpoints, the SpringConfigurator#getEndpointInstance method will be called every time a new WebSocket connection is received.

getEndpointInstance calls the <T> T getBeansOfType(Class<T> ) method of the web application context (technically this method comes from ListableBeanFactory) each time it is called to determine whether to return a new instance, return an existing singleton instance, or throw an exception. However, the JavaDoc for ListableBeanFactory says this:

With the exception of getBeanDefinitionCount and containsBeanDefinition, the methods in this interface are not designed for frequent invocation. Implementations may be slow.

I do not believe SpringConfigurator should be using this method as it could have seriously negative performance consequences. I recommend something more like this:

...
		try {
			T singleton = wac.getBean(endpointClass);
			if (logger.isTraceEnabled()) {
				logger.trace("Using @ServerEndpoint singleton " + singleton);
			}
			return singleton;
		} catch(NoSuchBeanDefinitionException e) {
			if (logger.isTraceEnabled()) {
				logger.trace("Creating new @ServerEndpoint instance of type " + endpointClass);
			}
			return wac.getAutowireCapableBeanFactory().createBean(endpointClass);
		} catch(NoUniqueBeanDefinitionException e) {
			// Should not happen ..
			String message = "Found more than one matching @ServerEndpoint beans of type " + endpointClass;
			logger.error(message);
			throw new IllegalStateException(message);
		}
...

---

Affects: 4.0 M1

Issue Links:

• @Scheduled does not work on @ServerEndpoint deployed with per-connection lifecycle [SPR-10611] #15240 @Scheduled does not work on @ServerEndpoint deployed with per-connection lifecycle

[spring-projects-issues]

Nick Williams commented

Also, I'm not sure: should SpringConfigurator be using wac.createBean(endpointClass) or wac.createBean(endpointClass, AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE, true)? The difference between these methods has always confused me. I've never understood the JavaDoc "Performs full initialization of the bean, including all applicable BeanPostProcessors. ... It does not imply traditional by-name or by-type autowiring of properties; use createBean(Class, int, boolean) for that purposes."

[spring-projects-issues]

Rossen Stoyanchev commented

After a closer look, it seems that wac.getBean(endpointClass) internally calls getBeansOfType<Class<?> so it doesn't buy performance. That said you have a point that these calls aren't meant to be invoked frequently at runtime, essentially every time a WebSocket session opens.

Perhaps we should revisit how a singleton is discovered. Consider this example:

@ServerEndpoint(value = "/echo", configurator = SpringConfigurator.class)
public class EchoEndpoint {

  private final EchoService echoService;

  @Autowired
  public EchoAnnotatedEndpoint(EchoService echoService) {
    this.echoService = echoService;
  }

  // ...

}

All that SpringConfigurator knows at runtime is the endpoint class:

  @Override
  public <T> T getEndpointInstance(Class<T> endpointClass) {

     // ...

  }

If we stick to a lookup by bean name, then we could start with a bean name that matches the simple name of the class (in this case "echoEndpoint"). So if this bean is declared as an @Bean in Java config this would just work. A second option is to check if the class has @Component (or is an @Component stereotype annotation) and then get the name from that annotation. A third option is to use the value of the @ServerEndpoint annotation as the bean name to look up (in this case "/endpoint").

What do you think?

/cc Juergen Hoeller

[spring-projects-issues]

Rossen Stoyanchev commented

SpringConfigurator will now look for beans as follows:

• check using the type name converted to a property (e.g. EchoEndpoint -> echoEndpoint)
• check using the value of @Component if present

[spring-projects-issues]

Nick Williams commented

I'm sorry I didn't respond to this earlier. I meant to and it got lost. Now I wish I had, because...

I don't think this is the best approach, either. The problem is that the following is also a common approach that some people use for configuration:

@Bean
public CustomerSupportChatEndpoint supportEndpoint() {
    return ...
}

If a user does this, the system you've devised for finding the bean won't work.

I was actually going to suggest a cache of some type. Do the expensive getBeansOfType call once for a particular endpoint class and application context, and then use the cached bean for later calls to the configurator. The cache definition might look like this:

private static final Map<ApplicationContext, Map<Class<?>, Object>> CACHE = new HashMap<>();

This would find a bean in all situations, but also be faster than calling getBeansOfType every time the endpoint is connected to. It's not the prettiest thing in the world, but I don't see a better way.

[spring-projects-issues]

Rossen Stoyanchev commented

Good point. The current solution caters entirely to the @Component style and is less convenient for external (Java/XML) config. That said I'm not sure I like the idea of keeping references to beans and ApplicationContext's. A call to getBeanNamesForType + a cache with a lookup by Applicationcontext id to bean names should work just as well.

[spring-projects-issues]

Nick Williams commented

Yes! A much better solution. No memory leak this way.