Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.
/ jdk18u Public archive

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
Backport-of: e07fd395bdc314867886a621ec76cf74a5f76b89
  • Loading branch information
iklam committed Mar 31, 2022
1 parent 9f39b4b commit a541111
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 @@ -495,7 +495,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 a541111

@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

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 a541111 Jun 29, 2022

Choose a reason for hiding this comment

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

@MBaesken the backport was successfully created on the branch MBaesken-backport-a5411119 in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev: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 a5411119 from the openjdk/jdk18u repository.

The commit being backported was authored by Ioi Lam on 31 Mar 2022 and was reviewed by David Holmes and 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-dev:

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

Please sign in to comment.