Re: RFR: 8255381: com/sun/jdi/EATests.java should not suspend graal threads [v3]
On Thu, 10 Dec 2020 02:08:38 GMT, Serguei Spitsyn wrote: >> Richard Reingruber has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Only main thread needs to be resumed in >> EARelockingObjectCurrentlyWaitingOn. > > Hi Richard, > The fix looks okay to me. > Thanks, > Serguei Hi Serguei, thanks a lot for reviewing this. Richard. - PR: https://git.openjdk.java.net/jdk/pull/1625
Re: RFR: 8253797: [cgroups v2] Account for the fact that swap accounting is disabled on some systems
On Mon, 7 Dec 2020 17:48:01 GMT, Severin Gehwolf wrote: > This has been implemented for cgroups v1 with > [JDK-8250984](https://bugs.openjdk.java.net/browse/JDK-8250984) but was > lacking some tooling support for cgroups v2. With podman 2.2.0 release this > could now be implemented (and tested). The idea is the same as for the > cgroups v1 fix. If we've got no swap limit capabilities, return the memory > limit only. > > Note that for cgroups v2 doesn't implement CgroupV1Metrics (obviously) and, > thus, doesn't have `getMemoryAndSwapFailCount()` and > `getMemoryAndSwapMaxUsage()`. > > Testing: > - [x] submit testing > - [x] container tests on cgroups v2 with swapaccount=0. > - [x] Manual container tests involving `-XshowSettings:system` on cgroups v2. > > Thoughts? The changes look good. Thanks for doing this. Harold - Marked as reviewed by hseigel (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1672
Re: RFR: 8253797: [cgroups v2] Account for the fact that swap accounting is disabled on some systems
On Thu, 10 Dec 2020 16:28:59 GMT, Harold Seigel wrote: > The changes look good. Thanks for doing this. Thanks for the review! - PR: https://git.openjdk.java.net/jdk/pull/1672
Integrated: 8253797: [cgroups v2] Account for the fact that swap accounting is disabled on some systems
On Mon, 7 Dec 2020 17:48:01 GMT, Severin Gehwolf wrote: > This has been implemented for cgroups v1 with > [JDK-8250984](https://bugs.openjdk.java.net/browse/JDK-8250984) but was > lacking some tooling support for cgroups v2. With podman 2.2.0 release this > could now be implemented (and tested). The idea is the same as for the > cgroups v1 fix. If we've got no swap limit capabilities, return the memory > limit only. > > Note that for cgroups v2 doesn't implement CgroupV1Metrics (obviously) and, > thus, doesn't have `getMemoryAndSwapFailCount()` and > `getMemoryAndSwapMaxUsage()`. > > Testing: > - [x] submit testing > - [x] container tests on cgroups v2 with swapaccount=0. > - [x] Manual container tests involving `-XshowSettings:system` on cgroups v2. > > Thoughts? This pull request has now been integrated. Changeset: 66936111 Author:Severin Gehwolf URL: https://git.openjdk.java.net/jdk/commit/66936111 Stats: 45 lines in 2 files changed: 24 ins; 1 del; 20 mod 8253797: [cgroups v2] Account for the fact that swap accounting is disabled on some systems Reviewed-by: hseigel - PR: https://git.openjdk.java.net/jdk/pull/1672
Integrated: 8256424: Move ciSymbol::symbol_name() to ciSymbols::symbol_name()
On Tue, 17 Nov 2020 12:53:48 GMT, Claes Redestad wrote: > This moves the mirroring of vmSymbols in ciSymbols to a separate file, > ciSymbols.hpp, to reduce includes throughout hotspot (and clean up the > ciSymbol namespace). In a few places code is moved from .hpp to .cpp to > facilitate this. > > With PCH disabled, this reduces total includes from 276949 to 276332 This pull request has now been integrated. Changeset: f5740561 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/f5740561 Stats: 183 lines in 32 files changed: 102 ins; 30 del; 51 mod 8256424: Move ciSymbol::symbol_name() to ciSymbols::symbol_name() Reviewed-by: kvn, iklam - PR: https://git.openjdk.java.net/jdk/pull/1256
Re: RFR: 8165276: Spec states to invoke the premain method in an agent class if it's public but implementation differs [v2]
On Thu, 10 Dec 2020 00:37:14 GMT, Serguei Spitsyn wrote:
>> This change have been already reviewed by Mandy, Sundar, Alan and David.
>> Please, see the jdk 15 review thread:
>>
>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2020-June/031998.html
>>
>> Now, the PR approval is needed.
>> The push was postponed because the CSR was not approved at that time (it is
>> now):
>>https://bugs.openjdk.java.net/browse/JDK-8248189
>> Investigation of existing popular java agents was requested by Joe.
>> --
>>
>> The java.lang.instrument spec:
>>
>> https://docs.oracle.com/en/java/javase/15/docs/api/java.instrument/java/lang/instrument/package-summary.html
>>
>> Summary:
>> The java.lang.instrument spec clearly states:
>> "The agent class must implement a public static premain method
>> similar in principle to the main application entry point."
>> Current implementation of sun/instrument/InstrumentationImpl.java
>> allows the premain method be non-public which violates the spec.
>> This fix aligns the implementation with the spec.
>>
>> Testing:
>> A mach5 run of jdk_instrument tests is in progress.
>
> Serguei Spitsyn has updated the pull request incrementally with one
> additional commit since the last revision:
>
> removed lookup for inherited premain/agentmain methods
Please update @bug in the tests to include this bug ID.
All InheritAgentXXX tests are updated to have the InheritAgentXXX class public.
However, I think you need to go through them individually for this behavioral
change. The existing comments may not be applicable and please update them
where appropriate.For example InheritAgent0100 previously verifies the
invocation of the premain in its superclass. Does the modified test ensure
that this fails to load? Per the comment in the new InheritAgent0100Super
class, it expects the superclass' premain should be called.
src/java.instrument/share/classes/sun/instrument/InstrumentationImpl.java line
499:
> 497: // reject non-public premain or agentmain method
> 498: if (!m.canAccess(null)) {
> 499: String msg = "method " + classname + "." + methodname + "
> must be declared public";
The comments in line 429-443 need to be updated.
-
PR: https://git.openjdk.java.net/jdk/pull/1694
