Skip to content

Commit

Permalink
8281181: Do not use CPU Shares to compute active processor count
Browse files Browse the repository at this point in the history
Reviewed-by: sgehwolf
Backport-of: a5411119c383225e9be27311c6cb7fe5d1700b68
  • Loading branch information
MBaesken committed Jul 1, 2022
1 parent 180f3d9 commit 437c634
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 15 deletions.
7 changes: 5 additions & 2 deletions src/hotspot/os/linux/cgroupSubsystem_linux.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -501,7 +501,10 @@ int CgroupSubsystem::active_processor_count() {
cpu_count = limit_count = os::Linux::active_processor_count();
int quota = cpu_quota();
int period = cpu_period();
int share = cpu_shares();

// It's not a good idea to use cpu_shares() to limit the number
// of CPUs used by the JVM. See JDK-8281181.
int share = UseContainerCpuShares ? cpu_shares() : -1;

if (quota > -1 && period > 0) {
quota_count = ceilf((float)quota / (float)period);
Expand Down
6 changes: 5 additions & 1 deletion src/hotspot/os/linux/globals_linux.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -59,6 +59,10 @@
product(bool, UseContainerSupport, true, \
"Enable detection and runtime container configuration support") \
\
product(bool, UseContainerCpuShares, false, \
"Include CPU shares in the CPU availability" \
" calculation.") \
\
product(bool, PreferContainerQuotaForCPUCount, true, \
"Calculate the container CPU availability based on the value" \
" of quotas (if set), when true. Otherwise, use the CPU" \
Expand Down
4 changes: 2 additions & 2 deletions test/hotspot/jtreg/containers/cgroup/PlainRead.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -50,7 +50,7 @@ static public void noMatch(OutputAnalyzer oa, String what, String value) {
static final String good_value = "(\\d+|-1|-2|Unlimited)";
static final String bad_value = "(failed)";

static final String[] variables = {"Memory Limit is:", "CPU Shares is:", "CPU Quota is:", "CPU Period is:", "active_processor_count:"};
static final String[] variables = {"Memory Limit is:", "CPU Quota is:", "CPU Period is:", "active_processor_count:"};

static public void isContainer(OutputAnalyzer oa) {
for (String v: variables) {
Expand Down
41 changes: 31 additions & 10 deletions test/hotspot/jtreg/containers/docker/TestCPUAwareness.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -100,6 +100,11 @@ private static void testComboWithCpuSets() throws Exception {
String cpuSetStr = CPUSetsReader.readFromProcStatus("Cpus_allowed_list");
System.out.println("cpuSetStr = " + cpuSetStr);

// OLD = use the -XX:+UseContainerCpuShares flag, which
// may be removed in a future JDK release. See JDK-8281181.
boolean OLD = true;
boolean NEW = false;

if (cpuSetStr == null) {
System.out.printf("The cpuset test cases are skipped");
} else {
Expand All @@ -108,23 +113,32 @@ private static void testComboWithCpuSets() throws Exception {
// Test subset of cpuset with one element
if (cpuSet.size() >= 1) {
String testCpuSet = CPUSetsReader.listToString(cpuSet, 1);
testAPCCombo(testCpuSet, 200*1000, 100*1000, 4*1024, true, 1);
testAPCCombo(OLD, testCpuSet, 200*1000, 100*1000, 4*1024, true, 1);
testAPCCombo(NEW, testCpuSet, 200*1000, 100*1000, 4*1024, true, 1);
}

// Test subset of cpuset with two elements
if (cpuSet.size() >= 2) {
String testCpuSet = CPUSetsReader.listToString(cpuSet, 2);
testAPCCombo(testCpuSet, 200*1000, 100*1000, 4*1024, true, 2);
testAPCCombo(testCpuSet, 200*1000, 100*1000, 1023, true, 2);
testAPCCombo(testCpuSet, 200*1000, 100*1000, 1023, false, 1);
testAPCCombo(OLD, testCpuSet, 200*1000, 100*1000, 4*1024, true, 2);
testAPCCombo(OLD, testCpuSet, 200*1000, 100*1000, 1023, true, 2);
testAPCCombo(OLD, testCpuSet, 200*1000, 100*1000, 1023, false,1);

testAPCCombo(NEW, testCpuSet, 200*1000, 100*1000, 4*1024, true, 2);
testAPCCombo(NEW, testCpuSet, 200*1000, 100*1000, 1023, true, 2);
testAPCCombo(NEW, testCpuSet, 200*1000, 100*1000, 1023, false,2);
}

// Test subset of cpuset with three elements
if (cpuSet.size() >= 3) {
String testCpuSet = CPUSetsReader.listToString(cpuSet, 3);
testAPCCombo(testCpuSet, 100*1000, 100*1000, 2*1024, true, 1);
testAPCCombo(testCpuSet, 200*1000, 100*1000, 1023, true, 2);
testAPCCombo(testCpuSet, 200*1000, 100*1000, 1023, false, 1);
testAPCCombo(OLD, testCpuSet, 100*1000, 100*1000, 2*1024, true, 1);
testAPCCombo(OLD, testCpuSet, 200*1000, 100*1000, 1023, true, 2);
testAPCCombo(OLD, testCpuSet, 200*1000, 100*1000, 1023, false,1);

testAPCCombo(NEW, testCpuSet, 100*1000, 100*1000, 2*1024, true, 1);
testAPCCombo(NEW, testCpuSet, 200*1000, 100*1000, 1023, true, 2);
testAPCCombo(NEW, testCpuSet, 200*1000, 100*1000, 1023, false,2);
}
}
}
Expand Down Expand Up @@ -181,15 +195,19 @@ private static void testCpuQuotaAndPeriod(int quota, int period)
}


// Test correctess of automatically selected active processor cound
private static void testAPCCombo(String cpuset, int quota, int period, int shares,
// Test correctess of automatically selected active processor count
// Note: when -XX:+UseContainerCpuShares is removed,
// useContainerCpuShares, shares, and usePreferContainerQuotaForCPUCount
// should also be removed.
private static void testAPCCombo(boolean useContainerCpuShares, String cpuset, int quota, int period, int shares,
boolean usePreferContainerQuotaForCPUCount,
int expectedAPC) throws Exception {
Common.logNewTestCase("test APC Combo");
System.out.println("cpuset = " + cpuset);
System.out.println("quota = " + quota);
System.out.println("period = " + period);
System.out.println("shares = " + shares);
System.out.println("useContainerCpuShares = " + useContainerCpuShares);
System.out.println("usePreferContainerQuotaForCPUCount = " + usePreferContainerQuotaForCPUCount);
System.out.println("expectedAPC = " + expectedAPC);

Expand All @@ -201,13 +219,15 @@ private static void testAPCCombo(String cpuset, int quota, int period, int share
.addDockerOpts("--cpu-quota=" + quota)
.addDockerOpts("--cpu-shares=" + shares);

if (useContainerCpuShares) opts.addJavaOpts("-XX:+UseContainerCpuShares");
if (!usePreferContainerQuotaForCPUCount) opts.addJavaOpts("-XX:-PreferContainerQuotaForCPUCount");

Common.run(opts)
.shouldMatch("active_processor_count.*" + expectedAPC);
}


// Note: when -XX:+UseContainerCpuShares is removed, this test should also be removed.
private static void testCpuShares(int shares, int expectedAPC) throws Exception {
Common.logNewTestCase("test cpu shares, shares = " + shares);
System.out.println("expectedAPC = " + expectedAPC);
Expand All @@ -216,6 +236,7 @@ private static void testCpuShares(int shares, int expectedAPC) throws Exception

DockerRunOptions opts = Common.newOpts(imageName)
.addDockerOpts("--cpu-shares=" + shares);
opts.addJavaOpts("-XX:+UseContainerCpuShares");
OutputAnalyzer out = Common.run(opts);
// Cgroups v2 needs to do some scaling of raw shares values. Hence,
// 256 CPU shares come back as 264. Raw value written to cpu.weight
Expand Down

3 comments on commit 437c634

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@MBaesken
Copy link
Member Author

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 437c634 Jul 1, 2022

Choose a reason for hiding this comment

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

@MBaesken Could not automatically backport 437c634b to openjdk/jdk11u-dev due to conflicts in the following files:

  • src/hotspot/os/linux/globals_linux.hpp
  • test/hotspot/jtreg/containers/cgroup/PlainRead.java
  • test/hotspot/jtreg/containers/docker/TestCPUAwareness.java

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

$ git checkout -b MBaesken-backport-437c634b
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev 437c634b61c277e6af6efd243399178dd44d7126
$ git cherry-pick --no-commit 437c634b61c277e6af6efd243399178dd44d7126
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 437c634b61c277e6af6efd243399178dd44d7126'

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

Please sign in to comment.