Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach() [v2]
On Wed, 11 May 2022 02:43:21 GMT, Ioi Lam wrote: >> The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never >> supported the value `"rw"` since the source code was imported to the openjdk >> repo more than 15 years ago. In fact HotSpot throws >> `IllegalArgumentException` when such a mode is specified. >> >> It's unlikely such a mode will be required for future enhancements. Support >> for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` >> is the only supported mode. >> >> I also cleaned up related code in the JDK and HotSpot. >> >> Testing: >> - Passed tiers 1 and 2 >> - Tiers 3, 4, 5 are in progress > > Ioi Lam has updated the pull request incrementally with one additional commit > since the last revision: > > review comments Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach() [v2]
On Tue, 10 May 2022 19:59:41 GMT, Alan Bateman wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> review comments
>
> src/jdk.internal.jvmstat/share/classes/sun/jvmstat/perfdata/monitor/protocol/file/PerfDataBuffer.java
> line 60:
>
>> 58: FileChannel fc = new RandomAccessFile(f, "r").getChannel();
>> 59: ByteBuffer bb = fc.map(FileChannel.MapMode.READ_ONLY, 0L,
>> (int)fc.size());
>> 60: fc.close(); // doesn't need to remain open
>
> I think you can change this to:
>
>
> try (FileChannel fc = FileChannel.open(f.toPath())) {
> ByteBuffer bb = ...
> createPerfDataBuffer(bb, 0);
> }
Fixed.
-
PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach() [v2]
On Tue, 10 May 2022 21:43:44 GMT, Claes Redestad wrote: >> Ioi Lam has updated the pull request incrementally with one additional >> commit since the last revision: >> >> review comments > > src/hotspot/os/windows/perfMemory_windows.cpp line 1781: > >> 1779: // address space. >> 1780: // >> 1781: void PerfMemory::attach(const char* user, int vmid, > > One line? Fixed > src/hotspot/share/prims/perf.cpp line 84: > >> 82: >> 83: // attach to the PerfData memory region for the specified VM >> 84: PerfMemory::attach(user_utf, vmid, > > One line? Fixed > src/hotspot/share/runtime/perfMemory.hpp line 146: > >> 144: // methods for attaching to and detaching from the PerfData >> 145: // memory segment of another JVM process on the same system. >> 146: static void attach(const char* user, int vmid, > > One line? Fixed > src/jdk.jstatd/share/classes/sun/tools/jstatd/RemoteHostImpl.java line 74: > >> 72: Integer v = lvmid; >> 73: RemoteVm stub = null; >> 74: StringBuilder sb = new StringBuilder(); > > Suggestion: > > String vmidStr = "local://" + lvmid + "@localhost"; Fixed - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach() [v2]
> The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never > supported the value `"rw"` since the source code was imported to the openjdk > repo more than 15 years ago. In fact HotSpot throws > `IllegalArgumentException` when such a mode is specified. > > It's unlikely such a mode will be required for future enhancements. Support > for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` > is the only supported mode. > > I also cleaned up related code in the JDK and HotSpot. > > Testing: > - Passed tiers 1 and 2 > - Tiers 3, 4, 5 are in progress Ioi Lam has updated the pull request incrementally with one additional commit since the last revision: review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8622/files - new: https://git.openjdk.java.net/jdk/pull/8622/files/22c22c30..34a01f71 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8622=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8622=00-01 Stats: 14 lines in 5 files changed: 1 ins; 7 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8622.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8622/head:pull/8622 PR: https://git.openjdk.java.net/jdk/pull/8622
