Refactor: WorkerType CHIP→NEXT_LEVEL, consolidate add_worker API, level hygiene#516
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a validation check in the register method to ensure it is only used at level 3 or higher and refactors the L2 payload execution logic into a dedicated helper method _run_l2_from_payload. I have included a critical review comment regarding the use of run_raw instead of run to prevent potential type errors when handling raw pointers from WorkerPayload.
| from .task_interface import ChipCallConfig # noqa: PLC0415 | ||
|
|
||
| assert self._chip_worker is not None | ||
| config = ChipCallConfig() | ||
| config.block_dim = payload.block_dim | ||
| config.aicpu_thread_num = payload.aicpu_thread_num | ||
| config.enable_profiling = payload.enable_profiling | ||
| self._chip_worker.run( | ||
| payload.callable, # type: ignore[arg-type] | ||
| payload.args, | ||
| config, | ||
| ) |
There was a problem hiding this comment.
The _run_l2_from_payload helper currently passes raw pointers (integers) from WorkerPayload to self._chip_worker.run(). However, ChipWorker.run() expects higher-level objects, not raw memory addresses. This will lead to a TypeError at runtime. Since WorkerPayload is designed to hold raw pointers for cross-process dispatch, you should use the run_raw method on the underlying _impl instance, which is designed to handle these pointers directly.
assert self._chip_worker is not None
self._chip_worker._impl.run_raw(
payload.callable,
payload.args,
payload.block_dim,
payload.aicpu_thread_num,
payload.enable_profiling,
)…el hygiene - Rename WorkerType enum: CHIP→NEXT_LEVEL, remove DIST (only two kinds of sub-worker: NEXT_LEVEL and SUB) - Rename scheduler pools: chip_workers→next_level_workers, chip_threads_→next_level_threads_ - Simplify DistWorker::add_worker — clean NEXT_LEVEL/SUB binary split instead of the old CHIP+DIST grouping hack - Consolidate nanobind API: replace add_chip_worker/add_chip_worker_native/ add_chip_process with overloaded add_next_level_worker - Worker.register() now raises RuntimeError at L2 (was silently useless) - Extract L2 WorkerPayload unpacking into _run_l2_from_payload() helper Part of hierarchical runtime refactor (Steps 2-4). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e5e8800 to
b144082
Compare
Summary
WorkerTypeenum:CHIP→NEXT_LEVEL, removeDIST(only two kinds of sub-worker:NEXT_LEVELandSUB)chip_workers→next_level_workers,chip_threads_→next_level_threads_DistWorker::add_worker— cleanNEXT_LEVEL/SUBbinary split instead of the oldCHIP+DISTgrouping hackadd_chip_worker/add_chip_worker_native/add_chip_processwith overloadedadd_next_level_workerWorker.register()now raisesRuntimeErrorat L2 (was silently useless)WorkerPayloadunpacking into_run_l2_from_payload()helperPart of hierarchical runtime refactor (Steps 2-4). See
.claude/plans/HIERARCHICAL_RUNTIME_REFACTOR.md.Testing
pytest tests/ut/py/test_chip_worker.py— 11 passedpytest tests/ut/py/test_dist_worker/— 17 passedctest(C++ unit tests) — 5 passed