Skip to content

[Misc] Remove host-side memory monitoring functionality#761

Merged
hughperkins merged 1 commit into
mainfrom
hp/remove-memory-monitor
Jun 24, 2026
Merged

[Misc] Remove host-side memory monitoring functionality#761
hughperkins merged 1 commit into
mainfrom
hp/remove-memory-monitor

Conversation

@hughperkins

Copy link
Copy Markdown
Collaborator

Removes the C++ memory monitor (memory_usage_monitor.{h,cpp}) and its start_memory_monitoring Python binding. This was a debug-only utility with no callers in the codebase, and its implementation embedded a Python interpreter (pybind11/embed.h) + psutil, which is awkward to maintain (and unsupported under nanobind). The QUADRANTS_PYBIND_SOURCE glob picks up python/*.cpp, so no CMake change is needed.

psutil is left in place: it is still used independently by the CI build scripts and tests/python/test_memory.py.

Issue: #

Brief Summary

copilot:summary

Walkthrough

copilot:walkthrough

Removes the C++ memory monitor (memory_usage_monitor.{h,cpp}) and its
`start_memory_monitoring` Python binding. This was a debug-only utility with no
callers in the codebase, and its implementation embedded a Python interpreter
(pybind11/embed.h) + psutil, which is awkward to maintain (and unsupported under
nanobind). The QUADRANTS_PYBIND_SOURCE glob picks up python/*.cpp, so no CMake
change is needed.

psutil is left in place: it is still used independently by the CI build scripts
and tests/python/test_memory.py.
@hughperkins hughperkins marked this pull request as ready for review June 24, 2026 14:32

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fb052c09c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

m.def("print_profile_info", [&]() { Profiling::get_instance().print_profile_info(); });
m.def("clear_profile_info", [&]() { Profiling::get_instance().clear_profile_info(); });
m.def("start_memory_monitoring", start_memory_monitoring);
m.def("get_repo_dir", get_repo_dir);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Document the removed memory-monitor API

For users who call the exported qd._lib.core.start_memory_monitoring binding, this deletion changes the Python API by turning that call into an AttributeError, but the commit only removes C++ files and does not add any docs/ migration/removal note. The repository AGENTS.md explicitly asks review agents to flag public API or usage changes without corresponding user-facing docs, so this removal should be documented or otherwise called out for users.

Useful? React with 👍 / 👎.

@github-actions

Copy link
Copy Markdown

@hughperkins hughperkins merged commit dc45894 into main Jun 24, 2026
54 checks passed
@hughperkins hughperkins deleted the hp/remove-memory-monitor branch June 24, 2026 15:44
@github-actions

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant