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

Change jvmtiGetAvailableProcessors to use J9PORT_CPU_TARGET #2255

Conversation

anikser
Copy link
Contributor

@anikser anikser commented Jun 22, 2018

This change causes jvmtiGetAvailableProcessors to behave identically
to JVM_ActiveProcessorCount, also taking into account cgroup limits
when appropriate as discussed in #1166.

Signed-off-by: Mohammad Ali Nikseresht anikser@ibm.com

@DanHeidinga
Copy link
Member

Can you add a comment to both this method and to the JVM_ function to say the two implementations should be kept consistent?

@anikser anikser force-pushed the jvmti-get-available-processors-use-target branch from 37de50d to b80ef11 Compare June 22, 2018 18:30
Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

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

Thanks for adding the comment.

@@ -208,7 +208,8 @@ jvmtiGetAvailableProcessors(jvmtiEnv* env,

ENSURE_NON_NULL(processor_count_ptr);

cpuCount = j9sysinfo_get_number_CPUs_by_type(J9PORT_CPU_ONLINE);
/* This implementation should be kept consistent with JVM_ActiveProcessorCount */
cpuCount = j9sysinfo_get_number_CPUs_by_type(J9PORT_CPU_TARGET);
*processor_count_ptr = ((cpuCount == 0) ? 1 : (jint) cpuCount);
Copy link
Member

Choose a reason for hiding this comment

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

To make the two consistent, can you modify the ((cpuCount == 0) to be ((cpuCount < 1)

This change causes `jvmtiGetAvailableProcessors` to behave identically
to `JVM_ActiveProcessorCount`, also taking into account cgroup limits
when appropriate as discussed in eclipse-openj9#1166.

Signed-off-by: Mohammad Ali Nikseresht <anikser@ibm.com>
@anikser anikser force-pushed the jvmti-get-available-processors-use-target branch from b80ef11 to e876642 Compare June 26, 2018 13:04
@DanHeidinga
Copy link
Member

Jenkins test sanity plinux jdk8

@DanHeidinga DanHeidinga self-assigned this Jun 26, 2018
@DanHeidinga
Copy link
Member

travis build failed for infra issues contacting sourceforge.

@DanHeidinga DanHeidinga merged commit 9cf8de6 into eclipse-openj9:master Jul 6, 2018
@anikser anikser deleted the jvmti-get-available-processors-use-target branch July 6, 2018 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants