docs: mark RuntimeConfig::chunk_emb_workers as currently unused - #14
docs: mark RuntimeConfig::chunk_emb_workers as currently unused#14attevon-admin wants to merge 2 commits into
Conversation
Confirmed by grep across the whole crate: no code path reads RuntimeConfig::chunk_emb_workers on either the CUDA or CoreML execution mode. Only the sibling chunk_emb_compute_units field is consumed, and only under the coreml feature. Document the gap rather than leave the field's purpose implied by its name (closes avencera#13's underlying confusion until it's actually wired up).
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| src/pipeline/config.rs | Documentation-only clarification accurately describes the existing unused runtime configuration field. |
Reviews (1): Last reviewed commit: "docs: mark chunk_emb_workers as currentl..." | Re-trigger Greptile
chunk_emb_compute_units (the sibling field) is already gated to the coreml feature, and CoreML sessions can only run one at a time - so this field almost certainly exists to parallelize chunk embedding across several single-threaded CoreML workers, not to control CUDA concurrency. CUDA already gets concurrency a different way, from shared mutex-protected ORT sessions (see the shared-sessions PR). Nothing to wire up on the CUDA path.
|
Pushed a follow-up: dug into why this field exists rather than leaving it a bare 'unused' note. Its sibling field, |
2 participants
Closes #13.
Confirmed by grep across the whole crate: no code path reads
RuntimeConfig::chunk_emb_workerson either the CUDA or CoreML execution mode — only the siblingchunk_emb_compute_unitsfield is consumed, and only under thecoremlfeature. This documents the gap rather than leave the field's purpose implied by its name, since actually wiring up a worker pool is a bigger design question than we wanted to guess at in a doc-only PR. Happy to take a swing at the real implementation if there's a preferred shape (rayon vs std::thread, where the pool should live).