Fix macOS collection of CPU data #7429
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Apparently StackOverflow doesn't have all the answers. The examples there I lifted this from didn't account to call `vm_deallocate` because it looks like we're handed allocated memory! Indeed running the previous state capture in a loop it infinitely allocated memory, but now it holds steady when called in a loop.
There's very little documentation on `host_processor_info` from what I can tell, so I'm just cribbing examples I've found elsewhere on the internet. It appears that I've misinterpreted one of the out parameters of `host_processor_info` as the number of elements of an array, but it actually is just something forwarded to `vm_deallocate`.
|
Seems to match what I can find as well. @bors r+ |
|
📌 Commit 0c812db has been approved by |
bors
added
the
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Sep 25, 2019
Fix macOS collection of CPU data There's very little documentation on `host_processor_info` from what I can tell, so I'm just cribbing examples I've found elsewhere on the internet. Turns out two things were wrong: * One is that `host_processor_info` returns allocated memory we need to deallocate. Who knew! * Next is that one of the out parameters, `cpu_info_cnt`, is only somehow related to the size of the return, but all example code appears to just read data regardless of what it is. In any case this commit reads [libuv's implementation](https://github.com/libuv/libuv/blob/040543eebf4983b1459a1e0e0e26dae68b80cc28/src/unix/darwin.c#L174-L225) which if good enough for node.js is probably good enough for us. Closes #7427
|
☀️ Test successful - checks-azure |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Sep 25, 2019
4 commits in aa6b7e01abce30091cc594cb23a15c46cead6e24..ab6fa8908c9b6c8f7e2249231c395eebfbc49891 2019-09-24 17:19:12 +0000 to 2019-09-25 17:00:56 +0000 - Fix macOS collection of CPU data (rust-lang/cargo#7429) - Don't ever capture CPU state if timings are disabled (rust-lang/cargo#7428) - Change build-std to use --sysroot (rust-lang/cargo#7421) - set -Zmtime_on_use from config or ENV (rust-lang/cargo#7411)
bors
added a commit
to rust-lang/rust
that referenced
this pull request
Sep 25, 2019
Update cargo 4 commits in aa6b7e01abce30091cc594cb23a15c46cead6e24..ab6fa8908c9b6c8f7e2249231c395eebfbc49891 2019-09-24 17:19:12 +0000 to 2019-09-25 17:00:56 +0000 - Fix macOS collection of CPU data (rust-lang/cargo#7429) - Don't ever capture CPU state if timings are disabled (rust-lang/cargo#7428) - Change build-std to use --sysroot (rust-lang/cargo#7421) - set -Zmtime_on_use from config or ENV (rust-lang/cargo#7411)
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's very little documentation on
host_processor_infofrom what I can tell, so I'm just cribbing examples I've found elsewhere on the internet. Turns out two things were wrong:host_processor_inforeturns allocated memory we need to deallocate. Who knew!cpu_info_cnt, is only somehow related to the size of the return, but all example code appears to just read data regardless of what it is.In any case this commit reads libuv's implementation which if good enough for node.js is probably good enough for us.
Closes #7427