Add idle model unload support#7
Conversation
hwdsl2
left a comment
There was a problem hiding this comment.
@valmayaki Thanks for working on this. I think idle model unloading is a useful addition for systems where Whisper shares limited RAM or GPU memory with other workloads. The feature is also backward-compatible because WHISPER_IDLE_UNLOAD_SECONDS=0 preserves the current behavior and does not start the idle monitor.
Before merging, I recommend addressing the following:
- Add tests for the actual idle-monitor behavior.
The current tests cover releasing and recreating the model directly, but they do not exercise the monitor itself, which contains most of the new timing and concurrency logic. It would be helpful to test at least:- The model is not unloaded before the configured timeout.
- The model is unloaded after the timeout.
- The monitor does not unload while inference holds _inference_lock.
- The monitor rechecks the last-used timestamp after acquiring the lock, so a recent request prevents an unload.
- A request successfully reloads the model after a monitor-triggered unload.
- Repeated unload/reload cycles continue to work.
- Cancel and await the monitor task during shutdown.
Calling cancel() requests cancellation but does not guarantee that the task has exited before model cleanup continues. Awaiting the cancelled task, while suppressing asyncio.CancelledError, would make shutdown deterministic and avoid leaving a pending task behind. The task reference should also be reset to None. - Remove the torch.cuda.empty_cache() logic and its related test.
The Whisper model is managed by faster-whisper/CTranslate2, not by PyTorch. Clearing PyTorch’s allocator does not release CTranslate2-owned model allocations and may be misleading about how memory is actually freed. Dropping the WhisperModel reference and allowing the CTranslate2 object to be destroyed is the relevant cleanup path for the current implementation. - Consider removing _active_inferences.
Both inference and unloading are already serialized by _inference_lock. Since the counter is incremented only after obtaining that lock and decremented before releasing it, it does not appear to add protection under the current single-inference design. Removing it would reduce global mutable state and simplify the monitor logic. Keeping it is reasonable only if it has a documented future purpose, such as supporting concurrent inference or exposing activity metrics. - Clarify the timeout semantics in the documentation.
Because the monitor polls periodically, unloading happens approximately after the configured idle duration rather than at the exact second. This is especially noticeable for very small timeout values because the polling interval has a minimum of five seconds. - Clarify what is and is not unloaded.
This change unloads the faster-whisper model, but it does not unload the separate diarization pipeline. Users who enable diarization may therefore see only partial memory reduction.
I do not think native CTranslate2 unload_model() / load_model() needs to be required for this PR. Releasing and reconstructing the complete WhisperModel is a reasonable initial implementation, may release more wrapper-side host memory, and avoids coupling this project directly to faster-whisper’s internal .model attribute.
Native unloading can still be benchmarked and adopted later behind a centralized model-lifecycle abstraction without changing the public API. It may provide faster reloads, but it should be selected based on measured RAM/VRAM release, reload latency, repeated-cycle stability, and dependency compatibility.
The key acceptance criteria for this PR should be:
- Meaningful model memory is released after the idle timeout.
- Existing behavior remains unchanged when the timeout is 0.
- The next request reloads successfully without restarting the container.
- Repeated unload/reload cycles do not leak memory or break inference.
- Offline/local-only configuration continues to work after reload.
With the items above addressed, I think this would be a valuable and maintainable long-term improvement.
|
Thank you very much. I will update as recommended |
Add an idle unload path to the Whisper server so the resident model is released after a configurable period of inactivity and reloaded on demand.
Changes:
WHISPER_IDLE_UNLOAD_SECONDS