Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Mon, 15 Feb 2021 17:59:54 GMT, Vladimir Kempik wrote: >> src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 810: >> >>> 808: #ifdef __APPLE__ >>> 809: // Less-than word types are stored one after another. >>> 810: // The code unable to handle this, bailout. >> >> Perhaps: // The code is unable to handle this so bailout. > > Hello, we have updated PR, now this bailout is used only by the code which > can handle it (native wrapper generator), for the rest it will cause > guarantee failed if this bailout is triggered I've fixed the spelling. Sorry for it to take so long :( >> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 195: >> >>> 193: frame os::get_sender_for_C_frame(frame* fr) { >>> 194: return frame(fr->link(), fr->link(), fr->sender_pc()); >>> 195: } >> >> Is this file going to be built by GCC or just macOS compilers? > > there is no support for compiling java with gcc on macos since about jdk11, > only clang. > considering this and the absence of gcc for macos_m1, the answer is - just > macOS compilers. I've fixed the comment. Now it states `JVM compiled with -fno-omit-frame-pointer, so RFP is saved on the stack.` - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Mon, 1 Mar 2021 10:46:34 GMT, Andrew Haley wrote: >> They are defined in 13.2.95. MIDR_EL1, Main ID Register. Apple's code is not >> there, but "Arm can assign codes that are not published in this manual. All >> values not assigned by Arm are reserved and must not be used.". I assume the >> value was obtained by digging around >> https://github.com/apple/darwin-xnu/blob/main/osfmk/arm/cpuid.h#L62 > > Anton, this paragraph looks like an excellent comment. Thanks, I've added the comment. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v22]
> Please review the implementation of JEP 391: macOS/AArch64 Port. > > It's heavily based on existing ports to linux/aarch64, macos/x86_64, and > windows/aarch64. > > Major changes are in: > * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks > JDK-8253817, JDK-8253818) > * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with necessary > adjustments (JDK-8253819) > * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), > required on macOS/AArch64 platform. It's implemented with > pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a > thread, so W^X mode change relates to the java thread state change (for java > threads). In most cases, JVM executes in write-only mode, except when calling > a generated stub like SafeFetch, which requires a temporary switch to > execute-only mode. The same execute-only mode is enabled when a java thread > executes in java or native states. This approach of managing W^X mode turned > out to be simple and efficient enough. > * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) Anton Kozlov has updated the pull request incrementally with one additional commit since the last revision: Update comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/663cb4a1..e42b82db Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=21 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=20-21 Stats: 4 lines in 2 files changed: 4 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v5]
> This rewrites the doc of ArraysSupport.newLength, adds detail to the > exception message, and adds a test. In addition to some renaming and a bit of > refactoring of the actual code, I also made two changes of substance to the > code: > > 1. I fixed a problem with overflow checking. In the original code, if > oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this > method could return a negative value. It turns out that writing tests helps > find bugs! > > 2. Under the old policy, if oldLength and minGrowth required a length above > SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would > return Integer.MAX_VALUE. That doesn't make any sense, because attempting to > allocate an array of that length will almost certainly cause the Hotspot to > throw OOME because its implementation limit was exceeded. Instead, if the > required length is in this range, this method returns that required length. > > Separately, I'll work on retrofitting various call sites around the JDK to > use this method. Stuart Marks has updated the pull request incrementally with one additional commit since the last revision: Update copyright year. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1617/files - new: https://git.openjdk.java.net/jdk/pull/1617/files/c520e8e8..8892eb47 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1617=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1617=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1617.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1617/head:pull/1617 PR: https://git.openjdk.java.net/jdk/pull/1617
Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v4]
> This rewrites the doc of ArraysSupport.newLength, adds detail to the > exception message, and adds a test. In addition to some renaming and a bit of > refactoring of the actual code, I also made two changes of substance to the > code: > > 1. I fixed a problem with overflow checking. In the original code, if > oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this > method could return a negative value. It turns out that writing tests helps > find bugs! > > 2. Under the old policy, if oldLength and minGrowth required a length above > SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would > return Integer.MAX_VALUE. That doesn't make any sense, because attempting to > allocate an array of that length will almost certainly cause the Hotspot to > throw OOME because its implementation limit was exceeded. Instead, if the > required length is in this range, this method returns that required length. > > Separately, I'll work on retrofitting various call sites around the JDK to > use this method. Stuart Marks has updated the pull request incrementally with one additional commit since the last revision: Add newline at end of file. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1617/files - new: https://git.openjdk.java.net/jdk/pull/1617/files/6710004c..c520e8e8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1617=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1617=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1617.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1617/head:pull/1617 PR: https://git.openjdk.java.net/jdk/pull/1617
Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v3]
> This rewrites the doc of ArraysSupport.newLength, adds detail to the > exception message, and adds a test. In addition to some renaming and a bit of > refactoring of the actual code, I also made two changes of substance to the > code: > > 1. I fixed a problem with overflow checking. In the original code, if > oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this > method could return a negative value. It turns out that writing tests helps > find bugs! > > 2. Under the old policy, if oldLength and minGrowth required a length above > SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would > return Integer.MAX_VALUE. That doesn't make any sense, because attempting to > allocate an array of that length will almost certainly cause the Hotspot to > throw OOME because its implementation limit was exceeded. Instead, if the > required length is in this range, this method returns that required length. > > Separately, I'll work on retrofitting various call sites around the JDK to > use this method. Stuart Marks has updated the pull request incrementally with one additional commit since the last revision: Update with recommendations from Martin Buchholz. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1617/files - new: https://git.openjdk.java.net/jdk/pull/1617/files/bacb5f91..6710004c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1617=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1617=01-02 Stats: 12 lines in 2 files changed: 1 ins; 4 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/1617.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1617/head:pull/1617 PR: https://git.openjdk.java.net/jdk/pull/1617
Integrated: 8261670: Add javadoc for the XML processing limits
On Thu, 25 Feb 2021 22:04:41 GMT, Joe Wang wrote: > Add the documentation for XML processing limits to module summary. The limits > were previously documented in Java tutorial and guide. This pull request has now been integrated. Changeset: 6635d7a5 Author:Joe Wang URL: https://git.openjdk.java.net/jdk/commit/6635d7a5 Stats: 92 lines in 1 file changed: 83 ins; 0 del; 9 mod 8261670: Add javadoc for the XML processing limits Reviewed-by: lancea, naoto, iris - PR: https://git.openjdk.java.net/jdk/pull/2732
Re: RFR: 8261670: Add javadoc for the XML processing limits [v4]
On Tue, 2 Mar 2021 00:19:22 GMT, Naoto Sato wrote: >> Joe Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Change the description of the property table to indicate that the list of >> the properties is not exhaustive > > Marked as reviewed by naoto (Reviewer). Thanks all for the review. - PR: https://git.openjdk.java.net/jdk/pull/2732
Re: RFR: JDK-8261518: jpackage looks for main module in current dir when there is no module-path
On Mon, 1 Mar 2021 15:55:39 GMT, Andy Herrick wrote: > when the app modules have already been jlinked with the runtime, and there is > no need for module-path, jpackage was acting as if the module-path was "." > and picking up jars in the current directory. Marked as reviewed by almatvee (Committer). - PR: https://git.openjdk.java.net/jdk/pull/2781
Re: Inconsistency in Constructor.getGenericParameterTypes()
Hi Remi, As package level statement would be helpful as long as it was linked to from specific types and methods (as java.lang.Class and java.lang.Package are part of core reflection but not the java.lang.reflect package). As you state, core reflection is *mostly* a view of what is represented in the class file. However, the class file in general does not present sufficient information in all cases to recover a source-file view of the world even as an option. For example, as a compiler-internal contract javac had added two additional parameters to the private constructors of enums. Other Java compilers are required to do this and javac could, in principle, change this implementation detail at any time. There is no MANDATED or SYNTHETIC bit (see javax.lang.model.util.Elements.Origin) that can be used to reliably tease apart what is going on. The package-level specs of javax.lang.model.element discuss analogous issue for the language model API. In any case, I've filed JDK-8262807: Note assumptions of core reflection modeling and parameter handling for core reflection. Thanks, -Joe On 3/1/2021 2:16 PM, Remi Forax wrote: Hi Joe, i think the overview of the package java.lang.reflect should discuss the fact that the reflection view is what is stored in the classfile, not exactly a reflection of the java code. So depending on what you are requesting, you can see synthetic parameters generated by javac or only the Java view because the Java view is directly stored in an attributes like with generics Rémi - Mail original - De: "joe darcy" À: "Oliver Drotbohm" Cc: "core-libs-dev" Envoyé: Lundi 1 Mars 2021 22:35:59 Objet: Re: Inconsistency in Constructor.getGenericParameterTypes() Hi Oliver, Perhaps the time has come to make a run at discussing this situation in the javadoc. One challenge in writing this material up is to phrase and structure the text so it offers a net-clarification of the situation. In other words, to not distract or confuse most readers on what is usually a tricky detail. The @apiNote javadoc tag offers a mechanism to separate such discussion. Thanks, -Joe On 2/26/2021 2:20 PM, Oliver Drotbohm wrote: Hi Joe, thanks for the explanation. We switched to rather iterating over ….getParameters() and take it from there. Do you think it makes sense to leave a note about this in the Javadoc? Cheers, Ollie Am 26.02.2021 um 22:38 schrieb Joe Darcy : Hello Oliver, This is long-standing if surprising and under-documented behavior. The getGenericFoo methods, when generic type information is present, give a source-level view of the element. At a source level, the implicit outer this parameter is not present and thus omitted by constructor.getGenericParameterTypes for the constructor in question. HTH, -Joe On 2/26/2021 5:41 AM, Oliver Drotbohm wrote: Previously sent to the wrong list. Sorry for the double post. Von: Oliver Drotbohm Betreff: Inconsistency in Constructor.getGenericParameterTypes() Datum: 25. Februar 2021 um 10:03:12 MEZ An: jdk-...@openjdk.java.net Hi all, we've just ran into the following issue: for a non-static, generic inner class with a constructor declaring a generic parameter, a call to constructor.getGenericParameterTypes() does not return the enclosing class parameter type. Is that by intention? If so, what's the reasoning behind that? Here's a the output of a reproducer (below): static class StaticGeneric - names: value, string static class StaticGeneric - parameters: [class java.lang.Object, class java.lang.String] static class StaticGeneric - generic parameters: [T, class java.lang.String] class NonStaticGeneric - names: this$0, value, String class NonStaticGeneric - parameters: [class Sample, class java.lang.Object, class java.lang.String] class NonStaticGeneric - generic parameters: [T, class java.lang.String] class NonStaticNonGeneric - names: this$0, String class NonStaticNonGeneric - parameters: [class Sample, class java.lang.String] class NonStaticNonGeneric - generic parameters: [class Sample, class java.lang.String] Note how the constructor of the NonStaticGeneric type exposes three parameter names, three parameter types but omits the enclosing class parameter in the list of generic parameter types. Tested on JDK 8 to 15. Same behavior. Cheers, Ollie class Sample { public static void main(String[] args) { Constructor first = StaticGeneric.class.getDeclaredConstructors()[0]; System.out.println("static class StaticGeneric - names: " + Arrays.stream(first.getParameters()).map(Parameter::getName).collect(Collectors.joining(", "))); System.out.println("static class StaticGeneric - parameters: " + Arrays.toString(first.getParameterTypes())); System.out.println( "static class StaticGeneric
Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v5]
On Sun, 28 Feb 2021 13:31:38 GMT, Jie Fu wrote: >> `@requires vm.compiler2.enabled` had been added. >> Thanks. > > @PaulSandoz , are you also OK with the latest version? > Thanks. > @DamonFool I think Vladimir is correct in the layering, in this respect i > think we can make things a littler clearer. This seems like a small thing but > i think its worth making very explicit as there is some hidden complexity. > > What if we add the following method to `VectorShape`: > > ```java > /** > * Returns the maximum vector bit size for a given element type. > * > * @param etype the element type. > * @return the maximum vector bit. > */ > /*package-private*/ > static int getMaxVectorBitSize(Class etype) { > // May return -1 if C2 is not enabled, > // or a value smaller than the S_64_BIT.vectorBitSize / > elementSizeInBits, on say 32-bit platforms > // If so default to S_64_BIT > int maxLaneCount = VectorSupport.getMaxLaneCount(etype); > int elementSizeInBits = LaneType.of(etype).elementSize; > return Math.max(maxLaneCount * elementSizeInBits, > S_64_BIT.vectorBitSize); > } > ``` > > It is package private so it can be tested explicitly if need be. > > Then we can reuse that method: > > ``` > S_Max_BIT(getMaxVectorBitSize(byte.class)); > ``` > > ``` > static VectorShape largestShapeFor(Class etype) { > return VectorShape.forBitSize(getMaxVectorBitSize(etype)); > } > ``` > > I think that's correct, but i have not tested. WDYT? Good suggestion. Updated. Testing: - jdk/incubator/vector with MaxVectorSize=default/8/4 on Linux/x64 - jdk/incubator/vector without C2 on Linux/x64 Thanks. - PR: https://git.openjdk.java.net/jdk/pull/2722
Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v7]
> Hi all, > > Vector API fails to work when: > - case 1: MaxVectorSize is set to <=8, or > - case 2: C2 is disabled > > The reason is that {max/preferred} VectorShape initialization fails in both > cases. > And the root cause is that VectorSupport_GetMaxLaneCount [1] returns > unreasonable values (0 for case 1 and -1 for case 2). > > Vector API should not depend on C2 to run. > It should work even there is no JIT compiler since it's a Java-level api. > So let's fix it. > > Testing: > - jdk/incubator/vector with -XX:MaxVectorSize=default/8 on Linux/x64 > > Thanks. > Best regards, > Jie > > [1] > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/vectorSupport.cpp#L364 Jie Fu has updated the pull request incrementally with one additional commit since the last revision: Add VectorShape.getMaxVectorBitSize - Changes: - all: https://git.openjdk.java.net/jdk/pull/2722/files - new: https://git.openjdk.java.net/jdk/pull/2722/files/79402411..d3b4cc35 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2722=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2722=05-06 Stats: 26 lines in 1 file changed: 16 ins; 6 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2722.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2722/head:pull/2722 PR: https://git.openjdk.java.net/jdk/pull/2722
Re: RFR: 8261670: Add javadoc for the XML processing limits [v4]
On Mon, 1 Mar 2021 23:59:11 GMT, Joe Wang wrote: >> Add the documentation for XML processing limits to module summary. The >> limits were previously documented in Java tutorial and guide. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > Change the description of the property table to indicate that the list of > the properties is not exhaustive Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2732
Re: RFR: 8261670: Add javadoc for the XML processing limits [v4]
On Mon, 1 Mar 2021 23:59:11 GMT, Joe Wang wrote: >> Add the documentation for XML processing limits to module summary. The >> limits were previously documented in Java tutorial and guide. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > Change the description of the property table to indicate that the list of > the properties is not exhaustive Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2732
Re: RFR: 8261670: Add javadoc for the XML processing limits [v4]
On Mon, 1 Mar 2021 23:59:11 GMT, Joe Wang wrote: >> Add the documentation for XML processing limits to module summary. The >> limits were previously documented in Java tutorial and guide. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > Change the description of the property table to indicate that the list of > the properties is not exhaustive Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2732
Re: RFR: 8261670: Add javadoc for the XML processing limits [v4]
> Add the documentation for XML processing limits to module summary. The limits > were previously documented in Java tutorial and guide. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: Change the description of the property table to indicate that the list of the properties is not exhaustive - Changes: - all: https://git.openjdk.java.net/jdk/pull/2732/files - new: https://git.openjdk.java.net/jdk/pull/2732/files/3e555d20..9be83cb5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2732=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2732=02-03 Stats: 6 lines in 1 file changed: 5 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2732.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2732/head:pull/2732 PR: https://git.openjdk.java.net/jdk/pull/2732
Re: Inconsistency in Constructor.getGenericParameterTypes()
Hi Joe, i think the overview of the package java.lang.reflect should discuss the fact that the reflection view is what is stored in the classfile, not exactly a reflection of the java code. So depending on what you are requesting, you can see synthetic parameters generated by javac or only the Java view because the Java view is directly stored in an attributes like with generics Rémi - Mail original - > De: "joe darcy" > À: "Oliver Drotbohm" > Cc: "core-libs-dev" > Envoyé: Lundi 1 Mars 2021 22:35:59 > Objet: Re: Inconsistency in Constructor.getGenericParameterTypes() > Hi Oliver, > > Perhaps the time has come to make a run at discussing this situation in > the javadoc. One challenge in writing this material up is to phrase and > structure the text so it offers a net-clarification of the situation. In > other words, to not distract or confuse most readers on what is usually > a tricky detail. > > The @apiNote javadoc tag offers a mechanism to separate such discussion. > > Thanks, > > -Joe > > On 2/26/2021 2:20 PM, Oliver Drotbohm wrote: >> Hi Joe, >> >> thanks for the explanation. We switched to rather iterating over >> ….getParameters() and take it from there. Do you think it makes sense to >> leave >> a note about this in the Javadoc? >> >> Cheers, >> Ollie >> >>> Am 26.02.2021 um 22:38 schrieb Joe Darcy : >>> >>> Hello Oliver, >>> >>> This is long-standing if surprising and under-documented behavior. >>> >>> The getGenericFoo methods, when generic type information is present, give a >>> source-level view of the element. At a source level, the implicit outer this >>> parameter is not present and thus omitted by >>> constructor.getGenericParameterTypes for the constructor in question. >>> >>> HTH, >>> >>> -Joe >>> >>> On 2/26/2021 5:41 AM, Oliver Drotbohm wrote: Previously sent to the wrong list. Sorry for the double post. Von: Oliver Drotbohm Betreff: Inconsistency in Constructor.getGenericParameterTypes() Datum: 25. Februar 2021 um 10:03:12 MEZ An: jdk-...@openjdk.java.net Hi all, we've just ran into the following issue: for a non-static, generic inner class with a constructor declaring a generic parameter, a call to constructor.getGenericParameterTypes() does not return the enclosing class parameter type. Is that by intention? If so, what's the reasoning behind that? Here's a the output of a reproducer (below): static class StaticGeneric - names: value, string static class StaticGeneric - parameters: [class java.lang.Object, class java.lang.String] static class StaticGeneric - generic parameters: [T, class java.lang.String] class NonStaticGeneric - names: this$0, value, String class NonStaticGeneric - parameters: [class Sample, class java.lang.Object, class java.lang.String] class NonStaticGeneric - generic parameters: [T, class java.lang.String] class NonStaticNonGeneric - names: this$0, String class NonStaticNonGeneric - parameters: [class Sample, class java.lang.String] class NonStaticNonGeneric - generic parameters: [class Sample, class java.lang.String] Note how the constructor of the NonStaticGeneric type exposes three parameter names, three parameter types but omits the enclosing class parameter in the list of generic parameter types. Tested on JDK 8 to 15. Same behavior. Cheers, Ollie class Sample { public static void main(String[] args) { Constructor first = StaticGeneric.class.getDeclaredConstructors()[0]; System.out.println("static class StaticGeneric - names: " + Arrays.stream(first.getParameters()).map(Parameter::getName).collect(Collectors.joining(", "))); System.out.println("static class StaticGeneric - parameters: " + Arrays.toString(first.getParameterTypes())); System.out.println( "static class StaticGeneric - generic parameters: " + Arrays.toString(first.getGenericParameterTypes())); System.out.println(); Constructor second = NonStaticGeneric.class.getDeclaredConstructors()[0]; System.out.println("class NonStaticGeneric - names: " + Arrays.stream(second.getParameters()).map(Parameter::getName).collect(Collectors.joining(", "))); System.out.println("class NonStaticGeneric - parameters: " + Arrays.toString(second.getParameterTypes())); System.out .println(
Re: Inconsistency in Constructor.getGenericParameterTypes()
Hi Oliver, Perhaps the time has come to make a run at discussing this situation in the javadoc. One challenge in writing this material up is to phrase and structure the text so it offers a net-clarification of the situation. In other words, to not distract or confuse most readers on what is usually a tricky detail. The @apiNote javadoc tag offers a mechanism to separate such discussion. Thanks, -Joe On 2/26/2021 2:20 PM, Oliver Drotbohm wrote: Hi Joe, thanks for the explanation. We switched to rather iterating over ….getParameters() and take it from there. Do you think it makes sense to leave a note about this in the Javadoc? Cheers, Ollie Am 26.02.2021 um 22:38 schrieb Joe Darcy : Hello Oliver, This is long-standing if surprising and under-documented behavior. The getGenericFoo methods, when generic type information is present, give a source-level view of the element. At a source level, the implicit outer this parameter is not present and thus omitted by constructor.getGenericParameterTypes for the constructor in question. HTH, -Joe On 2/26/2021 5:41 AM, Oliver Drotbohm wrote: Previously sent to the wrong list. Sorry for the double post. Von: Oliver Drotbohm Betreff: Inconsistency in Constructor.getGenericParameterTypes() Datum: 25. Februar 2021 um 10:03:12 MEZ An: jdk-...@openjdk.java.net Hi all, we've just ran into the following issue: for a non-static, generic inner class with a constructor declaring a generic parameter, a call to constructor.getGenericParameterTypes() does not return the enclosing class parameter type. Is that by intention? If so, what's the reasoning behind that? Here's a the output of a reproducer (below): static class StaticGeneric - names: value, string static class StaticGeneric - parameters: [class java.lang.Object, class java.lang.String] static class StaticGeneric - generic parameters: [T, class java.lang.String] class NonStaticGeneric - names: this$0, value, String class NonStaticGeneric - parameters: [class Sample, class java.lang.Object, class java.lang.String] class NonStaticGeneric - generic parameters: [T, class java.lang.String] class NonStaticNonGeneric - names: this$0, String class NonStaticNonGeneric - parameters: [class Sample, class java.lang.String] class NonStaticNonGeneric - generic parameters: [class Sample, class java.lang.String] Note how the constructor of the NonStaticGeneric type exposes three parameter names, three parameter types but omits the enclosing class parameter in the list of generic parameter types. Tested on JDK 8 to 15. Same behavior. Cheers, Ollie class Sample { public static void main(String[] args) { Constructor first = StaticGeneric.class.getDeclaredConstructors()[0]; System.out.println("static class StaticGeneric - names: " + Arrays.stream(first.getParameters()).map(Parameter::getName).collect(Collectors.joining(", "))); System.out.println("static class StaticGeneric - parameters: " + Arrays.toString(first.getParameterTypes())); System.out.println( "static class StaticGeneric - generic parameters: " + Arrays.toString(first.getGenericParameterTypes())); System.out.println(); Constructor second = NonStaticGeneric.class.getDeclaredConstructors()[0]; System.out.println("class NonStaticGeneric - names: " + Arrays.stream(second.getParameters()).map(Parameter::getName).collect(Collectors.joining(", "))); System.out.println("class NonStaticGeneric - parameters: " + Arrays.toString(second.getParameterTypes())); System.out .println( "class NonStaticGeneric - generic parameters: " + Arrays.toString(second.getGenericParameterTypes())); System.out.println(); Constructor third = NonStaticNonGeneric.class.getDeclaredConstructors()[0]; System.out.println("class NonStaticNonGeneric - names: " + Arrays.stream(third.getParameters()).map(Parameter::getName).collect(Collectors.joining(", "))); System.out.println("class NonStaticNonGeneric - parameters: " + Arrays.toString(third.getParameterTypes())); System.out .println( "class NonStaticNonGeneric - generic parameters: " + Arrays.toString(third.getGenericParameterTypes())); } static class StaticGeneric { StaticGeneric(T value, String string) {} } class NonStaticGeneric { NonStaticGeneric(T value, String String) {} } class NonStaticNonGeneric { NonStaticNonGeneric(String String) {} } }
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v26]
> This PR is to introduce a new random number API for the JDK. The primary API > is found in RandomGenerator and RandomGeneratorFactory. Further description > can be found in the JEP https://openjdk.java.net/jeps/356 . > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html > > old PR: https://github.com/openjdk/jdk/pull/1273 Jim Laskey has updated the pull request incrementally with one additional commit since the last revision: Update javadoc - Changes: - all: https://git.openjdk.java.net/jdk/pull/1292/files - new: https://git.openjdk.java.net/jdk/pull/1292/files/b9094279..99e92dd1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1292=25 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1292=24-25 Stats: 460 lines in 7 files changed: 183 ins; 85 del; 192 mod Patch: https://git.openjdk.java.net/jdk/pull/1292.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292 PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8260869: Test java/foreign/TestHandshake.java fails intermittently [v2]
On Mon, 1 Mar 2021 19:30:00 GMT, Maurizio Cimadamore wrote: >> This simple fix reduces the amount of concurrency on the foreign memory >> TestHandshake test. As this test spins a new accessor thread for each >> available processors, on machines which feature an high number of available >> processors (because of multi-threading), and which are slower in forking new >> threads (e.g. Windows), the test can sometimes timeout. >> >> The sensible step, for now, is to introduce an hard limit on the number of >> concurrent accessor threads being created. I've looked at the test logs >> quite in depth, and there's nothing suggesting that something else is amiss >> - the number of concurrent threads just seem to be too high in some >> instances. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments Marked as reviewed by psandoz (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2748
Integrated: JDK-8261839: Error creating runtime package on macos without mac-package-identifier
On Thu, 25 Feb 2021 21:15:44 GMT, Andy Herrick wrote: > …age-identifier This pull request has now been integrated. Changeset: 642f45f9 Author:Andy Herrick URL: https://git.openjdk.java.net/jdk/commit/642f45f9 Stats: 24 lines in 4 files changed: 8 ins; 5 del; 11 mod 8261839: Error creating runtime package on macos without mac-package-identifier Reviewed-by: asemenyuk, almatvee, kizune - PR: https://git.openjdk.java.net/jdk/pull/2730
Re: RFR: 8260869: Test java/foreign/TestHandshake.java fails intermittently [v2]
On Fri, 26 Feb 2021 17:56:21 GMT, Paul Sandoz wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address review comments > > test/jdk/java/foreign/TestHandshake.java line 60: > >> 58: >> 59: static final int NUM_ACCESSORS = Math.max(10, >> Runtime.getRuntime().availableProcessors()); >> 60: > > `min` rather than `max` so as to clamp at a maximum of 10 accessors? Whoops - of course :-) - PR: https://git.openjdk.java.net/jdk/pull/2748
Re: RFR: 8260869: Test java/foreign/TestHandshake.java fails intermittently [v2]
> This simple fix reduces the amount of concurrency on the foreign memory > TestHandshake test. As this test spins a new accessor thread for each > available processors, on machines which feature an high number of available > processors (because of multi-threading), and which are slower in forking new > threads (e.g. Windows), the test can sometimes timeout. > > The sensible step, for now, is to introduce an hard limit on the number of > concurrent accessor threads being created. I've looked at the test logs quite > in depth, and there's nothing suggesting that something else is amiss - the > number of concurrent threads just seem to be too high in some instances. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/2748/files - new: https://git.openjdk.java.net/jdk/pull/2748/files/aa71dee6..df3346f5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2748=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2748=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2748.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2748/head:pull/2748 PR: https://git.openjdk.java.net/jdk/pull/2748
Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v5]
On Sun, 28 Feb 2021 13:31:38 GMT, Jie Fu wrote: >> `@requires vm.compiler2.enabled` had been added. >> Thanks. > > @PaulSandoz , are you also OK with the latest version? > Thanks. @DamonFool I think Vladimir is correct in the layering, in this respect i think we can make things a littler clearer. This seems like a small thing but i think its worth making very explicit as there is some hidden complexity. What if we add the following method to `VectorShape`: /** * Returns the maximum vector bit size for a given element type. * * @param etype the element type. * @return the maximum vector bit. */ /*package-private*/ static int getMaxVectorBitSize(Class etype) { // May return -1 if C2 is not enabled, // or a value smaller than the S_64_BIT.vectorBitSize / elementSizeInBits, on say 32-bit platforms // If so default to S_64_BIT int maxLaneCount = VectorSupport.getMaxLaneCount(etype); int elementSizeInBits = LaneType.of(etype).elementSize; return Math.max(maxLaneCount * elementSizeInBits, S_64_BIT.vectorBitSize); } It is package private so it can be tested explicitly if need be. Then we can reuse that method: S_Max_BIT(getMaxVectorBitSize(byte.class)); static VectorShape largestShapeFor(Class etype) { return VectorShape.forBitSize(getMaxVectorBitSize(etype)); } I think that's correct, but i have not tested. WDYT? - PR: https://git.openjdk.java.net/jdk/pull/2722
Re: RFR: JDK-8261518: jpackage looks for main module in current dir when there is no module-path
On Mon, 1 Mar 2021 15:55:39 GMT, Andy Herrick wrote: > when the app modules have already been jlinked with the runtime, and there is > no need for module-path, jpackage was acting as if the module-path was "." > and picking up jars in the current directory. Marked as reviewed by asemenyuk (Committer). - PR: https://git.openjdk.java.net/jdk/pull/2781
Re: RFR: JDK-8261839: Error creating runtime package on macos without mac-package-identifier
On Thu, 25 Feb 2021 21:15:44 GMT, Andy Herrick wrote: > …age-identifier Looks fine. - Marked as reviewed by kizune (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2730
RFR: JDK-8261518: jpackage looks for main module in current dir when there is no module-path
when the app modules have already been jlinked with the runtime, and there is no need for module-path, jpackage was acting as if the module-path was "." and picking up jars in the current directory. - Commit messages: - JDK-8261518: jpackage looks for main module in current dir when there is no module-path - JDK-8261518: jpackage looks for main module in current dir when there is no module-path Changes: https://git.openjdk.java.net/jdk/pull/2781/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2781=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8261518 Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2781.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2781/head:pull/2781 PR: https://git.openjdk.java.net/jdk/pull/2781
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v25]
On Mon, 1 Mar 2021 15:12:46 GMT, Roger Riggs wrote: >> throw new IllegalArgumentException("The random number generator "" + name + >> "" can not be located"); > > The message only captures the failure if the result of `fm.get()` is null. > It does not capture the failure if the name is found but does not support the > category. if (provider == null) { throw new IllegalArgumentException("No implementation of the random number generator algorithm "" + name + "" is available"); } else if (!isSubclass(category, provider)) { throw new IllegalArgumentException("The random number generator algorithm "" + name + "" is not implemented with the interface "" + category.simpleName() + """); } - PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: JDK-8262471: Fix coding style in src/java.base/share/classes/java/lang/CharacterDataPrivateUse.java
On Sat, 27 Feb 2021 08:03:34 GMT, Alan Bateman wrote: >> Please review this small patch which fixes the coding style of >> CharacterDataPrivateUse.java > > Looks fine. This source file was a .template until a few weeks ago, probably > should have fixed the indentation issues when moving it to a .java file. Thanks for the review. Do we need a second reviewer? If not, could you please sponsor the change? - PR: https://git.openjdk.java.net/jdk/pull/2754
Re: RFR: 8262379: Add regression test for JDK-8257746
On Mon, 1 Mar 2021 14:33:06 GMT, Harold Seigel wrote: >> Fails prior the patch of JDK-8257746, passes after. As expected. >> >> Thoughts? > > The new tests looks good. Thanks for adding it. > > Harold Thanks for the review @hseigel! - PR: https://git.openjdk.java.net/jdk/pull/2725
Integrated: 8262379: Add regression test for JDK-8257746
On Thu, 25 Feb 2021 16:27:20 GMT, Severin Gehwolf wrote: > Fails prior the patch of JDK-8257746, passes after. As expected. > > Thoughts? This pull request has now been integrated. Changeset: 4c9adce2 Author:Severin Gehwolf URL: https://git.openjdk.java.net/jdk/commit/4c9adce2 Stats: 56 lines in 1 file changed: 56 ins; 0 del; 0 mod 8262379: Add regression test for JDK-8257746 Reviewed-by: hseigel - PR: https://git.openjdk.java.net/jdk/pull/2725
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v25]
On Mon, 1 Mar 2021 13:23:48 GMT, Jim Laskey wrote: >> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java >> line 232: >> >>> 230: Provider provider = fm.get(name); >>> 231: if (!isSubclass(category, provider)) { >>> 232: throw new IllegalArgumentException(name + " is an unknown >>> random number generator"); >> >> The message is a bit vague. How about: >> >> "The random number generator does not support the category" > > throw new IllegalArgumentException("The random number generator "" + name + > "" can not be located"); The message only captures the failure if the result of `fm.get()` is null. It does not capture the failure if the name is found but does not support the category. - PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8262379: Add regression test for JDK-8257746
On Thu, 25 Feb 2021 16:27:20 GMT, Severin Gehwolf wrote: > Fails prior the patch of JDK-8257746, passes after. As expected. > > Thoughts? The new tests looks good. Thanks for adding it. Harold - Marked as reviewed by hseigel (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2725
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v25]
On Fri, 26 Feb 2021 21:32:12 GMT, Roger Riggs wrote: >> Jim Laskey has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 57 commits: >> >> - Merge branch 'master' into 8248862 >> - Adjust ThreadLocalRandom javadoc inheritence >> - L32X64StarStarRandom -> L32X64MixRandom >> - Various corrects >> - Revised javadoc per CSR reviews >> - Remove tabs from random/package-info.java >> - Correct copyright notice. >> - Merge branch 'master' into 8248862 >> - Update tests for RandomGeneratorFactory.all() >> - Merge branch 'master' into 8248862 >> - ... and 47 more: >> https://git.openjdk.java.net/jdk/compare/0257caad...b9094279 > > src/java.base/share/classes/java/util/random/RandomGenerator.java line 1313: > >> 1311: * furthermore can easily jump to an arbitrarily specified >> distant >> 1312: * point in the state cycle. >> 1313: * > > The similarity of the first sentence of each of the Jumpable, Leapable, and > arbitrarlyJumpable interface is so similar as to obscure the differences. You > have to read 25 words in to be able to find the description that makes them > different. The italics help but should include the whole of the phrase that > distinguishes them. > Alternatively, move the phrase to the beginning of the sentence or drop the > redundant phrasing. > "provide a common protocol of objects that generate pseudorandom sequences of > numbers of Boolean values", etc. Jumpable: * This interface is designed to provide a common protocol for objects that * generate pseudorandom sequences of numbers (or Boolean values) and * furthermore can easily jump forward, by a moderate amount (ex. * 264) to a distant point in the state cycle. Leapable: * This interface is designed to provide a common protocol for objects that * generate sequences of pseudorandom numbers (or Boolean values) and * furthermore can easily not only jump but also * leap forward, by a large amount (ex. 2128), to a * very distant point in the state cycle. ArbitrarilyJumpable: * This interface is designed to provide a common protocol for objects that * generate sequences of pseudorandom numbers (or Boolean values) and * furthermore can easily jump forward, by an arbitrary amount, to a * distant point in the state cycle. > src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line > 55: > >> 53: * >> 54: * A specific {@link RandomGeneratorFactory} can be located by using the >> 55: * {@link RandomGenerator#factoryOf(String)} method, where the argument >> string > > Broken link: the method is in this class. should be just "#factoryOf". Modified > src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line > 600: > >> 598: try { >> 599: ensureConstructors(); >> 600: return ctorBytes.newInstance((Object)seed); > > IntelliJ warns that the cast to (Object) is redundant. Modified > src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line > 29: > >> 27: >> 28: import java.lang.reflect.Constructor; >> 29: import java.lang.reflect.Method; > > Used import. Modified - PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v25]
On Fri, 26 Feb 2021 21:25:38 GMT, Roger Riggs wrote: >> Jim Laskey has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 57 commits: >> >> - Merge branch 'master' into 8248862 >> - Adjust ThreadLocalRandom javadoc inheritence >> - L32X64StarStarRandom -> L32X64MixRandom >> - Various corrects >> - Revised javadoc per CSR reviews >> - Remove tabs from random/package-info.java >> - Correct copyright notice. >> - Merge branch 'master' into 8248862 >> - Update tests for RandomGeneratorFactory.all() >> - Merge branch 'master' into 8248862 >> - ... and 47 more: >> https://git.openjdk.java.net/jdk/compare/0257caad...b9094279 > > src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line > 240: > >> 238: * Returns a {@link RandomGenerator} that utilizes the {@code name} >> 239: * algorithm. >> 240: * > > In javadoc, this seems like is should read as: "utilizes the named algorithm". > The use of the parameter name is unnecessary in this case because it is > obvious and readability suffers as is. Modified. - PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v25]
On Fri, 26 Feb 2021 19:30:09 GMT, Roger Riggs wrote: >> Jim Laskey has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 57 commits: >> >> - Merge branch 'master' into 8248862 >> - Adjust ThreadLocalRandom javadoc inheritence >> - L32X64StarStarRandom -> L32X64MixRandom >> - Various corrects >> - Revised javadoc per CSR reviews >> - Remove tabs from random/package-info.java >> - Correct copyright notice. >> - Merge branch 'master' into 8248862 >> - Update tests for RandomGeneratorFactory.all() >> - Merge branch 'master' into 8248862 >> - ... and 47 more: >> https://git.openjdk.java.net/jdk/compare/0257caad...b9094279 > > src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line > 232: > >> 230: Provider provider = fm.get(name); >> 231: if (!isSubclass(category, provider)) { >> 232: throw new IllegalArgumentException(name + " is an unknown >> random number generator"); > > The message is a bit vague. How about: > > "The random number generator does not support the category" throw new IllegalArgumentException("The random number generator "" + name + "" can not be located"); - PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8262379: Add regression test for JDK-8257746
On Thu, 25 Feb 2021 16:27:20 GMT, Severin Gehwolf wrote: > Fails prior the patch of JDK-8257746, passes after. As expected. > > Thoughts? @poonamparhar @hseigel Could you please take a look? It's just a simple regression test for the JDK-8257746 NPE regression. - PR: https://git.openjdk.java.net/jdk/pull/2725
Re: RFR: 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with "AssertionError: Should have GCd a method handle by now" [v4]
On Sun, 28 Feb 2021 10:28:56 GMT, Attila Szegedi wrote: >> 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with >> "AssertionError: Should have GCd a method handle by now" > > Attila Szegedi has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains one commit: > > 8261483: Eliminate flakiness of the tests by using iteration number limit > and explicitly running GC I think this looks good to go, Attila. - Marked as reviewed by plevart (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2617
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Fri, 12 Feb 2021 11:42:59 GMT, Vladimir Kempik wrote: >> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 194: >> >>> 192: // may get turned off by -fomit-frame-pointer. >>> 193: frame os::get_sender_for_C_frame(frame* fr) { >>> 194: return frame(fr->link(), fr->link(), fr->sender_pc()); >> >> Why is it >> >> return frame(fr->link(), fr->link(), fr->sender_pc()); >> >> and not >> >> return frame(fr->sender_sp(), fr->link(), fr->sender_pc()); >> >> like in the bsd-x86 counter part? > > bsd_aarcb64 was based on linux_aarch64, with addition of bsd-specific things > from bsd_x86 > You think the bsd-x86 way is better here ? There's no point copying x86. We don't have any way to know what the sender's SP was in a C frame without using unwind info. I think this is just used when trying to print the stack in a crash dump. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Fri, 5 Feb 2021 17:20:55 GMT, Anton Kozlov wrote: >> src/hotspot/cpu/aarch64/vm_version_aarch64.hpp line 93: >> >>> 91: CPU_MARVELL = 'V', >>> 92: CPU_INTEL = 'i', >>> 93: CPU_APPLE = 'a', >> >> The `ARM Architecture Reference Manual ARMv8, for ARMv8-A architecture >> profile` has 8538 pages, can we be more specific and point to the particular >> section of the document where the CPU codes are defined? > > They are defined in 13.2.95. MIDR_EL1, Main ID Register. Apple's code is not > there, but "Arm can assign codes that are not published in this manual. All > values not assigned by Arm are reserved and must not be used.". I assume the > value was obtained by digging around > https://github.com/apple/darwin-xnu/blob/main/osfmk/arm/cpuid.h#L62 Anton, this paragraph looks like an excellent comment. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v21]
On Fri, 26 Feb 2021 19:17:12 GMT, Anton Kozlov wrote: >> Please review the implementation of JEP 391: macOS/AArch64 Port. >> >> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and >> windows/aarch64. >> >> Major changes are in: >> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks >> JDK-8253817, JDK-8253818) >> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with >> necessary adjustments (JDK-8253819) >> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), >> required on macOS/AArch64 platform. It's implemented with >> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a >> thread, so W^X mode change relates to the java thread state change (for java >> threads). In most cases, JVM executes in write-only mode, except when >> calling a generated stub like SafeFetch, which requires a temporary switch >> to execute-only mode. The same execute-only mode is enabled when a java >> thread executes in java or native states. This approach of managing W^X mode >> turned out to be simple and efficient enough. >> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) > > Anton Kozlov has updated the pull request incrementally with two additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Minor fixes Thanks. With this, I think we're done. - Changes requested by aph (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 23:01:52 GMT, Gerard Ziemski wrote: >> Anton Kozlov has updated the pull request incrementally with six additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos >> - Add comments to WX transitions >> >>+ minor change of placements >> - Use macro conditionals instead of empty functions >> - Add W^X to tests >> - Do not require known W^X state >> - Revert w^x in gtests > > src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 652: > >> 650: >> 651: void os::setup_fpu() { >> 652: } > > Is there really nothing to do here, or does still need to be implemented? A > clarification comment here would help/. There is really nothing to do here. > src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 198: > >> 196: >> 197: NOINLINE frame os::current_frame() { >> 198: intptr_t *fp = *(intptr_t **)__builtin_frame_address(0); > > In the bsd_x86 counter part we initialize `fp` to `_get_previous_fp()` - do > we need to implement it on aarch64 as well (and using address 0 is just a > temp workaround) or is it doing the right thing here? (0)``` looks right to me. > src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 291: > >> 289: bool is_unsafe_arraycopy = (thread->doing_unsafe_access() && >> UnsafeCopyMemory::contains_pc(pc)); >> 290: if ((nm != NULL && nm->has_unsafe_access()) || >> is_unsafe_arraycopy) { >> 291: address next_pc = pc + NativeCall::instruction_size; > > Replace > > address next_pc = pc + NativeCall::instruction_size; > > with > > address next_pc = Assembler::locate_next_instruction(pc); > > there is at least one other place that needs it. Why is this change needed? AFAICS ```locate_next_instruction()``` is an x86 thing for variable-length instructions, and no other port uses it. - PR: https://git.openjdk.java.net/jdk/pull/2200
RFR: 8262503: Support records in Dynalink
8262503: Support records in Dynalink - Commit messages: - Document behavior changes - Add support for records to Dynalink - Test the desired functionality Changes: https://git.openjdk.java.net/jdk/pull/2767/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2767=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262503 Stats: 165 lines in 8 files changed: 153 ins; 6 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/2767.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2767/head:pull/2767 PR: https://git.openjdk.java.net/jdk/pull/2767
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v21]
On Fri, 26 Feb 2021 19:17:12 GMT, Anton Kozlov wrote: >> Please review the implementation of JEP 391: macOS/AArch64 Port. >> >> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and >> windows/aarch64. >> >> Major changes are in: >> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks >> JDK-8253817, JDK-8253818) >> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with >> necessary adjustments (JDK-8253819) >> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), >> required on macOS/AArch64 platform. It's implemented with >> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a >> thread, so W^X mode change relates to the java thread state change (for java >> threads). In most cases, JVM executes in write-only mode, except when >> calling a generated stub like SafeFetch, which requires a temporary switch >> to execute-only mode. The same execute-only mode is enabled when a java >> thread executes in java or native states. This approach of managing W^X mode >> turned out to be simple and efficient enough. >> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) > > Anton Kozlov has updated the pull request incrementally with two additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Minor fixes src/hotspot/cpu/aarch64/globalDefinitions_aarch64.hpp line 62: > 60: > 61: #if defined(__APPLE__) || defined(_WIN64) > 62: #define R18_RESERVED #define R18_RESERVED true``` - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8252399: Update mapMulti documentation to use type test pattern instead of instanceof once JEP 375 exits preview [v9]
> Hi, > > Could someone please review my changeset for JDK-8252399: 'Update mapMulti > documentation to use type test pattern instead of instanceof once JEP 375 > exits preview' ? > > This change updates the example code displayed in the API documentation for > mapMulti to use the type test pattern introduced in > [JEP375](https://openjdk.java.net/jeps/375). > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision: 8252399: Added note for keeping example in sync with test; updated example to use real values - Changes: - all: https://git.openjdk.java.net/jdk/pull/2544/files - new: https://git.openjdk.java.net/jdk/pull/2544/files/c47fbf92..944c7d5e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2544=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2544=07-08 Stats: 9 lines in 2 files changed: 2 ins; 5 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2544.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2544/head:pull/2544 PR: https://git.openjdk.java.net/jdk/pull/2544
Re: RFR: 8252399: Update mapMulti documentation to use type test pattern instead of instanceof once JEP 375 exits preview [v7]
On Wed, 24 Feb 2021 19:26:02 GMT, Stuart Marks wrote: >> Patrick Concannon has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8252399: Converted JavadocExamples to test > > src/java.base/share/classes/java/util/stream/Stream.java line 414: > >> 412: * } >> 413: * } >> 414: * } > > I'd suggest putting a comment somewhere outside the javadoc that points to > the test and has a statement similar to the one in the test, about keeping > the example here in synch with the test. I'm not sure what the best way is to > do this though. It probably cannot be after the javadoc comment, because the > javadoc comment needs to be immediately prior to the method declaration. > Maybe a // comment above the doc comment? Good idea. I've added a comment above the javadoc as you've suggested. See 944c7d5 > src/java.base/share/classes/java/util/stream/Stream.java line 410: > >> 408: * >> 409: * public static void main(String[] args) { >> 410: * var nestedList = ...; > > I think it would be good to expand the RHS to use what you use in the test, > namely > > var nestedList = List.of(1, List.of(2, List.of(3, 4)), 5); > > That would clarify the example quite a bit. Similar for the numbers example > above. Sounds good. I've added that to the example now. See 944c7d5 - PR: https://git.openjdk.java.net/jdk/pull/2544
Re: RFR: 8252399: Update mapMulti documentation to use type test pattern instead of instanceof once JEP 375 exits preview [v8]
> Hi, > > Could someone please review my changeset for JDK-8252399: 'Update mapMulti > documentation to use type test pattern instead of instanceof once JEP 375 > exits preview' ? > > This change updates the example code displayed in the API documentation for > mapMulti to use the type test pattern introduced in > [JEP375](https://openjdk.java.net/jeps/375). > > Kind regards, > Patrick Patrick Concannon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8252399 - 8252399: Converted JavadocExamples to test - Merge remote-tracking branch 'origin/master' into JDK-8252399 - 825399: Added wildcard to Iterable - Merge remote-tracking branch 'origin/master' into JDK-8252399 - 8252399: Added more appropriate test stream for Expand Iterable example - 8252399: Corrected Expand Iterable Example - 8252399: Update mapMulti documentation to use type test pattern instead of instanceof once JEP 375 exits preview - Changes: - all: https://git.openjdk.java.net/jdk/pull/2544/files - new: https://git.openjdk.java.net/jdk/pull/2544/files/47dd74df..c47fbf92 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2544=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2544=06-07 Stats: 25663 lines in 916 files changed: 16302 ins; 5038 del; 4323 mod Patch: https://git.openjdk.java.net/jdk/pull/2544.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2544/head:pull/2544 PR: https://git.openjdk.java.net/jdk/pull/2544
Re: RFR: 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with "AssertionError: Should have GCd a method handle by now" [v4]
On Sun, 28 Feb 2021 10:28:56 GMT, Attila Szegedi wrote: >> 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with >> "AssertionError: Should have GCd a method handle by now" > > Attila Szegedi has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains one additional > commit since the last revision: > > 8261483: Eliminate flakiness of the tests by using iteration number limit > and explicitly running GC Good. Minor suggestion follows. test/jdk/jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java line 25: > 23: > 24: /* > 25: * @test id=with_SerialGC No need to be that explicit ("with_SerialGC"). "id=serial" should be enough. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2617