Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory profiling on macOS #98164

Closed
wants to merge 25 commits into from
Closed

Conversation

rdzhaafar
Copy link
Contributor

Currently, rustc_data_structures::profiling::get_resident_set_size() always returns None on macOS.
This proposed solution uses proc_pidinfo(...) from libproc. This function is available on any mac since 10.5
if the function signature in libproc.h is to be believed:

int proc_pidinfo(int pid, int flavor, uint64_t arg, void *buffer, int buffersize) __OSX_AVAILABLE_STARTING(__MAC_10_5, __IPHONE_2_0);

P.S: This is my first pull request to the Rust project. I'm open to criticism in case I messed something up.

`rustc_data_structures::profiling::get_resident_set_size()` for macos
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 16, 2022
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @michaelwoerister (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 16, 2022
notriddle and others added 21 commits June 16, 2022 21:22
This commit adds a new unstable attribute, `#[doc(tuple_varadic)]`, that
shows a 1-tuple as `(T, ...)` instead of just `(T,)`, and links to a section
in the tuple primitive docs that talks about these.
Before:

    impl<T, U> UnwindSafe for (T, ...) where
        T: UnwindSafe,
        U: UnwindSafe,

After:

    impl<T> UnwindSafe for (T, ...) where
        T: UnwindSafe,
Co-authored-by: Jacob Hoffman-Andrews <[email protected]>
@rdzhaafar
Copy link
Contributor Author

Just found out that libproc.h definitions exist in libc. Re-wrote the PR using those for cleanliness.

@rdzhaafar
Copy link
Contributor Author

Closing this PR, because I messed up git workflow

@rdzhaafar rdzhaafar closed this Jun 22, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 28, 2022
…r=davidtwco,michaelwoerister

Fix RSS reporting on macOS

> NOTE: This is a duplicate of rust-lang#98164, which I closed because I borked my rustc fork

Currently, `rustc_data_structures::profiling::get_resident_set_size()` always returns `None` on macOS. This is because
macOS does not implement procfs used in the unix version of the function:

```rust
...
else if #[cfg(unix)] {
        pub fn get_resident_set_size() -> Option<usize> {
            let field = 1;
            let contents = fs::read("/proc/self/statm").ok()?;
            let contents = String::from_utf8(contents).ok()?;
            let s = contents.split_whitespace().nth(field)?;
            let npages = s.parse::<usize>().ok()?;
            Some(npages * 4096)
        }
...
```

The proposed solution uses libproc, and more specifically `proc_pidinfo`, which has been available on macOS since 10.5 if the function signature inside libproc.h is to be believed:

```c
int proc_pidinfo(int pid, int flavor, uint64_t arg, void *buffer, int buffersize) __OSX_AVAILABLE_STARTING(__MAC_10_5, __IPHONE_2_0);
```
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 28, 2022
…r=davidtwco,michaelwoerister

Fix RSS reporting on macOS

> NOTE: This is a duplicate of rust-lang#98164, which I closed because I borked my rustc fork

Currently, `rustc_data_structures::profiling::get_resident_set_size()` always returns `None` on macOS. This is because
macOS does not implement procfs used in the unix version of the function:

```rust
...
else if #[cfg(unix)] {
        pub fn get_resident_set_size() -> Option<usize> {
            let field = 1;
            let contents = fs::read("/proc/self/statm").ok()?;
            let contents = String::from_utf8(contents).ok()?;
            let s = contents.split_whitespace().nth(field)?;
            let npages = s.parse::<usize>().ok()?;
            Some(npages * 4096)
        }
...
```

The proposed solution uses libproc, and more specifically `proc_pidinfo`, which has been available on macOS since 10.5 if the function signature inside libproc.h is to be believed:

```c
int proc_pidinfo(int pid, int flavor, uint64_t arg, void *buffer, int buffersize) __OSX_AVAILABLE_STARTING(__MAC_10_5, __IPHONE_2_0);
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants