-
Notifications
You must be signed in to change notification settings - Fork 5.8k
8356000: C1/C2-only modes use 2 compiler threads on low CPU count machines #24972
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
base: master
Are you sure you want to change the base?
8356000: C1/C2-only modes use 2 compiler threads on low CPU count machines #24972
Conversation
👋 Welcome back shade! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
} | ||
assert((!c1_only && !c2_only) || count <= active_cpus, "Too many threads: %d", count); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be the general rule: don't create more compiler threads than available cpus?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except when specified on command line with -XX:CICompilerCount=n
.
Actually your changes does not take this flag into account.
There is an unfortunate limitation with default tiered policy that we would have at least 2 threads on 1 CPU machine: 1 thread for C1, and 1 thread for C2.
But if we select C1-only or C2-only modes, we also get 2 compiler threads, for which we have no good reason. These threads would just step on each other toes. The fix changes the behavior for 1..3 CPU hosts in C1/C2-only configurations, by using 1 thread instead of 2 threads. The change for 1 CPU config is what we really need. The change in 2..3 CPU configs is an additional effect, but I think it is still good not to use 100%/66% of the CPUs in those configurations as well.
It is a minor bug in
CompilationPolicy::initialize
, but it gets in the way studying Leyden in tight CPU scenarios.Additional testing:
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24972/head:pull/24972
$ git checkout pull/24972
Update a local copy of the PR:
$ git checkout pull/24972
$ git pull https://git.openjdk.org/jdk.git pull/24972/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 24972
View PR using the GUI difftool:
$ git pr show -t 24972
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24972.diff
Using Webrev
Link to Webrev Comment