feat: coroutines #214
feat: coroutines #214
Conversation
loks0n
force-pushed
the
feat-coroutines
branch
4 times, most recently
from
43b89b8 to
63c546e
Compare
loks0n
force-pushed
the
feat-coroutines
branch
from
63c546e to
bcea5b2
Compare
| 'buffer_output_size' => $payloadSize, | ||
| 'worker_num' => (int)System::getEnv('OPR_EXECUTOR_WORKER_NUM', '1'), | ||
| 'task_worker_num' => 0, | ||
| 'max_coroutine' => (int)System::getEnv('OPR_EXECUTOR_MAX_COROUTINES', '100000'), |
There was a problem hiding this comment.
small suggestion to make this clearer
-'max_coroutine' => (int)System::getEnv('OPR_EXECUTOR_MAX_COROUTINES', '100000'),
+'max_coroutine' => (int)System::getEnv('OPR_EXECUTOR_MAX_COROUTINES', '100_000'), // Swoole's built-in default
| 'package_max_length' => $payloadSize, | ||
| 'buffer_output_size' => $payloadSize, | ||
| 'worker_num' => (int)System::getEnv('OPR_EXECUTOR_WORKER_NUM', '1'), | ||
| 'task_worker_num' => 0, |
There was a problem hiding this comment.
task_worker_num by default is 0, any reason to explicitly mark the value here?
| } | ||
| batch($jobsRuntimes); | ||
|
|
||
| $this->networkManager->removeAll(); |
There was a problem hiding this comment.
do we need to call removeNetworks here?
There was a problem hiding this comment.
We can't do it here, Http controls the signal handling
| } | ||
|
|
||
| public function connectAll(Container $container): void | ||
| private function ensure(string $network): ?string |
There was a problem hiding this comment.
nit: very small opinion of mine to have public methods placed above private ones so the class is easier to follow
in this case maybe current setup makes more sense since ensure is used directly above? so feel free to ignore if u feel otherwise
| $this->orchestration->createNetwork($network, false); | ||
| Console::success("[NetworkManager] Created network: {$network}"); | ||
| return $network; | ||
| } catch (\Throwable $e) { |
There was a problem hiding this comment.
should we retain the TODO comment here?
| fn ($network) => fn () => $this->remove($network), | ||
| $this->available | ||
| )); | ||
| foreach ($networks as $network) { |
There was a problem hiding this comment.
any reason for removing batch here?
There was a problem hiding this comment.
It's unnecessary imo, we only have one network... even multiple would be fast
No description provided.