8365606: Container code should not be using jlong/julong #27743
+1,138
−772
Conversation
This file contains hidden or 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
|
👋 Welcome back sgehwolf! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
Testing details:cgroup v2 F42 (docker): cgroup v2 F39 (docker): cgroup v1 RHEL 8 (podman): Note: The skipped test on RHEL 8 is |
|
/label add hotspot-jfr |
|
@jerboaa |
|
GHA test failure on Windows x64 is unrelated (this is a Linux only patch): |
|
There seems to be some collision here with JDK-8367319 |
Thanks, yes, I'm aware. We'll resolve conflicts once we know which one goes in first. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Please review this revised version of getting rid of
jlongandjulongin internal HotSpot code. The single remaining usage is usingos::elapsed_counter()which I think is still ok. This refactoring is for the container detection code to (mostly) do away with negative return values.It gets rid of the trifold-use of return value: 1.) error, 2) unlimited values 3) actual numbers/values/limits. Instead, all container related values are now being read from the interface files as
uint64_tand afterwards interpreted in the way that make sense for the API implementations. For example,cpuvalues will essentially be treated asints as before, potentially returning a negative value-1for unlimited. For memory sizes the typephysical_memory_size_typehas been chosen. When there is no limit for a specific memory size a valuevalue_unlimitedis being returned.All error cases have been changed to returning
falsein the API functions (and no value is being set in the passed in reference for the value). The effect of this is that all container related functions now return abooland require a reference to be passed in for thevaluethat is being asked for.All usages of the API have been changed to use the revised API. There is no more usages for
OSCONTAINER_ERROR(`-2) in HotSpot code.While working on this, I've noticed that there are still some calls deep in the cgroup subsystem code to query "machine" info (e.g.
os::Linux::active_processor_count()). I've filed JDK-8369503 to get this cleaned-up as this patch was already getting large.Testing (looking good):
jdk.SwapSpaceevent) andVM.infodiagnostic command.Thoughts? Opinions?
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27743/head:pull/27743$ git checkout pull/27743Update a local copy of the PR:
$ git checkout pull/27743$ git pull https://git.openjdk.org/jdk.git pull/27743/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27743View PR using the GUI difftool:
$ git pr show -t 27743Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27743.diff
Using Webrev
Link to Webrev Comment