Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes
On 4/18/20 12:47 AM, Chris Plummer wrote: It's a link to https://download.java.net/java/early_access/jdk15/docs/specs/jni/types.html#type-signatures. This is how the current JVM TI spec defines. Since it looks like this reference was present before your changes, I guess it's ok to leave it, but like JDWP maybe a bug should be filed to clean it up. Right, they are existing reference. I'll let Serguei to file a JBS issue to follow up. Regarding your JNI spec reference, it say "The JNI uses the Java VM’s representation of type signatures" and then has a table called "Java VM Type Signatures". No where in the JNI spec do you see "JNI Signature" or "JNI Type Signature". It seems we should always just use "Type Signature"? I agree that the svc specs should be examined and use the terminologies consistently. Even the JVMTI spec is not consistent. It has a couple of references to JNI Type Signature as links to the JNI spec, but also says "type signatures" in a couple of places, including for GetLocalVariableTable() which refers the the JVMS: "The local variable's type signature, encoded as a modified UTF-8 string. The signature format is the same as that defined in The Java™ Virtual Machine Specification, Chapter 4.3.2. " Mandy
Re: RFR 15 8243099: Adding ADQ support to JDK
On 18/04/2020 00:13, Ivanov, Vladimir A wrote: Hello everyone, I would like to add support for the Application Device Queue (ADQ) technology to the JDK. The JBS entry and webrev were created for this improvement: JBS: https://bugs.openjdk.java.net/browse/JDK-8243099 Webrev: http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.00/ Please let us know your feedback and comments for this enhancement. Just to put more context on this. Vladimir brought this to nio-dev in March with some background and initial proposed it as a standard socket option [1]. A socket option that allows an application to split incoming flows based on their receive queue is very Linux specific so we suggested it should be in the JDK-specific ExtendedSocketOptions API with the other non-standard socket options. net-dev is the right mailing list to progress this and get a sponsor (maybe Sandhya Viswanathan will do it?). We can probably drop core-libs-dev from the this one. Just a few comments on the current webrev: The API docs in the webrev is a good start but will need to be fleshed out as it uses several terms such as "device queue", "last frame", "Rx queue" without any definition. One important point that we will need to be covered is that it's a read-only socket option so the docs will need to specify the exception that will be thrown when trying to set it. From a quick look at the implementation then it appears to fall through to InternalError so that might need to be fixed. It will also be important to document the socket types and where in their lifecycle (listening, connected) that the socket option can be used. Will SO_INCOMING_NAPI_ID be defined in a header file? It probably needs #ifndef to avoid the hardcoded value when building on distributions that define it. A passing comment is that native methods static methods so "unused" is a jclass. Tests. Read-only socket options might trip up on existing tests in the jdk_net or jdk_nio test groups that enumerate all socket options and exercise get and set on each one. The proposed test will probably go several iterations to get to something that is maintainable. It will also need to be extended to exercise SocketChannel, ServerSocketChannel and more. -Alan [1] https://mail.openjdk.java.net/pipermail/nio-dev/2020-March/007109.html
Re: Type-parameterized complement to Object.equals(Object)
Without commenting on the merits of the proposal, let me point out that asking for a sponsor at this point is skipping many important steps. I would recommend you focus first on *building consensus* that (a) there is a problem, (b) you have identified the correct problem, (c) it is a problem that needs to be solved, (d) it is problem that needs to be solved in the JDK, (e) that you have evaluated the possible solutions and analyzed their strengths and weaknesses, and (f) that the correct solution has been identified. (There are more steps, but you get the idea.) > On Apr 17, 2020, at 7:44 AM, dmytro sheyko > wrote: > > I need a sponsor for following JDK change.
Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java
Sundar - do you recognize this? I assume truncating this test to 0 bytes was a mistake but I can't tell if the test should be brought back or not (to ensure that`jlink --order-resources` is tested). -Alan On 17/04/2020 11:34, Ao Qi wrote: Hi, The original email is waiting for moderator approval. Subscribed to jigsaw-dev and resent the RFR (cc'ed core-libs-dev). Thanks, Ao Qi On Wed, Apr 15, 2020 at 10:11 PM Ao Qi wrote: Hi all, test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java was changed to an empty file by JDK-8162538, but not removed. I think we should remve this file. JBS: https://bugs.openjdk.java.net/browse/JDK-8242846 webrev: http://cr.openjdk.java.net/~aoqi/8242846/webrev.00/ Thanks, Ao Qi
Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes
Hi Mandy, Comments inline. On 4/17/20 4:52 PM, Mandy Chung wrote: On 4/17/20 3:51 PM, Chris Plummer wrote: Hi Mandy, Thanks for updating the svc specs. Some comments below: In the JDWP spec update, you changed "JNI signature" to "type signature" in one place, but left it as "JNI signature" everywhere else. Should they all be changed? JDWP signature is changed because there is no JNI signature representing a hidden class. I leave other references to JNI signature as is since those only apply for normal classes as I wanted to keep this change specifically due to hidden classes. I think it's good to file a JBS issue to follow up on JDWP and JDI to determine if the spec should be upgraded to reference the new TypeDescriptor API. In the JDWP spec for ClassLoaderReference.VisibleClasses: "That is, this class loader has been recorded as an initiating loader of the returned classes." -> "That is, all classes for which this class loader has been recorded as an initiating loader." This seems like too much detail to be put here. Basically the term "initiating ClassLoader" has turned into a short essay. Is it possible that all this detail could be put elsewhere and referenced? Any suggestion? We attempted to place those description in JVM TI Class section or ClassLoad event. However, that's not ideal place since that's needed by JDWP, JDI and Instrumentation. I found inlining this description is not ideal but it provides adequate clarification. See other thread. Aren't there other places in other specs where a similar clarification of "initiating ClassLoader" is needed (I see now that ClassLoaderClasses in the JVMTI spec, ClassLoaderReference,visibleClasses in the JDI spec, and Instrumentation.getInitiatedClasses are all dealing with this, but not all in the exact same way). I took the conservative side and make sure the clarification is in place for all APIs. I'm open to any suggestion for example having JDWP and JDI to link to JVM TI spec if you think appropriate. In the JVMTI spec for GetLoadedClasses: This suffers in a way similar to ClassLoaderReference.VisibleClasses in the JDWP spec, although not as badly. A simple concept ends up with a complex description, and it seems that description should really be in a more centralized place. I would also suggest a bit of cleanup of these lines: 6866 An array class is created directly by Java virtual machine. An array class 6867 creation can be triggered by using class loaders or by invoking methods in certain 6868 Java SE Platform API such as reflection. "Created by [the] Java virtual machine" (add "the") Change "An array class creation" to "The creation" since your are repeating "An array class" from the previous sentence. In the JVMTI spec ClassLoaderClasses section: "That is, initiating_loader has been recorded as an initiating loader of the returned classes." -> "That is, all classes for which initiating_loader has been recorded as an initiating loader." In the JVMTI spec GetClassSignature section: "If the class indicated by klass is ..." -> "If the class ..." (you just finished the previous sentence with "class indicated by Klass"). "the returned name is [the] JNI type signature" (add "the"). Also, is "JNI type signature" formally defined somewhere? This relates to my JDWP spec comment above. It's a link to https://download.java.net/java/early_access/jdk15/docs/specs/jni/types.html#type-signatures. This is how the current JVM TI spec defines. Since it looks like this reference was present before your changes, I guess it's ok to leave it, but like JDWP maybe a bug should be filed to clean it up. Regarding your JNI spec reference, it say "The JNI uses the Java VM’s representation of type signatures" and then has a table called "Java VM Type Signatures". No where in the JNI spec do you see "JNI Signature" or "JNI Type Signature". It seems we should always just use "Type Signature"? Even the JVMTI spec is not consistent. It has a couple of references to JNI Type Signature as links to the JNI spec, but also says "type signatures" in a couple of places, including for GetLocalVariableTable() which refers the the JVMS: "The local variable's type signature, encoded as a modified UTF-8 string. The signature format is the same as that defined in The Java™ Virtual Machine Specification, Chapter 4.3.2. " " where N is the binary name encoded in internal form indicated by the class file". Is "binary name encoded in internal form" explained somewhere? JVMS 4.2.1 https://docs.oracle.com/javase/specs/jvms/se14/html/jvms-4.html#jvms-4.2.1 Thanks. I see you've also added a reference in your edits below. Also, can you add an example of a returned hidden class signature? OK In the JVMTI spec ClassLoad section: "representation using [a] class loader" (add "a") "By invoking Lookup::defineHiddenClass, that creates ..." -> "By
Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes
On 4/17/20 9:37 PM, serguei.spit...@oracle.com wrote: On 4/17/20 16:52, Mandy Chung wrote: On 4/17/20 3:51 PM, Chris Plummer wrote: Hi Mandy, Thanks for updating the svc specs. Some comments below: In the JDWP spec update, you changed "JNI signature" to "type signature" in one place, but left it as "JNI signature" everywhere else. Should they all be changed? JDWP signature is changed because there is no JNI signature representing a hidden class. I leave other references to JNI signature as is since those only apply for normal classes as I wanted to keep this change specifically due to hidden classes. I think it's good to file a JBS issue to follow up on JDWP and JDI to determine if the spec should be upgraded to reference the new TypeDescriptor API. In the JDWP spec for ClassLoaderReference.VisibleClasses: "That is, this class loader has been recorded as an initiating loader of the returned classes." -> "That is, all classes for which this class loader has been recorded as an initiating loader." This seems like too much detail to be put here. Basically the term "initiating ClassLoader" has turned into a short essay. Is it possible that all this detail could be put elsewhere and referenced? Any suggestion? We attempted to place those description in JVM TI Class section or ClassLoad event. However, that's not ideal place since that's needed by JDWP, JDI and Instrumentation. I found inlining this description is not ideal but it provides adequate clarification. The JDI (transitively via JDWP), JDWP and Instrumentation implementations are based on the JVMTI. I've tried to suggest once to link these API's to the JVMTI. The problem is there was no such practice in the specs of these API's before but we can make a step to introduce it now. Placing this description either in JVM TI Class section or ClassLoad event would be good enough. An alternative approach is to make JVMTI/JDI/JDWP/Instrument to refer to the java.lang.Class spec for general information about class loading and classes defined and initiated by class loaders. I'd first like to clarify on my comment above just in case there was any confusion. At the last minute I re-ordered the two paragraph and now I see it makes it seem like I was only referring to to the one sentence about initiating loaders. I was referring to pretty much all the changes that were made in this section and other similar APIs in the other specs. Yes. I'd like to see all this as part of the Class/Classloading spec in some sort of section that gives an overview of all these topics, but mostly clarifies what an initiating loader is, and the (non) relationship to hidden classes. thanks, Chris Thanks, Serguei Aren't there other places in other specs where a similar clarification of "initiating ClassLoader" is needed (I see now that ClassLoaderClasses in the JVMTI spec, ClassLoaderReference,visibleClasses in the JDI spec, and Instrumentation.getInitiatedClasses are all dealing with this, but not all in the exact same way). I took the conservative side and make sure the clarification is in place for all APIs. I'm open to any suggestion for example having JDWP and JDI to link to JVM TI spec if you think appropriate. In the JVMTI spec for GetLoadedClasses: This suffers in a way similar to ClassLoaderReference.VisibleClasses in the JDWP spec, although not as badly. A simple concept ends up with a complex description, and it seems that description should really be in a more centralized place. I would also suggest a bit of cleanup of these lines: 6866 An array class is created directly by Java virtual machine. An array class 6867 creation can be triggered by using class loaders or by invoking methods in certain 6868 Java SE Platform API such as reflection. "Created by [the] Java virtual machine" (add "the") Change "An array class creation" to "The creation" since your are repeating "An array class" from the previous sentence. In the JVMTI spec ClassLoaderClasses section: "That is, initiating_loader has been recorded as an initiating loader of the returned classes." -> "That is, all classes for which initiating_loader has been recorded as an initiating loader." In the JVMTI spec GetClassSignature section: "If the class indicated by klass is ..." -> "If the class ..." (you just finished the previous sentence with "class indicated by Klass"). "the returned name is [the] JNI type signature" (add "the"). Also, is "JNI type signature" formally defined somewhere? This relates to my JDWP spec comment above. It's a link to https://download.java.net/java/early_access/jdk15/docs/specs/jni/types.html#type-signatures. This is how the current JVM TI spec defines. " where N is the binary name encoded in internal form indicated by the class file". Is "binary name encoded in internal form" explained somewhere? JVMS 4.2.1
