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: dholmes, sgehwolf
  • Loading branch information
@iklam
iklam committed Mar 4, 2022
1 parent 9c817d3 commit e07fd39
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 18 deletions.
7 changes: 6 additions & 1 deletion src/hotspot/os/linux/cgroupSubsystem_linux.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,12 @@ 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.
// UseContainerCpuShares and PreferContainerQuotaForCPUCount are
// deprecated and will be removed in the next JDK release.
int share = UseContainerCpuShares ? cpu_shares() : -1;

if (quota > -1 && period > 0) {
quota_count = ceilf((float)quota / (float)period);
Expand Down
12 changes: 8 additions & 4 deletions src/hotspot/os/linux/globals_linux.hpp
View file
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,10 +59,14 @@
product(bool, UseContainerSupport, true, \
"Enable detection and runtime container configuration support") \
\
product(bool, UseContainerCpuShares, false, \
"(Deprecated) 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" \
" shares value, provided it is less than quota.") \
"(Deprecated) Calculate the container CPU availability based" \
" on the value of quotas (if set), when true. Otherwise, use" \
" the CPU shares value, provided it is less than quota.") \
\
product(bool, AdjustStackSizeForTLS, false, \
"Increase the thread stack size to include space for glibc " \
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/runtime/arguments.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,8 @@ static SpecialFlag const special_jvm_flags[] = {
{ "UseHeavyMonitors", JDK_Version::jdk(18), JDK_Version::jdk(19), JDK_Version::jdk(20) },
#endif
{ "ExtendedDTraceProbes", JDK_Version::jdk(19), JDK_Version::jdk(20), JDK_Version::jdk(21) },
{ "UseContainerCpuShares", JDK_Version::jdk(19), JDK_Version::jdk(20), JDK_Version::jdk(21) },
{ "PreferContainerQuotaForCPUCount", JDK_Version::jdk(19), JDK_Version::jdk(20), JDK_Version::jdk(21) },

// --- Deprecated alias flags (see also aliased_jvm_flags) - sorted by obsolete_in then expired_in:
{ "DefaultMaxRAMFraction", JDK_Version::jdk(8), JDK_Version::undefined(), JDK_Version::undefined() },
Expand Down
4 changes: 2 additions & 2 deletions test/hotspot/jtreg/containers/cgroup/PlainRead.java
View file
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
43 changes: 32 additions & 11 deletions test/hotspot/jtreg/containers/docker/TestCPUAwareness.java
View file
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 deprecated -XX:+UseContainerCpuShares flag, which
// will be removed in the next 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 (!usePreferContainerQuotaForCPUCount) opts.addJavaOpts("-XX:-PreferContainerQuotaForCPUCount");
if (useContainerCpuShares) opts.addJavaOpts("-XX:+UseContainerCpuShares"); // deprecated
if (!usePreferContainerQuotaForCPUCount) opts.addJavaOpts("-XX:-PreferContainerQuotaForCPUCount"); // deprecated

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"); // deprecated
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
2 changes: 2 additions & 0 deletions test/hotspot/jtreg/runtime/CommandLine/VMDeprecatedOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ public class VMDeprecatedOptions {
{"InitialRAMFraction", "64"},
{"TLABStats", "false"},
{"AllowRedefinitionToAddDeleteMethods", "true"},
{"UseContainerCpuShares", "false"},
{"PreferContainerQuotaForCPUCount", "true"},

// deprecated alias flags (see also aliased_jvm_flags):
{"DefaultMaxRAMFraction", "4"},
Expand Down

3 comments on commit e07fd39

@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

@MBaesken
Copy link
Member

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on e07fd39 Jun 29, 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 e07fd395 to openjdk/jdk17u-dev due to conflicts in the following files:

  • src/hotspot/share/runtime/arguments.cpp
  • test/hotspot/jtreg/runtime/CommandLine/VMDeprecatedOptions.java

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

$ git checkout -b MBaesken-backport-e07fd395
$ git fetch --no-tags https://git.openjdk.org/jdk e07fd395bdc314867886a621ec76cf74a5f76b89
$ git cherry-pick --no-commit e07fd395bdc314867886a621ec76cf74a5f76b89
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport e07fd395bdc314867886a621ec76cf74a5f76b89'

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

Please sign in to comment.