Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K [v2]
On Mon, 28 Jun 2021 12:20:38 GMT, Daniel Fuchs wrote: >> This cast is only to tell the compiler which overloaded method to call, and >> I don't think there will be a real cast at runtime. It might look a little >> ugly but extracting it into a variable declaration/definition plus a new >> `initStatic` method seems not worth doing, IMHO. > > Why not simply declare a local variable in the static initializer below? > > > private static final long CURRENT_PID; > private static final boolean ALLOW_ATTACH_SELF; > static { > PrivilegedAction pa = ProcessHandle::current; > @SuppressWarnings("removal") > long pid = AccessController.doPrivileged(pa).pid(); > CURRENT_PID = pid; > String s = VM.getSavedProperty("jdk.attach.allowAttachSelf"); > ALLOW_ATTACH_SELF = "".equals(s) || Boolean.parseBoolean(s); > } I've just pushed a commit with a different fix: private static final long CURRENT_PID = pid(); @SuppressWarnings("removal") private static long pid() { PrivilegedAction pa = () -> ProcessHandle.current(); return AccessController.doPrivileged(pa).pid(); } - PR: https://git.openjdk.java.net/jdk17/pull/152
Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K [v2]
On Fri, 25 Jun 2021 23:40:27 GMT, Weijun Wang wrote: >> More refactoring to limit the scope of `@SuppressWarnings` annotations. >> >> Sometimes I introduce new methods. Please feel free to suggest method names >> you like to use. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > one more I'm going to move this to jdk18. - PR: https://git.openjdk.java.net/jdk17/pull/152
Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K [v2]
On Sat, 26 Jun 2021 23:55:46 GMT, Weijun Wang wrote: >> src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java >> line 53: >> >>> 51: private static final long CURRENT_PID = >>> AccessController.doPrivileged( >>> 52: (PrivilegedAction) >>> ProcessHandle::current).pid(); >>> 53: >> >> The original code separated out the declaration of the PrivilegedAction to >> avoid this cast. If you move the code from the original static initializer >> into a static method that it called from initializer then it might provide >> you with a cleaner way to refactor this. There are several other places in >> this patch that could do with similar cleanup. > > This cast is only to tell the compiler which overloaded method to call, and I > don't think there will be a real cast at runtime. It might look a little ugly > but extracting it into a variable declaration/definition plus a new > `initStatic` method seems not worth doing, IMHO. Why not simply declare a local variable in the static initializer below? private static final long CURRENT_PID; private static final boolean ALLOW_ATTACH_SELF; static { PrivilegedAction pa = ProcessHandle::current; @SuppressWarnings("removal") long pid = AccessController.doPrivileged(pa).pid(); CURRENT_PID = pid; String s = VM.getSavedProperty("jdk.attach.allowAttachSelf"); ALLOW_ATTACH_SELF = "".equals(s) || Boolean.parseBoolean(s); } - PR: https://git.openjdk.java.net/jdk17/pull/152
Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K [v2]
On Sat, 26 Jun 2021 16:53:30 GMT, Alan Bateman wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> one more > > src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java line > 53: > >> 51: private static final long CURRENT_PID = >> AccessController.doPrivileged( >> 52: (PrivilegedAction) >> ProcessHandle::current).pid(); >> 53: > > The original code separated out the declaration of the PrivilegedAction to > avoid this cast. If you move the code from the original static initializer > into a static method that it called from initializer then it might provide > you with a cleaner way to refactor this. There are several other places in > this patch that could do with similar cleanup. This cast is only to tell the compiler which overloaded method to call, and I don't think there will be a real cast at runtime. It might look a little ugly but extracting it into a variable declaration/definition plus a new `initStatic` method seems not worth doing, IMHO. - PR: https://git.openjdk.java.net/jdk17/pull/152
Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K [v2]
On Fri, 25 Jun 2021 23:40:27 GMT, Weijun Wang wrote: >> More refactoring to limit the scope of `@SuppressWarnings` annotations. >> >> Sometimes I introduce new methods. Please feel free to suggest method names >> you like to use. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > one more src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java line 53: > 51: private static final long CURRENT_PID = AccessController.doPrivileged( > 52: (PrivilegedAction) > ProcessHandle::current).pid(); > 53: The original code separated out the declaration of the PrivilegedAction to avoid this cast. If you move the code from the original static initializer into a static method that it called from initializer then it might provide you with a cleaner way to refactor this. There are several other places in this patch that could do with similar cleanup. - PR: https://git.openjdk.java.net/jdk17/pull/152
Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K [v2]
On Fri, 25 Jun 2021 23:40:27 GMT, Weijun Wang wrote: >> More refactoring to limit the scope of `@SuppressWarnings` annotations. >> >> Sometimes I introduce new methods. Please feel free to suggest method names >> you like to use. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > one more Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/152
Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K [v2]
> More refactoring to limit the scope of `@SuppressWarnings` annotations. > > Sometimes I introduce new methods. Please feel free to suggest method names > you like to use. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: one more - Changes: - all: https://git.openjdk.java.net/jdk17/pull/152/files - new: https://git.openjdk.java.net/jdk17/pull/152/files/d8b384df..774eb9da Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17=152=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=152=00-01 Stats: 2 lines in 1 file changed: 1 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk17/pull/152.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/152/head:pull/152 PR: https://git.openjdk.java.net/jdk17/pull/152