This issue documents a set of improvements identified during a review of the codebase.
1. collectDescendants() rebuilds $childMap on every call
In ProcessKiller::killTreesUnixMapped(), collectDescendants() is called once per PID. Inside that method, a $childMap is rebuilt from $parentMap every time by iterating all entries. When shutting down a pool with N workers, this results in N redundant full passes over the parent map. The child map should be built once from the already-available $parentMap and passed into collectDescendants() as a parameter.
File: src/Utilities/ProcessKiller.php
2. debug_backtrace() called on every pool task submission
ProcessPool::run() calls debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 4) on every task submission to determine a source location for error attribution. Every submitted task pays a full stack walk even on the happy path where no error occurs. The source location is only meaningful when a task fails. This overhead is avoidable by deferring source location resolution to the error path, or by making it opt-in via a pool-level flag.
File: src/ProcessPool.php
3. Config::loadFromRoot() called on every pool task submission
ProcessPool::run() calls Config::loadFromRoot('hibla_parallel', 'process.timeout', 60) on every invocation to resolve the configured timeout. This value never changes between task submissions. It should be resolved once, at pool construction time, and stored as a property. ProcessManager already follows this pattern correctly by caching config values in its constructor.
File: src/ProcessPool.php
4. killTreesUnixFallback() uses unbounded recursion
The fallback kill strategy in ProcessKiller recursively calls itself per discovered child PID using pgrep. For deeply nested process trees, this can exhaust the call stack. An iterative approach using an explicit queue would eliminate this risk entirely.
File: src/Utilities/ProcessKiller.php
5. assert() used for type narrowing in ExceptionHandler
ExceptionHandler::createFromWorkerError() uses assert(\is_string($message)) and assert(\is_string($workerTrace)) for type narrowing before operating on those values. PHP's assert() is a no-op when assertions are disabled in production (zend.assertions = -1), which is the common production default. These should be real conditional checks with explicit fallback handling.
File: src/Handlers/ExceptionHandler.php
6. /proc/cpuinfo CPU count has a potential off-by-one
SystemUtilities::getCpuCount() uses substr_count($output, "\nprocessor") + 1 to count CPU cores from /proc/cpuinfo. This relies on a leading newline before each "processor" entry. If the file begins with "processor" on the very first line (no preceding newline), the first core is missed and the + 1 compensation produces a wrong count for a single-core system. Using preg_match_all('/^processor/m', $output, $m) is more reliable.
File: src/Utilities/SystemUtilities.php
7. emit() serialization gap for arrays containing objects
The emit() helper in src/functions.php checks \is_object($data) || \is_resource($data) to decide whether to serialize before JSON-encoding. An array value like ['result' => new Dto()] passes this check as a plain array and goes directly to json_encode(). Since json_encode() silently encodes objects as {} when they lack JsonSerializable, callers passing arrays of objects receive silently corrupted data. The documentation should make this constraint explicit, or the check should cover arrays containing objects.
File: src/functions.php
8. ProcessManager and ProcessSpawnHandler are not final
All other concrete classes in the library are declared final. ProcessManager and ProcessSpawnHandler are not. This is inconsistent with the codebase convention and allows accidental subclassing of internal implementation classes.
Files: src/Managers/ProcessManager.php, src/Handlers/ProcessSpawnHandler.php
9. ExceptionHandler creates two ReflectionObject instances for the same exception
ExceptionHandler::createFromWorkerError() calls appendWorkerStackTrace() and setExceptionLocation() in sequence on the same exception object. Each method independently creates a new ReflectionObject($exception). These two calls can be merged into a single reflection pass.
File: src/Handlers/ExceptionHandler.php
10. strpos() used instead of str_contains() in BackgroundProcess::isRunning()
BackgroundProcess::isRunning() uses strpos($output, (string)$this->pid) !== false for the Windows and Unix shell fallback paths. The rest of the codebase consistently uses str_contains(). This is a minor inconsistency against the PHP 8.x idiom adopted throughout.
File: src/Internals/BackgroundProcess.php
11. Missing space before array in return[$parentMap, $pgidMap]
ProcessKiller::buildProcMaps() contains return[$parentMap, $pgidMap]; without a space between return and [. PSR-12 requires a space here. This should be caught by Pint but is currently present in the codebase.
File: src/Utilities/ProcessKiller.php
12. Missing @throws on ProcessPool::run()
ProcessPool::run() returns a rejected Promise when the pool is shut down (wrapping PoolShutdownException), but the public API docblock does not document this. Callers relying solely on docblocks for error handling will miss this rejection path. A note in the docblock clarifying rejection reasons would improve the public API contract.
File: src/ProcessPool.php
Happy to submit a pull request addressing any or all of the above. I will start with an issue per the contributing guidelines and await your direction on which items to prioritize.
This issue documents a set of improvements identified during a review of the codebase.
1.
collectDescendants()rebuilds$childMapon every callIn
ProcessKiller::killTreesUnixMapped(),collectDescendants()is called once per PID. Inside that method, a$childMapis rebuilt from$parentMapevery time by iterating all entries. When shutting down a pool with N workers, this results in N redundant full passes over the parent map. The child map should be built once from the already-available$parentMapand passed intocollectDescendants()as a parameter.File:
src/Utilities/ProcessKiller.php2.
debug_backtrace()called on every pool task submissionProcessPool::run()callsdebug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 4)on every task submission to determine a source location for error attribution. Every submitted task pays a full stack walk even on the happy path where no error occurs. The source location is only meaningful when a task fails. This overhead is avoidable by deferring source location resolution to the error path, or by making it opt-in via a pool-level flag.File:
src/ProcessPool.php3.
Config::loadFromRoot()called on every pool task submissionProcessPool::run()callsConfig::loadFromRoot('hibla_parallel', 'process.timeout', 60)on every invocation to resolve the configured timeout. This value never changes between task submissions. It should be resolved once, at pool construction time, and stored as a property.ProcessManageralready follows this pattern correctly by caching config values in its constructor.File:
src/ProcessPool.php4.
killTreesUnixFallback()uses unbounded recursionThe fallback kill strategy in
ProcessKillerrecursively calls itself per discovered child PID usingpgrep. For deeply nested process trees, this can exhaust the call stack. An iterative approach using an explicit queue would eliminate this risk entirely.File:
src/Utilities/ProcessKiller.php5.
assert()used for type narrowing inExceptionHandlerExceptionHandler::createFromWorkerError()usesassert(\is_string($message))andassert(\is_string($workerTrace))for type narrowing before operating on those values. PHP'sassert()is a no-op when assertions are disabled in production (zend.assertions = -1), which is the common production default. These should be real conditional checks with explicit fallback handling.File:
src/Handlers/ExceptionHandler.php6.
/proc/cpuinfoCPU count has a potential off-by-oneSystemUtilities::getCpuCount()usessubstr_count($output, "\nprocessor") + 1to count CPU cores from/proc/cpuinfo. This relies on a leading newline before each "processor" entry. If the file begins with "processor" on the very first line (no preceding newline), the first core is missed and the+ 1compensation produces a wrong count for a single-core system. Usingpreg_match_all('/^processor/m', $output, $m)is more reliable.File:
src/Utilities/SystemUtilities.php7.
emit()serialization gap for arrays containing objectsThe
emit()helper insrc/functions.phpchecks\is_object($data) || \is_resource($data)to decide whether to serialize before JSON-encoding. An array value like['result' => new Dto()]passes this check as a plain array and goes directly tojson_encode(). Sincejson_encode()silently encodes objects as{}when they lackJsonSerializable, callers passing arrays of objects receive silently corrupted data. The documentation should make this constraint explicit, or the check should cover arrays containing objects.File:
src/functions.php8.
ProcessManagerandProcessSpawnHandlerare notfinalAll other concrete classes in the library are declared
final.ProcessManagerandProcessSpawnHandlerare not. This is inconsistent with the codebase convention and allows accidental subclassing of internal implementation classes.Files:
src/Managers/ProcessManager.php,src/Handlers/ProcessSpawnHandler.php9.
ExceptionHandlercreates twoReflectionObjectinstances for the same exceptionExceptionHandler::createFromWorkerError()callsappendWorkerStackTrace()andsetExceptionLocation()in sequence on the same exception object. Each method independently creates anew ReflectionObject($exception). These two calls can be merged into a single reflection pass.File:
src/Handlers/ExceptionHandler.php10.
strpos()used instead ofstr_contains()inBackgroundProcess::isRunning()BackgroundProcess::isRunning()usesstrpos($output, (string)$this->pid) !== falsefor the Windows and Unix shell fallback paths. The rest of the codebase consistently usesstr_contains(). This is a minor inconsistency against the PHP 8.x idiom adopted throughout.File:
src/Internals/BackgroundProcess.php11. Missing space before array in
return[$parentMap, $pgidMap]ProcessKiller::buildProcMaps()containsreturn[$parentMap, $pgidMap];without a space betweenreturnand[. PSR-12 requires a space here. This should be caught by Pint but is currently present in the codebase.File:
src/Utilities/ProcessKiller.php12. Missing
@throwsonProcessPool::run()ProcessPool::run()returns a rejectedPromisewhen the pool is shut down (wrappingPoolShutdownException), but the public API docblock does not document this. Callers relying solely on docblocks for error handling will miss this rejection path. A note in the docblock clarifying rejection reasons would improve the public API contract.File:
src/ProcessPool.phpHappy to submit a pull request addressing any or all of the above. I will start with an issue per the contributing guidelines and await your direction on which items to prioritize.