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

Darwin running time #1622

Merged
merged 6 commits into from
Feb 25, 2025
Merged

Darwin running time #1622

merged 6 commits into from
Feb 25, 2025

Conversation

aestriplex
Copy link
Contributor

@aestriplex aestriplex commented Feb 24, 2025

This follows from issue #1619. The branch is rebased on the PR #1617.

At the moment, the running time of processes on macOS is not updated correctly. I made two commit to try to fix this problem.

Summary:

  • Removed the call to Platform_machTicksToNanoseconds. The proc_taskinfo structure contains the running time (both system and user) of the process. We should just take those value in nanoseconds and convert them in the proper unit. How the kernel handles this calculation (starting from low level cpu-dependent details) should not concern us.
  • I also removed the static function nanosecondsToCentiseconds because for some reason we lose precision (± 1ms) with that calculation. If we divide directly by 1e7, we get a more precise measure. Probably it's a good idea to have an inline function or (maybe better) a macro, just to give a name to that operation and avoid the magic number;
  • Since the GPU percentage update depends on the running time of the process, I updated the condition in the if statement. In fact, suppose we are at the nth iteration of the DarwinProcess_setFromLibprocPidinfo function. We should update the CPU usage value if the total CPU running time of the process is greater than the CPU running time of the process in the previous iteration. To do that, we do not need to use the timeIntervalNS.

TODO:

  • remove the static double Platform_nanosecondsPerMachTick var in Darwin/Platform.c as well. It's only used in the (removed) Platform_machTicksToNanoseconds.
  • remove the Platform_calculateNanosecondsPerMachTick function in Darwin/PlatformHelpers.c. It's only used to set the static double Platform_nanosecondsPerMachTick var once.

Reduce one level of indent by tweaking the conditional. No changes in
code behavior.
@aestriplex
Copy link
Contributor Author

Those two commit:

belong to the PR #1617. I rebased this branch on that one to minimize the conflicts with those changes

@BenBE BenBE added enhancement Extension or improvement to existing feature MacOS 🍏 MacOS / Darwin related issues labels Feb 25, 2025
@BenBE BenBE added this to the 3.4.0 milestone Feb 25, 2025
@BenBE BenBE linked an issue Feb 25, 2025 that may be closed by this pull request
aestriplex and others added 2 commits February 25, 2025 11:21
This deduces the RUNNING state from the proc_taskinfo structure
by using the number of running threads.

Fixes: htop-dev#1611
This uses kinfo_procs to deduce some further states,
that were not covered in the libproc code.

Co-authored-by: aestriplex <[email protected]>
Co-authored-by: Benny Baumann <[email protected]>
@BenBE
Copy link
Member

BenBE commented Feb 25, 2025

@aestriplex Could you complete the pending two TODOs? TIA.

@aestriplex
Copy link
Contributor Author

@BenBE Yes, I am committing those asap

@aestriplex
Copy link
Contributor Author

@BenBE I made a separate commit

@aestriplex
Copy link
Contributor Author

OpenBSD build pipeline failed with error

[build-openbsd-latest-clang](https://github.com/htop-dev/htop/actions/runs/13520021995/job/37777020241)
The job running on runner GitHub Actions 14 has exceeded the maximum execution time of 20 minutes.

May be due to temporary conditions, should I re-run it manually later?

@BenBE
Copy link
Member

BenBE commented Feb 25, 2025

Can you check if mach_timebase_info is used anywhere else and update the configure.ac if there's no further reference left?

P.S.: Can go in the same commit as the code cleanup, where the last reference is removed.

@aestriplex
Copy link
Contributor Author

@BenBE configure.ac updated. I squashed the changes in 802494f

Copy link
Member

@BenBE BenBE left a comment

Choose a reason for hiding this comment

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

LGTM.

@fasterit fasterit merged commit 77132de into htop-dev:main Feb 25, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Extension or improvement to existing feature MacOS 🍏 MacOS / Darwin related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running time of macOS process
4 participants