grundprinzip commented on code in PR #46993:
URL: https://github.com/apache/spark/pull/46993#discussion_r1642326032
##########
python/pyspark/sql/connect/client/reattach.py:
##########
@@ -58,28 +58,38 @@ class ExecutePlanResponseReattachableIterator(Generator):
# Lock to manage the pool
_lock: ClassVar[RLock] = RLock()
- _release_thread_pool: Optional[ThreadPool] = ThreadPool(os.cpu_count() if
os.cpu_count() else 8)
+ _release_thread_pool_instance = None
+
+ @classmethod # type: ignore[misc]
+ @property
+ def _release_thread_pool(cls) -> ThreadPool:
+ with cls._lock:
+ if cls._release_thread_pool_instance is None:
+ cls._release_thread_pool_instance = ThreadPool(
+ os.cpu_count() if os.cpu_count() else 8
+ )
+ return cls._release_thread_pool_instance
Review Comment:
Nit: The goal of this code is to initialize the code exactly once. The above
implementation requires all callers to always acquire the lock even if it is
not needed. For that reason and to avoid congestion on the lock, we should do a
first check outside the critical path:
```suggestion
# Perform a first check outside the critical path.
if not cls._release_thread_pool_instance is None:
return cls._release_thread_pool_instance
with cls._lock:
if cls._release_thread_pool_instance is None:
cls._release_thread_pool_instance = ThreadPool(
os.cpu_count() if os.cpu_count() else 8
)
return cls._release_thread_pool_instance
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
