Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode [v2]
On Wed, 30 Mar 2022 02:08:16 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: > > update copyright year Marked as reviewed by jiefu (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode
On Wed, 30 Mar 2022 02:44:52 GMT, xpbob wrote: > 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. - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283903: GetContainerCpuLoad does not return the correct result in share mode
On Wed, 30 Mar 2022 02:29:47 GMT, xpbob 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? - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 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 It would be better to describe how to reproduce the bug. Is it possible to add a jtreg test for this case? Please also update the copyright year. - PR: https://git.openjdk.java.net/jdk/pull/8028
Re: RFR: 8283728: jdk.hotspot.agent: Wrong location for RISCV64ThreadContext.java
On Sun, 27 Mar 2022 08:59:40 GMT, Christoph Langer wrote: > Fix risv64 -> riscv64 Looks reasonable to me. - Marked as reviewed by jiefu (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7977
Re: RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize [v2]
On Wed, 23 Mar 2022 09:27:07 GMT, Severin Gehwolf wrote: >> Please review this container test improvement. The test in question only >> makes sense iff the total swap space size as reported by the container aware >> OperatingSystemMXBean is `0`. If that's not the case, then something else >> might be amiss, e.g. OperatingSystemMXBean reporting the host swap limits. >> In such a case a passing test tells us nothing. Certainly not if the >> fix from [JDK-8242480](https://bugs.openjdk.java.net/browse/JDK-8242480) is >> present or not. >> >> Testing: >> - [x] Manual with and without the code fix of JDK-8242480. Still passes with >> the fix, and fails without. Tested the test on cgroups v1 and cgroups v2. > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Incorporate review feedback > - Merge branch 'master' into test_getswapspace_improvement_8283279 > - 8283279: [Testbug] Improve TestGetSwapSpaceSize LGTM Thanks for the update. - Marked as reviewed by jiefu (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7854
Re: RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize
On Thu, 17 Mar 2022 13:40:53 GMT, Severin Gehwolf wrote:
> Please review this container test improvement. The test in question only
> makes sense iff the total swap space size as reported by the container aware
> OperatingSystemMXBean is `0`. If that's not the case, then something else
> might be amiss, e.g. OperatingSystemMXBean reporting the host swap limits. In
> such a case a passing test tells us nothing. Certainly not if the
> fix from [JDK-8242480](https://bugs.openjdk.java.net/browse/JDK-8242480) is
> present or not.
>
> Testing:
> - [x] Manual with and without the code fix of JDK-8242480. Still passes with
> the fix, and fails without. Tested the test on cgroups v1 and cgroups v2.
Please also update the copyright year.
Thanks.
test/jdk/jdk/internal/platform/docker/GetFreeSwapSpaceSize.java line 28:
> 26:
> 27: // Usage:
> 28: // GetFreeSwapSpaceSize
>
I would suggest
// GetFreeSwapSpaceSize
test/jdk/jdk/internal/platform/docker/GetFreeSwapSpaceSize.java line 32:
> 30: public static void main(String[] args) {
> 31: if (args.length != 4) {
> 32: throw new RuntimeException("Unexpected arguments. Expected 2,
> got " + args.length);
Shouldn't be `Expected 4` ?
-
PR: https://git.openjdk.java.net/jdk/pull/7854
Re: RFR: 8276139: TestJpsHostName.java not reliable, better to expand HostIdentifierCreate.java test
On Fri, 29 Oct 2021 11:29:32 GMT, Kevin Walls wrote: > Remove a specific test which invokes jps on a fictional process, and can > timeout and fail. > Instead, add to the patterns used in > open/test/jdk/sun/jvmstat/monitor/HostIdentifier/HostIdentifierCreate.java > which will perform the same test, that we can create a HostIdentifier using > numeric values. By the way, you will also backport it to jdk11u, right? - PR: https://git.openjdk.java.net/jdk/pull/6170
Re: RFR: 8276139: TestJpsHostName.java not reliable, better to expand HostIdentifierCreate.java test
On Fri, 29 Oct 2021 11:29:32 GMT, Kevin Walls wrote: > Remove a specific test which invokes jps on a fictional process, and can > timeout and fail. > Instead, add to the patterns used in > open/test/jdk/sun/jvmstat/monitor/HostIdentifier/HostIdentifierCreate.java > which will perform the same test, that we can create a HostIdentifier using > numeric values. LGTM Thanks for your explanation. - Marked as reviewed by jiefu (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6170
Re: RFR: 8276139: TestJpsHostName.java not reliable, better to expand HostIdentifierCreate.java test
On Fri, 29 Oct 2021 11:29:32 GMT, Kevin Walls wrote: > Remove a specific test which invokes jps on a fictional process, and can > timeout and fail. > Instead, add to the patterns used in > open/test/jdk/sun/jvmstat/monitor/HostIdentifier/HostIdentifierCreate.java > which will perform the same test, that we can create a HostIdentifier using > numeric values. TestJpsHostName.java works fine on our test machines. Do you have any idea why it gets timeout on your machines? Or how can I reproduce the timeout situation? Thanks. - PR: https://git.openjdk.java.net/jdk/pull/6170
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package [v2]
On Sat, 31 Jul 2021 20:42:10 GMT, Igor Ignatyev wrote: >> Hi all, >> >> could you please review this big tedious and trivial(-ish) patch which moves >> `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? >> >> the majority of the patch is the following substitutions: >> - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` >> - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` >> - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` >> - `s/sun.hotspot.code/jdk.test.whitebox.code/g` >> - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` >> - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` >> >> testing: tier1-4 >> >> Thanks, >> -- Igor > > Igor Ignatyev has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains 12 new > commits since the last revision: > > - fixed ctw build > - updated runtime/cds/appcds/JarBuilder to copy j.t.w.WhiteBox's inner class > - updated requires.VMProps > - updated TEST.ROOT > - adjusted hotspot source > - added test > - moved and adjusted WhiteBox tests (test/lib-test/sun/hotspot/whitebox) > - updated ClassFileInstaller to copy j.t.w.WhiteBox's inner class > - removed sun/hotspot/parser/DiagnosticCommand > - deprecated sun/hotspot classes >disallowed s.h.WhiteBox w/ security manager > - ... and 2 more: > https://git.openjdk.java.net/jdk17/compare/8f12f2cf...237e8860 Shall we also update the copyright year like test/lib/sun/hotspot/cpuinfo/CPUInfo.java? Thanks. - PR: https://git.openjdk.java.net/jdk17/pull/290
Re: RFR: 8270901: Typo PHASE_CPP in CompilerPhaseType [v2]
On Wed, 28 Jul 2021 08:06:56 GMT, Yi Yang wrote: >> Should be PHASE_CCP instead of PHASE_CPP, it also affects dumped ideal graph >> XML. > > Yi Yang has updated the pull request incrementally with one additional commit > since the last revision: > > update copyright year For hotspot changes, it would be better to have a second review & wait for 24 hours before integration. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/4917
Re: RFR: 8270901: Typo PHASE_CPP in CompilerPhaseType
On Wed, 28 Jul 2021 07:19:08 GMT, Yi Yang wrote: > Should be PHASE_CCP instead of PHASE_CPP, it also affects dumped ideal graph > XML. Looks good to me. The copyright year should also be updated. Thanks. - Marked as reviewed by jiefu (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4917
Re: RFR: 8269851: OperatingSystemMXBean getProcessCpuLoad reports incorrect process cpu usage in containers [v8]
On Wed, 14 Jul 2021 04:00:39 GMT, xpbob wrote: >> …ocess cpu usage in containers > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > Add comments Will sponsor it tomorrow if there is no objection. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/4702
Re: RFR: 8270814: ProblemList the failing serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor* tests
On Fri, 16 Jul 2021 06:02:05 GMT, David Holmes wrote: > Two tests have started failing regularly in our CI testing, and also in GHA, > so I'm ProblemListing them until the issue can be resolved. > > Thanks, > David Marked as reviewed by jiefu (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4804
Re: RFR: 8260878: com/sun/jdi/JdbOptions.java fails without jfr
On Tue, 2 Feb 2021 20:46:08 GMT, Alex Menkov wrote: >> Hi all, >> >> com/sun/jdi/JdbOptions.java fails in our ci/cd when jfr is disabled. >> It would be better to fix it. >> >> Thanks. >> Best regards, >> Jie > > Marked as reviewed by amenkov (Reviewer). Thanks @alexmenkov and @plummercj . - PR: https://git.openjdk.java.net/jdk/pull/2346
Integrated: 8260878: com/sun/jdi/JdbOptions.java fails without jfr
On Tue, 2 Feb 2021 03:43:29 GMT, Jie Fu wrote: > Hi all, > > com/sun/jdi/JdbOptions.java fails in our ci/cd when jfr is disabled. > It would be better to fix it. > > Thanks. > Best regards, > Jie This pull request has now been integrated. Changeset: a47befc8 Author:Jie Fu URL: https://git.openjdk.java.net/jdk/commit/a47befc8 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod 8260878: com/sun/jdi/JdbOptions.java fails without jfr Reviewed-by: amenkov, cjplummer - PR: https://git.openjdk.java.net/jdk/pull/2346
Re: RFR: 8260878: com/sun/jdi/JdbOptions.java fails without jfr
On Tue, 2 Feb 2021 04:09:24 GMT, Chris Plummer wrote:
>> Hi all,
>>
>> com/sun/jdi/JdbOptions.java fails in our ci/cd when jfr is disabled.
>> It would be better to fix it.
>>
>> Thanks.
>> Best regards,
>> Jie
>
> test/jdk/com/sun/jdi/JdbOptions.java line 99:
>
>> 97: test("-connect",
>> 98:
>> "com.sun.jdi.CommandLineLaunch:vmexec=java,options=\"-client\"
>> \"-XX:+PrintVMOptions\""
>> 99: + " -XX:+IgnoreUnrecognizedVMOptions"
>
> You need to be careful when using `-xx:+IgnoreUnrecognizedVMOptions`, because
> it can result in not detecting typos in the option names, which could happen
> with this test in the future if any other mods are made. Other options here
> are to skip this part of the test if JFR is not present, or better yet don't
> make the test rely on JFR. I don't think there is a reason that it needs to.
> It seems other options could have been used for the testing. Perhaps
> @alexmenkov can comment on why it was done this way.
Thanks @plummercj for your review.
Let's wait for @alexmenkov 's comments.
Thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/2346
RFR: 8260878: com/sun/jdi/JdbOptions.java fails without jfr
Hi all, com/sun/jdi/JdbOptions.java fails in our ci/cd when jfr is disabled. It would be better to fix it. Thanks. Best regards, Jie - Commit messages: - 8260878: com/sun/jdi/JdbOptions.java fails without jfr Changes: https://git.openjdk.java.net/jdk/pull/2346/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2346=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8260878 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2346.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2346/head:pull/2346 PR: https://git.openjdk.java.net/jdk/pull/2346
