Skip to content

Commit

Permalink
8269851: OperatingSystemMXBean getProcessCpuLoad reports incorrect pr…
Browse files Browse the repository at this point in the history 
…ocess cpu usage in containers

Co-authored-by: Severin Gehwolf <sgehwolf@openjdk.org>
Reviewed-by: sgehwolf
  • Loading branch information
@jerboaa @DamonFool
2 people authored and DamonFool committed Jul 28, 2021
1 parent 41b4c19 commit 25f00d7
Showing 1 changed file with 140 additions and 81 deletions.
221 changes: 140 additions & 81 deletions src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@

package com.sun.management.internal;

import java.util.concurrent.TimeUnit;
import java.util.function.DoubleSupplier;
import java.util.function.LongSupplier;
import java.util.function.ToDoubleFunction;

import jdk.internal.platform.Metrics;
import sun.management.BaseOperatingSystemImpl;
import sun.management.VMManagement;

import java.util.concurrent.TimeUnit;
/**
* Implementation class for the operating system.
* Standard and committed hotspot-specific metrics if any.
Expand All @@ -42,8 +45,137 @@ class OperatingSystemImpl extends BaseOperatingSystemImpl

private static final int MAX_ATTEMPTS_NUMBER = 10;
private final Metrics containerMetrics;
private long usageTicks = 0; // used for cpu load calculation
private long totalTicks = 0; // used for cpu load calculation
private ContainerCpuTicks systemLoadTicks = new SystemCpuTicks();
private ContainerCpuTicks processLoadTicks = new ProcessCpuTicks();

private abstract class ContainerCpuTicks {
private long usageTicks = 0;
private long totalTicks = 0;

private double getUsageDividesTotal(long usageTicks, long totalTicks) {
// If cpu quota or cpu shares are in effect. Calculate the cpu load
// based on the following formula (similar to how
// getCpuLoad0() is being calculated):
//
// | usageTicks - usageTicks' |
// ------------------------------
// | totalTicks - totalTicks' |
//
// where usageTicks' and totalTicks' are historical values
// retrieved via an earlier call of this method.
if (usageTicks < 0 || totalTicks <= 0) {
return -1;
}
long distance = usageTicks - this.usageTicks;
this.usageTicks = usageTicks;
long totalDistance = totalTicks - this.totalTicks;
this.totalTicks = totalTicks;
double systemLoad = 0.0;
if (distance > 0 && totalDistance > 0) {
systemLoad = ((double)distance) / totalDistance;
}
// Ensure the return value is in the range 0.0 -> 1.0
systemLoad = Math.max(0.0, systemLoad);
systemLoad = Math.min(1.0, systemLoad);
return systemLoad;
}

public double getContainerCpuLoad() {
assert(containerMetrics != null);
long quota = containerMetrics.getCpuQuota();
long share = containerMetrics.getCpuShares();
if (quota > 0) {
long numPeriods = containerMetrics.getCpuNumPeriods();
long quotaNanos = TimeUnit.MICROSECONDS.toNanos(quota * numPeriods);
return getUsageDividesTotal(cpuUsageSupplier().getAsLong(), quotaNanos);
} else if (share > 0) {
long hostTicks = getHostTotalCpuTicks0();
int totalCPUs = getHostOnlineCpuCount0();
int containerCPUs = getAvailableProcessors();
// scale the total host load to the actual container cpus
hostTicks = hostTicks * containerCPUs / totalCPUs;
return getUsageDividesTotal(cpuUsageSupplier().getAsLong(), hostTicks);
} else {
// If CPU quotas and shares are not active then find the average load for
// all online CPUs that are allowed to run this container.

// If the cpuset is the same as the host's one there is no need to iterate over each CPU
if (isCpuSetSameAsHostCpuSet()) {
return defaultCpuLoadSupplier().getAsDouble();
} else {
int[] cpuSet = containerMetrics.getEffectiveCpuSetCpus();
// in case the effectiveCPUSetCpus are not available, attempt to use just cpusets.cpus
if (cpuSet == null || cpuSet.length <= 0) {
cpuSet = containerMetrics.getCpuSetCpus();
}
if (cpuSet == null) {
// cgroups is mounted, but CPU resource is not limited.
// We can assume the VM is run on the host CPUs.
return defaultCpuLoadSupplier().getAsDouble();
} else if (cpuSet.length > 0) {
return cpuSetCalc().applyAsDouble(cpuSet);
}
return -1;
}
}
}

protected abstract DoubleSupplier defaultCpuLoadSupplier();
protected abstract ToDoubleFunction<int[]> cpuSetCalc();
protected abstract LongSupplier cpuUsageSupplier();
}

private class ProcessCpuTicks extends ContainerCpuTicks {

@Override
protected DoubleSupplier defaultCpuLoadSupplier() {
return () -> getProcessCpuLoad0();
}

@Override
protected ToDoubleFunction<int[]> cpuSetCalc() {
return (int[] cpuSet) -> {
int totalCPUs = getHostOnlineCpuCount0();
int containerCPUs = getAvailableProcessors();
return Math.min(1.0, getProcessCpuLoad0() * totalCPUs / containerCPUs);
};
}

@Override
protected LongSupplier cpuUsageSupplier() {
return () -> getProcessCpuTime();
}

}

private class SystemCpuTicks extends ContainerCpuTicks {

@Override
protected DoubleSupplier defaultCpuLoadSupplier() {
return () -> getCpuLoad0();
}

@Override
protected ToDoubleFunction<int[]> cpuSetCalc() {
return (int[] cpuSet) -> {
double systemLoad = 0.0;
for (int cpu : cpuSet) {
double cpuLoad = getSingleCpuLoad0(cpu);
if (cpuLoad < 0) {
return -1;
}
systemLoad += cpuLoad;
}
return systemLoad / cpuSet.length;
};
}

@Override
protected LongSupplier cpuUsageSupplier() {
return () -> containerMetrics.getCpuUsage();
}

}

OperatingSystemImpl(VMManagement vm) {
super(vm);
Expand Down Expand Up @@ -134,90 +266,17 @@ public long getMaxFileDescriptorCount() {
return getMaxFileDescriptorCount0();
}

private double getUsageDividesTotal(long usageTicks, long totalTicks) {
// If cpu quota or cpu shares are in effect calculate the cpu load
// based on the following formula (similar to how
// getCpuLoad0() is being calculated):
//
// | usageTicks - usageTicks' |
// ------------------------------
// | totalTicks - totalTicks' |
//
// where usageTicks' and totalTicks' are historical values
// retrieved via an earlier call of this method.
//
// Total ticks should be scaled to the container effective number
// of cpus, if cpu shares are in effect.
if (usageTicks < 0 || totalTicks <= 0) {
return -1;
}
long distance = usageTicks - this.usageTicks;
this.usageTicks = usageTicks;
long totalDistance = totalTicks - this.totalTicks;
this.totalTicks = totalTicks;

double systemLoad = 0.0;
if (distance > 0 && totalDistance > 0) {
systemLoad = ((double)distance) / totalDistance;
}
// Ensure the return value is in the range 0.0 -> 1.0
systemLoad = Math.max(0.0, systemLoad);
systemLoad = Math.min(1.0, systemLoad);
return systemLoad;
}

public double getCpuLoad() {
if (containerMetrics != null) {
long quota = containerMetrics.getCpuQuota();
long share = containerMetrics.getCpuShares();
long usageNanos = containerMetrics.getCpuUsage();
if (quota > 0) {
long numPeriods = containerMetrics.getCpuNumPeriods();
long quotaNanos = TimeUnit.MICROSECONDS.toNanos(quota * numPeriods);
return getUsageDividesTotal(usageNanos, quotaNanos);
} else if (share > 0) {
long hostTicks = getHostTotalCpuTicks0();
int totalCPUs = getHostOnlineCpuCount0();
int containerCPUs = getAvailableProcessors();
// scale the total host load to the actual container cpus
hostTicks = hostTicks * containerCPUs / totalCPUs;
return getUsageDividesTotal(usageNanos, hostTicks);
} else {
// If CPU quotas and shares are not active then find the average system load for
// all online CPUs that are allowed to run this container.

// If the cpuset is the same as the host's one there is no need to iterate over each CPU
if (isCpuSetSameAsHostCpuSet()) {
return getCpuLoad0();
} else {
int[] cpuSet = containerMetrics.getEffectiveCpuSetCpus();
// in case the effectiveCPUSetCpus are not available, attempt to use just cpusets.cpus
if (cpuSet == null || cpuSet.length <= 0) {
cpuSet = containerMetrics.getCpuSetCpus();
}
if (cpuSet == null) {
// cgroups is mounted, but CPU resource is not limited.
// We can assume the VM is run on the host CPUs.
return getCpuLoad0();
} else if (cpuSet.length > 0) {
double systemLoad = 0.0;
for (int cpu : cpuSet) {
double cpuLoad = getSingleCpuLoad0(cpu);
if (cpuLoad < 0) {
return -1;
}
systemLoad += cpuLoad;
}
return systemLoad / cpuSet.length;
}
return -1;
}
}
return systemLoadTicks.getContainerCpuLoad();
}
return getCpuLoad0();
}

public double getProcessCpuLoad() {
if (containerMetrics != null) {
return processLoadTicks.getContainerCpuLoad();
}
return getProcessCpuLoad0();
}

Expand Down

5 comments on commit 25f00d7

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

@jerboaa
Copy link
Contributor

@jerboaa jerboaa commented on 25f00d7 Aug 2, 2021

Choose a reason for hiding this comment

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

/backport jdk11u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 25f00d7 Aug 2, 2021

Choose a reason for hiding this comment

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

@jerboaa could not automatically backport 25f00d78 to openjdk/jdk11u-dev due to conflicts in the following files:

  • src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java

To manually resolve these conflicts run the following commands in your personal fork of openjdk/jdk11u-dev:

$ git checkout -b jerboaa-backport-25f00d78
$ git fetch --no-tags https://git.openjdk.java.net/jdk 25f00d787cf56f6cdca6949115d04e7d8e675554
$ git cherry-pick --no-commit 25f00d787cf56f6cdca6949115d04e7d8e675554
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 25f00d787cf56f6cdca6949115d04e7d8e675554'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk11u-dev with the title Backport 25f00d787cf56f6cdca6949115d04e7d8e675554.

@jerboaa
Copy link
Contributor

@jerboaa jerboaa commented on 25f00d7 Aug 4, 2021

Choose a reason for hiding this comment

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

/backport jdk17u

@openjdk
Copy link

@openjdk openjdk bot commented on 25f00d7 Aug 4, 2021

Choose a reason for hiding this comment

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

@jerboaa the backport was successfully created on the branch jerboaa-backport-25f00d78 in my personal fork of openjdk/jdk17u. To create a pull request with this backport targeting openjdk/jdk17u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

this pull request contains a backport of commit 25f00d78 from the openjdk/jdk repository.

The commit being backported was authored by bobpengxie on 28 Jul 2021 and was reviewed by Severin Gehwolf.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u:

$ git fetch https://github.com/openjdk-bots/jdk17u jerboaa-backport-25f00d78:jerboaa-backport-25f00d78
$ git checkout jerboaa-backport-25f00d78
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u jerboaa-backport-25f00d78

Please sign in to comment.