websocket-spec
  1. websocket-spec
  2. WEBSOCKET_SPEC-184

ContainerProvider#getWebSocketContainer() has potential performance problems, needs caching

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.0
    • Fix Version/s: None
    • Labels:
      None

      Description

      Most other uses of the SPI system / `ServiceLoader` (read: every use I've ever seen) includes an underlying cache-by-classloader so that repeated calls do not perform badly. This comes in a form like this or similar:

      public abstract class ContainerProvider {
          private static final WeakHashMap<ClassLoader, SoftReference<Iterable<ContainerProvider>>> CACHE = new WeakHashMap<>();
      
          public static WebSocketContainer getWebSocketContainer() {
              WebSocketContainer wsc = null;
              for (ContainerProvider impl : getProviders()) {
                  wsc = impl.getContainer();
                  if (wsc != null) {
                      return wsc;
                  }
              }
              if (wsc == null) {
                  throw new RuntimeException("Could not find an implementation class.");
              } else {
                  throw new RuntimeException("Could not find an implementation class with a non-null WebSocketContainer.");
              }
          }
      
          private Iterable<ContainerProvider> getProviders() {
              ClassLoader loader = Thread.currentThread().getContextClassLoader();
              SoftReference<Iterable<ContainerProvider>> providers = CACHE.get(loader);
              if (providers != null && providers.get() != null) {
                  return providers.get();
              }
      
              synchronized(CACHE) {
                  providers = CACHE.get(loader);
                  if (providers == null || providers.get() == null) {
                      Set<ContainerProvider> set = new HashSet<>();
                      for (ContainerProvider provider : ServiceLoader.load(ContainerProvider.class, loader)) {
                          set.add(provider);
                      }
                      providers = new SoftReference<>(set);
                      CACHE.put(loader, providers);
                  }
              }
              
              return providers.get();
          }
          ...
      }
      

      The ContainerProvider should have this caching mechanism. Calls to ContainerProvider#getWebSocketContainer() are expensive right now, and repeated calls will be an unnecessary drain on system resources.

        Activity

        There are no comments yet on this issue.

          People

          • Assignee:
            Unassigned
            Reporter:
            Nick Williams
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated: