Integrated: 8283903: GetContainerCpuLoad does not return the correct result in share mode
On Wed, 30 Mar 2022 01:46:20 GMT, xpbob wrote: > ``` >long hostTicks = getHostTotalCpuTicks0(); > int totalCPUs = getHostOnlineCpuCount0(); > int containerCPUs = getAvailableProcessors(); > // scale the total host load to the actual container cpus > hostTicks = hostTicks * containerCPUs / totalCPUs; > > hostTicks=17547615556000 > totalCPUs=96 > containerCPUs=90 > > Calculate the overflow This pull request has now been integrated. Changeset: a625bfdb Author:bobpengxie Committer: Kevin Walls URL: https://git.openjdk.java.net/jdk/commit/a625bfdba45d49bc717bcc9be4112db93b50f50a Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod 8283903: GetContainerCpuLoad does not return the correct result in share mode Reviewed-by: jiefu, sgehwolf, kevinw, dholmes - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283805: [REDO] JDK 19 L10n resource files update - msgdrop 10
On Wed, 30 Mar 2022 00:43:49 GMT, Alisen Chung wrote: > redo of 8280400 I believe `src/jdk.localedata/share/classes/sun/util/resources/ext/CurrencyNames_zh_CN.properties` and `test/jdk/sun/text/resources/LocaleData` have to be adjusted according to the l10n changes. - PR: https://git.openjdk.java.net/jdk/pull/8026
Integrated: 8284015: ProblemList containers/docker/TestJcmd.java on linux-x64
On Wed, 30 Mar 2022 15:09:08 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList containers/docker/TestJcmd.java on linux-x64. This pull request has now been integrated. Changeset: d9d19e96 Author:Daniel D. Daugherty URL: https://git.openjdk.java.net/jdk/commit/d9d19e96b1c453f2e52c83de61f593810a5ed1e3 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8284015: ProblemList containers/docker/TestJcmd.java on linux-x64 Reviewed-by: bpb, hseigel - PR: https://git.openjdk.java.net/jdk/pull/8042
Integrated: 8284015: ProblemList containers/docker/TestJcmd.java on linux-x64
A trivial fix to ProblemList containers/docker/TestJcmd.java on linux-x64. - Commit messages: - 8284015: ProblemList containers/docker/TestJcmd.java on linux-x64 Changes: https://git.openjdk.java.net/jdk/pull/8042/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8042=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284015 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8042.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8042/head:pull/8042 PR: https://git.openjdk.java.net/jdk/pull/8042
Re: Integrated: 8284015: ProblemList containers/docker/TestJcmd.java on linux-x64
On Wed, 30 Mar 2022 15:11:36 GMT, Brian Burkhalter wrote: >> A trivial fix to ProblemList containers/docker/TestJcmd.java on linux-x64. > > Marked as reviewed by bpb (Reviewer). @bplb and @hseigel - Thanks for the fast reviews! - PR: https://git.openjdk.java.net/jdk/pull/8042
Re: Integrated: 8284015: ProblemList containers/docker/TestJcmd.java on linux-x64
On Wed, 30 Mar 2022 15:09:08 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList containers/docker/TestJcmd.java on linux-x64. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8042
Re: Integrated: 8284015: ProblemList containers/docker/TestJcmd.java on linux-x64
On Wed, 30 Mar 2022 15:09:08 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList containers/docker/TestJcmd.java on linux-x64. LGTM and is trivial. - Marked as reviewed by hseigel (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8042
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode
On Wed, 30 Mar 2022 03:17:23 GMT, Jie Fu wrote: >>> > In share mode, the process runs for a long time and the number of >>> > physical machine cores is large, making it easier to reappear. >>> >>> So if we run long enough, the `getHostTotalCpuTicks0()` may return >>> overflowed `hostTicks`, right? >> >> GetHostTotalCpuTicks0 is correct >> (GetHostTotalCpuTicks0() * containerCPUs) will overflowed > >> GetHostTotalCpuTicks0 is correct >> (GetHostTotalCpuTicks0() * containerCPUs) will overflowed > > Okay. > It makes sense for this case. > > Maybe, there is no way to prevent the overflow of `hostTicks ` returned by > `getHostTotalCpuTicks0()`. > So the change looks good to me. > Thanks. Thanks for the reviews @DamonFool @dholmes-ora @jerboaa @kevinjwalls - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v4]
On Wed, 30 Mar 2022 11:02:18 GMT, xpbob wrote: >> ``` >>long hostTicks = getHostTotalCpuTicks0(); >> int totalCPUs = getHostOnlineCpuCount0(); >> int containerCPUs = getAvailableProcessors(); >> // scale the total host load to the actual container cpus >> hostTicks = hostTicks * containerCPUs / totalCPUs; >> >> hostTicks=17547615556000 >> totalCPUs=96 >> containerCPUs=90 >> >> Calculate the overflow > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > Change the expression That works too. :) - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v4]
On Wed, 30 Mar 2022 11:02:18 GMT, xpbob wrote: >> ``` >>long hostTicks = getHostTotalCpuTicks0(); >> int totalCPUs = getHostOnlineCpuCount0(); >> int containerCPUs = getAvailableProcessors(); >> // scale the total host load to the actual container cpus >> hostTicks = hostTicks * containerCPUs / totalCPUs; >> >> hostTicks=17547615556000 >> totalCPUs=96 >> containerCPUs=90 >> >> Calculate the overflow > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > Change the expression Marked as reviewed by kevinw (Committer). OK thanks for the description update, and I think the updated code is better also. I guess the app has not been running with the fix long enough to test it fully yet? But if it's looking good so far and reporting good information, and any other tests are unaffected, then looks good. - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v4]
On Wed, 30 Mar 2022 11:02:18 GMT, xpbob wrote: >> ``` >>long hostTicks = getHostTotalCpuTicks0(); >> int totalCPUs = getHostOnlineCpuCount0(); >> int containerCPUs = getAvailableProcessors(); >> // scale the total host load to the actual container cpus >> hostTicks = hostTicks * containerCPUs / totalCPUs; >> >> hostTicks=17547615556000 >> totalCPUs=96 >> containerCPUs=90 >> >> Calculate the overflow > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > Change the expression Thanks. This seems easier to read and shows the intent more clearly. - Marked as reviewed by sgehwolf (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283805: [REDO] JDK 19 L10n resource files update - msgdrop 10
On Wed, 30 Mar 2022 00:43:49 GMT, Alisen Chung wrote: > redo of 8280400 Since this is identical to the original fix, I would expect the same Tier2 test failure as reported in [JDK-8283804](https://bugs.openjdk.java.net/browse/JDK-8283804). - PR: https://git.openjdk.java.net/jdk/pull/8026
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v3]
On Wed, 30 Mar 2022 09:20:06 GMT, Severin Gehwolf wrote: >> Hi, >> Could the bug description be updated to state what the problem is, and under >> what circumstances is there a problem? > >> Hi, Could the bug description be updated to state what the problem is, and >> under what circumstances is there a problem? > > +1 @jerboaa Thanks It is more accurate to explicitly convert data types - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v4]
> ``` >long hostTicks = getHostTotalCpuTicks0(); > int totalCPUs = getHostOnlineCpuCount0(); > int containerCPUs = getAvailableProcessors(); > // scale the total host load to the actual container cpus > hostTicks = hostTicks * containerCPUs / totalCPUs; > > hostTicks=17547615556000 > totalCPUs=96 > containerCPUs=90 > > Calculate the overflow xpbob has updated the pull request incrementally with one additional commit since the last revision: Change the expression - Changes: - all: https://git.openjdk.java.net/jdk/pull/8028/files - new: https://git.openjdk.java.net/jdk/pull/8028/files/a1fda8a4..96184502 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8028=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8028=02-03 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8028.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8028/head:pull/8028 PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v3]
On Wed, 30 Mar 2022 09:49:47 GMT, xpbob wrote: >> src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java >> line 96: >> >>> 94: int containerCPUs = getAvailableProcessors(); >>> 95: // scale the total host load to the actual container >>> cpus >>> 96: hostTicks = (long) (hostTicks * (1.0 * containerCPUs / >>> totalCPUs)); >> >> The intent is to ensure floating point arithmetic on the second expression, >> right? A clearer way to express this may be the following. This avoids the >> `1.0` constant: >> >> >> hostTicks = (long) (hostTicks * ((double) containerCPUs)/ totalCPUs) > >> The intent is to ensure floating point arithmetic on the second expression, >> right? A clearer way to express this may be the following. This avoids the >> `1.0` constant: >> >> ``` >> hostTicks = (long) (hostTicks * ((double) containerCPUs)/ totalCPUs) >> ``` > > 1.0 used to convert the data type to floating point and compute the division > first Right. It seems a strange way to achieve this. Maybe consider this instead? double scaleFactor = ((double)containerCPUs)/totalCPUs; hostTicks = (long)(hostTicks * scaleFactor); - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v3]
On Wed, 30 Mar 2022 09:17:03 GMT, Kevin Walls wrote: >> xpbob has updated the pull request incrementally with one additional commit >> since the last revision: >> >> Change the expression > > Hi, > Could the bug description be updated to state what the problem is, and under > what circumstances is there a problem? @kevinjwalls @jerboaa Thanks to review. I updated bug descriptions, runtime data, and reproduction methods - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v3]
On Wed, 30 Mar 2022 09:04:25 GMT, Severin Gehwolf wrote: > The intent is to ensure floating point arithmetic on the second expression, > right? A clearer way to express this may be the following. This avoids the > `1.0` constant: > > ``` > hostTicks = (long) (hostTicks * ((double) containerCPUs)/ totalCPUs) > ``` 1.0 used to convert the data type to floating point and compute the division first - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v3]
On Wed, 30 Mar 2022 09:17:03 GMT, Kevin Walls wrote: > Hi, Could the bug description be updated to state what the problem is, and > under what circumstances is there a problem? +1 - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v3]
On Wed, 30 Mar 2022 06:20:19 GMT, xpbob wrote: >> ``` >>long hostTicks = getHostTotalCpuTicks0(); >> int totalCPUs = getHostOnlineCpuCount0(); >> int containerCPUs = getAvailableProcessors(); >> // scale the total host load to the actual container cpus >> hostTicks = hostTicks * containerCPUs / totalCPUs; >> >> hostTicks=17547615556000 >> totalCPUs=96 >> containerCPUs=90 >> >> Calculate the overflow > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > Change the expression Hi, Could the bug description be updated to state what the problem is, and under what circumstances is there a problem? - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v3]
On Wed, 30 Mar 2022 06:20:19 GMT, xpbob wrote: >> ``` >>long hostTicks = getHostTotalCpuTicks0(); >> int totalCPUs = getHostOnlineCpuCount0(); >> int containerCPUs = getAvailableProcessors(); >> // scale the total host load to the actual container cpus >> hostTicks = hostTicks * containerCPUs / totalCPUs; >> >> hostTicks=17547615556000 >> totalCPUs=96 >> containerCPUs=90 >> >> Calculate the overflow > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > Change the expression src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java line 96: > 94: int containerCPUs = getAvailableProcessors(); > 95: // scale the total host load to the actual container cpus > 96: hostTicks = (long) (hostTicks * (1.0 * containerCPUs / > totalCPUs)); The intent is to ensure floating point arithmetic on the second expression, right? A clearer way to express this may be the following. This avoids the `1.0` constant: hostTicks = (long) (hostTicks * ((double) containerCPUs)/ totalCPUs) - PR: https://git.openjdk.java.net/jdk/pull/8028
Integrated: 8283800: Simplify String.indexOf/lastIndexOf calls
On Sun, 20 Mar 2022 12:45:34 GMT, Andrey Turbanov wrote: > In a few places String.indexOf/lastIndexOf methods are called with default > parameter for index: `0` for `indexOf`, length() for `lastIndexOf`. > I propose to cleanup such calls. It makes code a bit easier to read. In case > of `indexOf` it even could be faster, as there is separate intrinsic for > `indexOf` call without index argument. This pull request has now been integrated. Changeset: 9bb916db Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/9bb916db0a6af2c6476c0bfbc55c1bb8721b4354 Stats: 14 lines in 6 files changed: 0 ins; 0 del; 14 mod 8283800: Simplify String.indexOf/lastIndexOf calls Reviewed-by: xuelei, bpb, lmesnik - PR: https://git.openjdk.java.net/jdk/pull/7877
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v2]
On Wed, 30 Mar 2022 04:40:07 GMT, David Holmes wrote: >> xpbob has updated the pull request incrementally with one additional commit >> since the last revision: >> >> update copyright year > > 17547615556000 host ticks at 1ns/tick => 5.5 years! > > This is a theoretical overflow - right? > > David @dholmes-ora Thanks to review. This code expression is more accurate - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v3]
> ``` >long hostTicks = getHostTotalCpuTicks0(); > int totalCPUs = getHostOnlineCpuCount0(); > int containerCPUs = getAvailableProcessors(); > // scale the total host load to the actual container cpus > hostTicks = hostTicks * containerCPUs / totalCPUs; > > hostTicks=17547615556000 > totalCPUs=96 > containerCPUs=90 > > Calculate the overflow xpbob has updated the pull request incrementally with one additional commit since the last revision: Change the expression - Changes: - all: https://git.openjdk.java.net/jdk/pull/8028/files - new: https://git.openjdk.java.net/jdk/pull/8028/files/e59d894d..a1fda8a4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8028=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8028=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8028.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8028/head:pull/8028 PR: https://git.openjdk.java.net/jdk/pull/8028
