Re: RFR 4947890 : Minimize JNI upcalls in system property initialization
Hi, On 11/19/18 3:37 PM, Roger Riggs wrote: Raw::xxx_NDX are initialized to 1 + previous_NDX. It's a general good approach to increment the index but I find it error-prone and hard to catch mistake since the (adjacent) variable names look so alike. Perhaps some form of verification or assertion to ensure the indices are correctly initialized. > Added a test that uses reflection to verify the uniqueness and sequence. The test says: * Check that the Raw._*_NDX indexes are sequential and followed by the FIXED_LENGTH value. ^^^ though the code does not verify that FIXED_LENGTH is the final value. I don't know that it's all that important to do, but I thought I'd point it out. Thanks, -Brent
RFR 8148187 : Remove OS X-specific com.apple.concurrent package
Hi, A number of internal APIs were carried over into the JDK with the Apple port. Among them was com.apple.concurrent.Dispatch. Supportedness has always been murky here, but Jigsaw necessitates a firmer stance. Some of these APIs have already been removed from JDK 9 [1], some will be supplanted by new, supported APIs [2]. As already discussed in [3] and [4], com.apple.concurrent.Dispatch is no longer in use, as far as we've been able to find. com.apple.concurrent.Dispatch and its supporting code should be removed from JDK 9. It turns out this opens the door for a little module pruning as well. com.applet.concurrent makes up the bulk of the jdk.deploy.osx module. All that's left is native code for libosx, a library relied on by com.apple.eio.FileManager in the java.desktop module. By moving libosx over to java.desktop, we are able to do away with the jdk.deploy.osx module altogether. For your review is a webrev of this change: http://cr.openjdk.java.net/~bchristi/8148187/webrev.01/ JBS: https://bugs.openjdk.java.net/browse/JDK-8148187 Automated build+test runs look fine. If, in the future, there is desire for an ExecutorService backed by the native platform (as com.apple.concurrent.Dispatch does for libdispatch on OS X), such a feature could be proposed. Thanks, -Brent 1. "Remove apple script engine code in jdk repository" https://bugs.openjdk.java.net/browse/JDK-8143404 2. JEP 272 : "Platform-Specific Desktop Features" https://bugs.openjdk.java.net/browse/JDK-8048731 3. http://mail.openjdk.java.net/pipermail/macosx-port-dev/2015-May/006934.html 4. http://mail.openjdk.java.net/pipermail/macosx-port-dev/2015-September/006968.html
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null
On Fri, 11 Feb 2022 22:37:57 GMT, Mandy Chung wrote: > Thanks for taking on these null caller issue. > > To give more context to this issue, the spec of > `ClassLoader::isRegisteredAsParallelCapable` returns true if this class > loader is registered as parallel capable, otherwise false. The current spec > does not specify what if the caller class is not a class loader. The current > implementation throws NPE if caller is null. I initially proposed to return > false for simplicity. However, if the caller is not a subclass of > `ClassLoader`, the current implementation throws `ClassCastException`. Both > cases are invalid caller and they should be handled in the same way, either > return false or throw an exception. > > Having a second thought, since this API expects to be called by a class > loader, I think throwing `IllegalCallerException` to indicate this method is > called by an illegal caller. This will need a CSR due to the spec change. > > What do you think? Throwing IllegalCallerException sounds good to me. As a static method, from a language standpoint, the call could appear almost anywhere. But its intended usage is pretty narrow. I think it's worth notifying the developer of a pretty obvious error. Is this method mainly meant to be called from a classloader's static initializer? That might be worth mentioning in the JavaDoc (if we're doing a CSR anyway...). - PR: https://git.openjdk.java.net/jdk/pull/7448
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v7]
On Wed, 16 Feb 2022 21:39:20 GMT, Tim Prinzing wrote: >> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is >> null > > Tim Prinzing has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 10 commits: > > - Merge master > - fix test breakage from rename > - reformat test summary > - reformat test summary > - improve test name and remove experimental test code > - fix copyright date > - More changes from feedback. > >The javadoc is improved to reflect InvalidCallerException is thrown with >a caller that can't be assigned to a ClassLoader as well as a null >caller frame. Added a test IsParallelCapableBadCaller that uses >reflection hackery to create a case where called with an invalid caller >on the call stack. > - Changes from feedback. > >- Copyright dates fixed >- IllegalCallerException thrown for no caller frame, and associated >javadoc changes >- test changed to look for IllegalCallerException thrown. > - Merge branch 'master' into JDK-8281000 > - JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is > null Looks fine. - Marked as reviewed by bchristi (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7448
RFR 8073596 : Add jdk.management.cmm in boot.modules that needs sun.management.spi be exported to it
Hi, Please review a small update to the module config files for a new jdk.management.cmm module. Webrev: http://cr.openjdk.java.net/~bchristi/8073596/webrev/ Bug: https://bugs.openjdk.java.net/browse/JDK-8073596 I also found a needed hook missing from Gensrc-java.management.gmk FWIW, Erik Joelsson and Magnus Ihse Bursie have already seen and approved this build change. (Note that I am not on either of these aliases, so please include me directly in any responses. :) Thanks, -Brent
Re: RFR 8073596 : Add jdk.management.cmm in boot.modules that needs sun.management.spi be exported to it
On 2/28/15 1:18 AM, Alan Bateman wrote: Just a minor comment, we've been keeping the *.modules files in sorted order. Not a big deal but would be good to continue that if you can. Will do. Thanks, Alan -Brent
RFR: 8253497: Core Libs Terminology Refresh
This is part of an effort in the JDK to replace archaic/non-inclusive words with more neutral terms (see JDK-8253315 for details). Here are the changes covering core libraries code and tests. Terms were changed as follows: 1. grandfathered -> legacy 2. blacklist -> filter or reject 3. whitelist -> allow or accept 4. master -> coordinator 5. slave -> worker Addressing similar issues in upstream 3rd party code is out of scope of this PR. Such changes will be picked up from their upstream sources. - Commit messages: - Terminology Cleanup - corelibs terminology refresh - bchristi Changes: https://git.openjdk.java.net/jdk/pull/1771/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1771&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253497 Stats: 82 lines in 15 files changed: 1 ins; 0 del; 81 mod Patch: https://git.openjdk.java.net/jdk/pull/1771.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1771/head:pull/1771 PR: https://git.openjdk.java.net/jdk/pull/1771
Re: RFR: 8253497: Core Libs Terminology Refresh
On Mon, 14 Dec 2020 21:08:35 GMT, Joe Wang wrote: >> This is part of an effort in the JDK to replace archaic/non-inclusive words >> with more neutral terms (see JDK-8253315 for details). >> >> Here are the changes covering core libraries code and tests. Terms were >> changed as follows: >> 1. grandfathered -> legacy >> 2. blacklist -> filter or reject >> 3. whitelist -> allow or accept >> 4. master -> coordinator >> 5. slave -> worker >> >> Addressing similar issues in upstream 3rd party code is out of scope of this >> PR. Such changes will be picked up from their upstream sources. > > src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectorServer.java > line 152: > >> 150: * >> 151: * Care must be taken when defining such a filter, as defining >> 152: * an accept-list too restrictive or a too-wide reject-list may > > would "an allow-list too restrictive or a reject-list too wide" read better? I agree that there is room for improvement here. How about: "...an allow-list too restrictively, or a reject-list too broadly, may..." ? - PR: https://git.openjdk.java.net/jdk/pull/1771
Re: RFR: 8253497: Core Libs Terminology Refresh [v2]
> This is part of an effort in the JDK to replace archaic/non-inclusive words > with more neutral terms (see JDK-8253315 for details). > > Here are the changes covering core libraries code and tests. Terms were > changed as follows: > 1. grandfathered -> legacy > 2. blacklist -> filter or reject > 3. whitelist -> allow or accept > 4. master -> coordinator > 5. slave -> worker > > Addressing similar issues in upstream 3rd party code is out of scope of this > PR. Such changes will be picked up from their upstream sources. Brent Christian has updated the pull request incrementally with one additional commit since the last revision: updates, per code review - Changes: - all: https://git.openjdk.java.net/jdk/pull/1771/files - new: https://git.openjdk.java.net/jdk/pull/1771/files/4efa5d43..29525f05 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1771&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1771&range=00-01 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/1771.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1771/head:pull/1771 PR: https://git.openjdk.java.net/jdk/pull/1771
