grizzly
  1. grizzly
  2. GRIZZLY-937

TCPNIOTransport forces thread pool to contain at least one worker thread even when the SameThreadStrategy is being used

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0 RC3
    • Fix Version/s: 2.1
    • Component/s: framework
    • Labels:
      None

      Description

      The start() method in TCPNIOTransport and presumably UDPNIOTransport assumes that the WorkerThreadStrategy will always be used. In particular, the following code does not work well with the SameThreadStrategy:

      if (threadPool == null)

      { >>>> No need for worker threads if SameThreadStrategy final ThreadPoolConfig config = ThreadPoolConfig.defaultConfig() .setCorePoolSize(selectorRunnersCount * 2) .setMaxPoolSize(selectorRunnersCount * 2) .setMemoryManager(getMemoryManager()); config.getInitialMonitoringConfig().addProbes( getThreadPoolMonitoringConfig().getProbes()); setThreadPool0(GrizzlyExecutorService.createInstance(config)); }

      else {
      if (threadPool instanceof GrizzlyExecutorService) {
      final ThreadPoolConfig config =
      ((GrizzlyExecutorService) threadPool).getConfiguration();

      >>>>> This condition should be true for SameThreadStrategy but we end up reducing the selector count by half and wasting half the threads.
      if (selectorRunnersCount >= config.getMaxPoolSize())

      { selectorRunnersCount = config.getMaxPoolSize() / 2; }

      }
      }

      Perhaps the strategy can provide methods for creating a default thread pool as well validating the config if provided by the caller.

        Activity

        Hide
        Ryan Lubke added a comment -

        I'll look into this...

        Show
        Ryan Lubke added a comment - I'll look into this...
        Hide
        Ryan Lubke added a comment -

        Applied changes to resolve this one (see revision 5582).

        I'm still mulling over the idea about config validation, but with the current changes, you shouldn't have threads sitting around.

        Show
        Ryan Lubke added a comment - Applied changes to resolve this one (see revision 5582). I'm still mulling over the idea about config validation, but with the current changes, you shouldn't have threads sitting around.
        Hide
        Ryan Lubke added a comment -

        Marking issue as resolved. Validation suggestion will not be pursued at this time.

        Show
        Ryan Lubke added a comment - Marking issue as resolved. Validation suggestion will not be pursued at this time.

          People

          • Assignee:
            Ryan Lubke
            Reporter:
            matthew_swift
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: