title: Performance Issues with Multithreaded code when using CallableTask/Futures and ObjectMapper
body: <p>I am working on a REST service based project in which I have two components as mentioned below-</p>
  
  <ol>
  <li>Client which will make the necessary <code>URL's</code> for the Service component</li>
  <li>Then Service(REST service) component will use those <code>URL's</code> to get the data from database.</li>
  </ol>
  
  <p>In general URL will look like this-</p>
  
  <p><code>http://host.qa.ebay.com:8080/deservice/DEService/get/USERID=9012/PROFILE.ACCOUNT,PROFILE.ADVERTISING,PROFILE.DEMOGRAPHIC,PROFILE.FINANCIAL</code></p>
  
  <p>What it means from the above URL is- For the <code>USERID- 9012</code> give me the data from database for these columns-</p>
  
  <p><code>[PROFILE.ACCOUNT, PROFILE.ADVERTISING, PROFILE.DEMOGRAPHIC, PROFILE.FINANCIAL]</code></p>
  
  <p>And currently I am doing Benchmarking on the client component side. And I found out that below method is taking bunch of <code>time(95 Percentile)</code> around <code>~15ms</code>.</p>
  
  <p>Below method will accept two parameters-</p>
  
  <p><code>List&lt;DEKey&gt; keys- sample data in keys will have USERID=9012</code></p>
  
  <p><code>List&lt;String&gt; reqAttrNames- sample data for reqAttrNames will be-</code></p>
  
  <p><code>[PROFILE.ACCOUNT, PROFILE.ADVERTISING, PROFILE.DEMOGRAPHIC, PROFILE.FINANCIAL]</code></p>
  
  <p>Below is the code-</p>
  
  <pre><code>public DEResponse getDEAttributes(List&lt;DEKey&gt; keys, List&lt;String&gt; reqAttrNames) {
  
      DEResponse response = null;
      try {
          String url = buildGetUrl(keys,reqAttrNames);
  
          if(url!=null){
              List&lt;CallableTask&lt;DEResponse&gt;&gt; tasks = new ArrayList&lt;CallableTask&lt;DEResponse&gt;&gt;();
              CallableTask&lt;DEResponse&gt; task = new DEResponseTask(url); 
              tasks.add(task);
  
              // STEP 2: Execute worker threads for all the generated urls
              List&lt;LoggingFuture&lt;DEResponse&gt;&gt; futures = null;
              try {
                  long waitTimeout = getWaitTimeout(keys);
                  futures = executor.executeAll(tasks, null, waitTimeout, TimeUnit.MILLISECONDS);
  
                  // STEP 3: Consolidate results of the executed worker threads
                  if(futures!=null &amp;&amp; futures.size()&gt;0){
                      LoggingFuture&lt;DEResponse&gt; future = futures.get(0);
                      response = future.get();
                  }
              } catch (InterruptedException e1) {
                  logger.log(LogLevel.ERROR,"Transport:getDEAttributes Request timed-out :",e1);
              }
          }else{
              //
          }
      }  catch(Throwable th) {
  
      }
  
      return response;
  }
  </code></pre>
  
  <p>And the above method will give me back the <code>DEResponse</code> object.</p>
  
  <p>Below is the <code>DEResponseTask class</code></p>
  
  <pre><code>public class DEResponseTask  extends BaseNamedTask implements CallableTask&lt;DEResponse&gt; {
  
          private final ObjectMapper m_mapper = new ObjectMapper();
  
          @Override
          public DEResponse call() throws Exception {
              URL url = null;
              DEResponse DEResponse = null;
              try {
                  if(buildUrl!=null){
                      url = new URL(buildUrl);
  
                      DEResponse = m_mapper.readValue(url, DEResponse.class);
  
                  }else{
                      logger.log(LogLevel.ERROR, "DEResponseTask:call is null ");
                  }
              } catch (MalformedURLException e) {
  
              }catch (Throwable th) {
  
              }finally{
              }
  
              return DEResponse;
          }
      }
  </code></pre>
  
  <p>Is there any problem with the way this multithreaded code is written? If yes, how can I make this efficient?</p>
  
  <p>Signature for <code>executeAll</code> method for <code>executor</code> as in my company they have there own executor which will implement Sun Executor class-</p>
  
  <pre><code>/**
       * Executes the given tasks, returning a list of futures holding their 
       * status and results when all complete or the timeout expires, whichever
       * happens first.  &lt;tt&gt;Future.isDone()&lt;/tt&gt; is &lt;tt&gt;true&lt;/tt&gt; for each
       * element of the returned list.  Upon return, tasks that have not completed
       * are cancelled.  Note that a &lt;i&gt;completed&lt;/i&gt; task could have terminated
       * either normally or by throwing an exception.  The results of this method
       * are undefined if the given collection is modified while this operation is
       * in progress.  This is entirely analogous to
       * &lt;tt&gt;ExecutorService.invokeAll()&lt;/tt&gt; except for a couple of important
       * differences.  First, it cancels but does not &lt;b&gt;interrupt&lt;/b&gt; any 
       * unfinished tasks, unlike &lt;tt&gt;ExecutorService.invokeAll()&lt;/tt&gt; which
       * cancels and interrupts unfinished tasks.  This results in a better 
       * adherence to the specified timeout value, as interrupting threads may
       * have unexpected delays depending on the nature of the tasks.  Also, all 
       * eBay-specific features apply when the tasks are submitted with this 
       * method.
       * 
       * @param tasks the collection of tasks
       * @param timeout the maximum time to wait
       * @param unit the time unit of the timeout argument
       * @return a list of futures representing the tasks, in the same sequential
       * order as produced by the iterator for the given task list.  If the 
       * operation did not time out, each task will have completed.  If it did
       * time out, some of these tasks will not have completed.
       * @throws InterruptedException if interrupted while waiting, in which case
       * unfinished tasks are cancelled
       */
      public &lt;V&gt; List&lt;LoggingFuture&lt;V&gt;&gt; executeAll(Collection&lt;? extends CallableTask&lt;V&gt;&gt; tasks, 
                                                   Options options, 
                                                   long timeout, TimeUnit unit)
              throws InterruptedException {
          return executeAll(tasks, options, timeout, unit, false);
      }
  </code></pre>
  
  <p><strong>Update:-</strong></p>
  
  <p>This component is taking time as soon as I increase the load of my program which is doing Benchmarking by increasing the threads to <code>20</code></p>
  
  <p><code>newFixedThreadPool(20)</code></p>
  
  <p>But I believe this component works fine if I use-</p>
  
  <p><code>newSingleThreadExecutor</code></p>
  
  <p>The only reason, I can think of is, might be in the above code, there is a blocking call so that is the reason threads get blocked and that's why it is taking time?</p>
  
  <p><strong>Updated:-</strong></p>
  
  <p>So this line should be written like this?-</p>
  
  <pre><code>if(futures!=null &amp;&amp; futures.size()&gt;0){
                      LoggingFuture&lt;DEResponse&gt; future = futures.get(0);
                      //response = future.get();//replace this with below code-
  
                      while(!future.isDone()) {
                          Thread.sleep(500);
                      } 
  
                      response = future.get();
                  }
  </code></pre>