Do not use ServiceLoader every time a WebSocketSession is created

[SerCeMan]

Jetty version
9.4.27.v20200227

Java version

openjdk version "13.0.2" 2020-01-14
OpenJDK Runtime Environment Zulu13.29+9-CA (build 13.0.2+6-MTS)
OpenJDK 64-Bit Server VM Zulu13.29+9-CA (build 13.0.2+6-MTS, mixed mode, sharing)

OS type/version
Ubuntu 18.04.4 LTS

Description

Hey, Jetty Maintainers! After running a benchmark with a few thousands of WebSocket clients, we noticed in the CPU profile (it was also very noticeable in the allocation profile) that jetty scans jars looking for org.eclipse.jetty.websocket.common.RemoteEndpointFactory for every new accepted connection.

The graph corresponds to the following lines in WebSocketSession. Overriding this behaviour doesn't seem to be easy since a concrete class is created in the session factory.

https://github.com/eclipse/jetty.project/blob/67eb9b35311dfd67b149204105a4f980772fa755/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java#L171-L172

As far as I can tell from browsing the code, this is not an issue for the 10.x branch. However, it would be great to resolve it in 9.4.x somehow. As far as I can tell, there are a few options:

• Perform the lookup statically once on the startup.
• Provide a configuration option to disable this lookup.

What do you think? If any of the above sounds good to you, I'll be happy to submit a PR.

[gregw]

@SerCeMan Good catch!
I think a once only (perhaps not static - but maybe if no suitable lifecycle exists) is a good idea. I PR would be most welcome. @joakime and/or @lachlan-roberts can you give advice here as to the best way forward?

[lachlan-roberts]

I don't think it will be easy to do without using the static, the only way I see of doing it would be to add a method to the WebSocketContainerScope interface which is not ideal. I don't imagine many people will be using the feature to provide a custom RemoteEndpoint anyway, and its not a feature in jetty-10.

I think the best way to do this is probably just as a static on the WebSocketSession so we maintain the same API and behaviour. I can put up a quick PR, should only be a small change.

[lachlan-roberts]

Fixed with PR #4664