[Misc] Reduce the logs generated by lazy memory allocator#3068
[Misc] Reduce the logs generated by lazy memory allocator#3068deng451e merged 1 commit intoLMCache:devfrom
Conversation
Signed-off-by: ApostaC <yihua98@uchicago.edu>
There was a problem hiding this comment.
Code Review
This pull request introduces periodic logging of memory expansion progress in the LazyMemoryAllocator. It adds a LOG_INTERVAL constant, a new _log_expansion_progress method to report cumulative progress and percentage, and updates the _expand_worker to trigger logging every 10 GB or at the end of the expansion process. A review comment identifies a missing return type hint for the new method, which is required by the repository's style guide.
| """ | ||
| self._address_manager.sbrk(expand_size) | ||
|
|
||
| def _log_expansion_progress(self, expanded_since_last_log: int): |
There was a problem hiding this comment.
The new method _log_expansion_progress is missing a return type hint. According to the repository style guide, all new functions must have type hints for both arguments and return values.
| def _log_expansion_progress(self, expanded_since_last_log: int): | |
| def _log_expansion_progress(self, expanded_since_last_log: int) -> None: |
References
- All new functions have type hints (arguments + return values) (link)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 16d2931. Configure here.
| """ | ||
| self._address_manager.sbrk(expand_size) | ||
|
|
||
| def _log_expansion_progress(self, expanded_since_last_log: int): |
There was a problem hiding this comment.
New method missing return type annotation
Low Severity
The new _log_expansion_progress method is missing a -> None return type hint. The project's AGENTS.md convention requires all functions and methods to have type hints for both arguments and return values, and the review rule lists missing type hints on new functions as an error-level concern.
Triggered by project rule: LMCache Code Review Style Guide
Reviewed by Cursor Bugbot for commit 16d2931. Configure here.
What this PR does / why we need it:
The
LazyMemoryAllocatorcurrently logs every 1 GB of background pinned-memory expansion (COMMIT_SIZE). On machines with large pools (e.g. ~164 GB seen during MP server runs), this floods the log with hundreds of lines like:This PR:
The commit cadence (1 GB) is unchanged — only logging is decoupled from it.
Special notes for your reviewers:
No functional change to allocation behavior. Introduces a new class constant
LOG_INTERVAL = 10 << 30and a small_log_expansion_progresshelper.If applicable:
Note
Low Risk
Logging-only change in the background expansion worker; allocation/commit behavior is unchanged, with minimal runtime risk beyond altered observability cadence.
Overview
Reduces log volume from
LazyMemoryAllocatorbackground pinned-memory expansion by decoupling logging from the 1GB commit cadence and emitting progress logs only every 10GB (and on the final commit).Adds
LOG_INTERVAL, trackslast_log_size, and updates the log message to include the final target size and percent complete via a new_log_expansion_progresshelper.Reviewed by Cursor Bugbot for commit 16d2931. Bugbot is set up for automated code reviews on this repo. Configure here.