[GitHub] [beam] lukecwik commented on a change in pull request #11590: [BEAM-8944] Improve UnboundedThreadPoolExecutor performance

[GitBox <gi...@apache.org>]

lukecwik commented on a change in pull request #11590:
URL: https://github.com/apache/beam/pull/11590#discussion_r420894920



##########
File path: sdks/python/apache_beam/utils/thread_pool_executor.py
##########
@@ -137,35 +101,33 @@ def submit(self, fn, *args, **kwargs):
     """
     future = _base.Future()
     work_item = _WorkItem(future, fn, args, kwargs)
-    try:
-      # Keep trying to get an idle worker from the queue until we find one
-      # that accepts the work.
-      while not self._idle_worker_queue.get(
-          block=False).accepted_work(work_item):
-        pass
-      return future
-    except queue.Empty:
-      with self._lock:
-        if self._shutdown:
-          raise RuntimeError(
-              'Cannot schedule new tasks after thread pool '
-              'has been shutdown.')
-
-        worker = _Worker(
-            self._idle_worker_queue,
-            self._permitted_thread_age_in_seconds,
-            work_item)
+    with self._lock:
+      if self._shutdown:
+        raise RuntimeError(
+            'Cannot schedule new tasks after thread pool has been shutdown.')
+      try:
+        self._idle_worker_queue.get(block=False).assign_work(work_item)
+
+        # If we have more idle threads then the max allowed, shutdown a thread.
+        if self._idle_worker_queue.qsize() > self._max_idle_threads:
+          try:
+            self._idle_worker_queue.get(block=False).shutdown()

Review comment:
       I was thinking removing one at a time to reduce the rate at which we kill threads would "average" out the thread creation/death rate better but I have no data to support this hunch.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org