Port hotspot/os/bsd/os_perf_bsd.cpp to BSD - #9
Conversation
Implements CPUPerformanceInterface::CPUPerformance for FreeBSD, OpenBSD and NetBSD. This is essentially a parallel implementation to the macOS implementation in this source file. This work is sponsored by The FreeBSD Foundation Co-authored-by: Kurt Miller <kurt@openjdk.org> Co-authored-by: Greg Lewis <glewis@openjdk.org>
|
👋 Welcome back haraldei! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
Webrevs
|
|
Is it really necessary to have a full parallel implementation? I would expect a large part to be sharable between bsd variants. |
|
@dean-long I don't see much overlap between the macOS and *BSD implementations. The macOS code seems to be targeted for the Mach kernel, which is not used by any of the others. To be fair, there's enough differences even among the traditional *BSD's that this code will be quite hairy regardless of how we approach it. |
dholmes-ora
left a comment
There was a problem hiding this comment.
This is certainly complex with all the conditionals. I can't help wonder if we wouldn't better off splitting some things out into separate per-BSD files.
| #define NET_RT_IFLIST2 NET_RT_IFLIST | ||
| #define RTM_IFINFO2 RTM_IFINFO | ||
| #endif | ||
| #ifdef __NetBSD__ |
There was a problem hiding this comment.
Why a simple ifdef here but a if !defined above? Can we use one style please.
There was a problem hiding this comment.
Probably to align with the #ifdef __APPLE__ above, but I've cleaned up this section in the new version of the patch so it's hopefully better now.
| #if !defined(KERN_PROC_PATHNAME) | ||
| #define KERN_PROC_PATHNAME 5 | ||
| #endif |
There was a problem hiding this comment.
Can this be handled where we do the #include?
There was a problem hiding this comment.
Ack, moved it up in the new version of the patch.
| static const double NANOS_PER_SEC = 1000000000.0; | ||
| static const time_t NANOS_PER_SEC = 1000000000LL; |
There was a problem hiding this comment.
I don't understand this change when most use-sites cast back to double anyway - and the one that doesn't should have been casting to an integral type in the first place.
There was a problem hiding this comment.
It's used once as each, but I agree, there's no reason to change this. I've reverted this and added the cast to uint64_t where it's being used as such.
- Clean up #include section - Revert NANOS_PER_SEC back to a double - Move NetBSD def of KERN_PROC_PATHNAME to #include section This work is sponsored by The FreeBSD Foundation
I would actually really like it if we could split this up instead. We have just tried to adapt to the structure that was already there, but if it's ok with you I'll be happy to work out a suggestion for a patch where we split it up instead. If it's ok, I would also like to split the different interface implementations (CPUPerformance, SystemProcess and NetworkPerformance) into separate source files as well. Will be many files then :) |
Lets try to not to go overboard with too fine-grained an approach. We also need to allow sharing where possible. Just in terms of the bigger picture, I am a bit concerned about all the different kinds of BSD being supported here. From a "supported port" perspective there needs to be a maintainer willing to take on all aspects of this. |
That makes sense. I'll try to split the BSD and macOS implementations, and then we can see from there.
I'm aware. As my work is supported by the FreeBSD Foundation, I can only commit to support for FreeBSD. @bsdkurt and @battleblow may be able to say more about OpenBSD and NetBSD. |
I'm happy to continue to maintain OpenBSD's portion of the bsd-port. I've been doing that for around 25 years now on a volunteer basis (except for some consulting work I did for the FreeBSD foundation's prior work at certification). NetBSD has not had a consistent developer engaged with the project over those years, but we've (Greg and myself) always tried to maintain support for it along the way. |
|
Mailing list message from Christos Zoulas on bsd-port-dev:
Thanks Kurt, I am around and I can try to help if needed for NetBSD. christos |
|
Mailing list message from Stephanie Bergmann on bsd-port-dev: OptOut! Stephanie Bergmann Christos Zoulas <christos@zoulas.com> schrieb am Mo., 1. Juni 2026, 15:37:
|
|
@snake66 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a |
Split implementation into separate source files for macOS and BSD where necessary, and consolidate the code that's common between them into a common source. - Split CPUPerformanceInterface impl for BSD and macOS, but keep the common PIMPL wrapper class. - Move SystemProcessInterface::SystemProcess to common code, implement functions to imitate relevant macOS proc.h function for the BSD's. - Drop retreiving process args for FreeBSD and NetBSD for now, this was not implemented for macOS or OpenBSD in any case. Can add it back later if it has value. - Move NetworkPerformanceInterface::NetworkPerformance to common code. Only minor adjustments via conditional sections between macOS and BSD implementation. This work is sponsored by The FreeBSD Foundation Co-authored-by: Kurt Miller <bsdkurt@gmail.com> Co-authored-by: Greg Lewis <glewis@eyesbeyond.com>
|
@snake66 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a |
Implements CPUPerformanceInterface::CPUPerformance for FreeBSD, OpenBSD and NetBSD. This is essentially a parallel implementation to the macOS implementation in this source file.
This work is sponsored by The FreeBSD Foundation
Progress
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/bsd-port.git pull/9/head:pull/9$ git checkout pull/9Update a local copy of the PR:
$ git checkout pull/9$ git pull https://git.openjdk.org/bsd-port.git pull/9/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 9View PR using the GUI difftool:
$ git pr show -t 9Using diff file
Download this PR as a diff file:
https://git.openjdk.org/bsd-port/pull/9.diff
Using Webrev
Link to Webrev Comment