Draft: async worker exploration plan - #40
Closed
ausimian wants to merge 5 commits into
Closed
Conversation
Captures phased migration away from WorkerThread::run_sync toward an async model where NIFs enqueue work and the worker posts results back via enif_send. Includes four spike proposals, a four-PR phased plan, backpressure/failure-mode analysis, and measurement criteria.
ASan/TSan on a NIF require rebuilding the BEAM VM with matching flags, which is not practical. Switch Spike A to a pure stress-test verification: 16 processes × 1M tensors, assert memory returns to baseline via :erlang.memory and Emily.Native.get_active_memory.
Spike A passed on OTP 28.3. Key results: enif_send does not transfer
msg_env ownership (caller must enif_free_env); enif_self must be
called from the scheduler thread; plain {ref, {:error, binary}} term
transports errors cleanly; 160k round-trip stress shows no MLX or
BEAM memory growth; dead-sender case does not leak. Phase 1 can
proceed on the substrate validated by the spike.
Reference: branch spike-a-enif-send, commit c119819.
Spike B passed on OTP 28.3. Option 3a (build the resource binary on the worker-allocated msg_env and send via enif_send) is viable; fallback strategies 3b/3c are not needed. Refcount arithmetic confirmed: fine::make_resource_binary + enif_send + enif_free_env nets to one resource ref on the receiver. 250-binary same-tensor stress reads correctly after dropping the source tensor; 250-distinct-tensor stress reclaims ~1 GB of MLX memory fully after GC + clear_cache. Noted caveats for future Phase 3 implementation: mx::contiguous aliases when the input is already contiguous (informs stress design); MLX cache must be cleared before measuring active memory to avoid false positives on leak tests. Reference: branch spike-b-resource-binary, commit 06eac7d.
Spike C (mailbox hygiene) passed: the receive-with-^ref pattern scales linearly when draining 5k-deep mailbox backlogs, at <100 µs per receive. The BEAM's reference-creation marker optimisation prevents O(N^2) scans. Spike D (PID liveness) passed: enif_send to dead PIDs silently drops, 500-process dead-sender race produces no MLX growth, and dropping a Stream mid-flight drains the worker cleanly. All four spikes (A, B, C, D) now complete; the phased implementation in section 2 can proceed unchanged. Reference: branch spike-cd-mailbox-liveness, commit 4c0f08e.
3 tasks
Owner
Author
|
Migration complete — Phases 1-3 landed as PRs #41, #42, #43. The plan doc on this branch served as a working scratchpad during implementation; now that every worker-bound NIF is async and backpressure is naturally handled by the receive-per-dispatch pattern, the doc is historical. Closing rather than merging since the phased TODO nature of the doc doesn't add value on main. Telemetry follow-up tracked separately. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Planning-only PR. Adds
docs/planning/async-worker-exploration.mdexploring droppingWorkerThread::run_syncin favour of an async model where the NIF enqueues work and the worker posts results back viaenif_send.What's in the doc
to_binary(c_src/emily_nif.cpp:93) andeval(c_src/emily_nif.cpp:116) are dirty-flagged. Every other op NIF runs on a regular scheduler and blocks it viarun_syncwhile queued behind the worker.evalcanary → Phase 2 bulk op NIF conversion → Phase 3to_binary(three strategies depending on Spike B outcome) → Phase 4 optional bounded queue + telemetry.Status
Draft. Not for merge. Opened for visibility on the proposed direction. Implementation begins only after the spikes clear in a separate branch.
Test plan
docs/planning/async-worker-exploration.md