Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow
On Fri, 20 Aug 2021 22:44:34 GMT, Weijun Wang wrote: > This change modifies the default value of the `java.security.manager` system > property from "allow" to "disallow". This means unless it's explicitly set to > "allow", any call to `System.setSecurityManager()` would throw an UOE. > > The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are > updated to confirm this behavior change. Two other tests are updated because > they were added after JDK-8267184 and do not have > `-Djava.security.manager=allow` on its `@run` line even it they need to > install a `SecurityManager` at runtime. Thanks for the clarification @AlanBateman . @wangweij my apologies as I misunderstood the affect this change was going to have on the existing warnings - which is none. David - PR: https://git.openjdk.java.net/jdk/pull/5204
RFR: 8272836: Optimize run time for java/lang/invoke/LFCaching tests
See the RFE for discussion. Current PR improves the test time like this: $ make run-test TEST=java/lang/invoke/LFCaching/ # Before real3m51.608s user5m21.612s sys 0m5.391s # After real1m13.606s user2m26.827s sys 0m4.761s - Commit messages: - 8272836: Optimize run time for java/lang/invoke/LFCaching tests Changes: https://git.openjdk.java.net/jdk/pull/5214/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5214&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272836 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5214.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5214/head:pull/5214 PR: https://git.openjdk.java.net/jdk/pull/5214
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow
On Fri, 20 Aug 2021 22:44:34 GMT, Weijun Wang wrote: > This change modifies the default value of the `java.security.manager` system > property from "allow" to "disallow". This means unless it's explicitly set to > "allow", any call to `System.setSecurityManager()` would throw an UOE. > > The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are > updated to confirm this behavior change. Two other tests are updated because > they were added after JDK-8267184 and do not have > `-Djava.security.manager=allow` on its `@run` line even it they need to > install a `SecurityManager` at runtime. > > Please note that this code change requires jtreg to be upgraded to 6.1, where > a security manager [will not be > set](https://bugs.openjdk.java.net/browse/CODETOOLS-7902990). This issue (https://bugs.openjdk.java.net/browse/JDK-8270380) is blocked by jtreg 6.1 (https://bugs.openjdk.java.net/browse/CODETOOLS-7902990). Precisely, it should be blocked by a JDK bug that updates the jtreg required version to 6.1. I should have mentioned this in the PR. - PR: https://git.openjdk.java.net/jdk/pull/5204
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow
On Mon, 23 Aug 2021 03:22:18 GMT, Jaikiran Pai wrote: > Would this then allow the security manager to be used? In other words, can > the value of `java.security.manager` be changed dynamically at runtime or is > it restricted to be set only at launch time (via command line arugment > `-Djava.security.manager`)? Whether to allow a SecurityManager to be installed later is determined at system initialization time, so there is no use to set it to "allow" inside a program. See https://github.com/openjdk/jdk/blob/3a690240336bda8582a15ca52f4dcb78be323dcd/src/java.base/share/classes/java/lang/System.java#L2191 The spec in `SecurityManager.java` uses the words "if the Java virtual machine **is started with** the java.security.manager system property..." to describe this. - PR: https://git.openjdk.java.net/jdk/pull/5204
Re: RFR: 8272836: Optimize run time for java/lang/invoke/LFCaching tests
On Mon, 23 Aug 2021 11:33:35 GMT, Aleksey Shipilev wrote: > See the RFE for discussion. > > Current PR improves the test time like this: > > > $ make run-test TEST=java/lang/invoke/LFCaching/ > > # Before > real 3m51.608s > user 5m21.612s > sys 0m5.391s > > # After > real 1m13.606s > user 2m26.827s > sys 0m4.761s Looks reasonable. Maybe "Limit" rather than "Optimize"? - Marked as reviewed by redestad (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5214
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow
On Mon, 23 Aug 2021 12:09:42 GMT, Weijun Wang wrote: > > Would this then allow the security manager to be used? In other words, can > > the value of `java.security.manager` be changed dynamically at runtime or > > is it restricted to be set only at launch time (via command line arugment > > `-Djava.security.manager`)? > > Whether to allow a SecurityManager to be installed later is determined at > system initialization time, so there is no use to set it to "allow" inside a > program. See > > https://github.com/openjdk/jdk/blob/3a690240336bda8582a15ca52f4dcb78be323dcd/src/java.base/share/classes/java/lang/System.java#L2191 > > The spec in `SecurityManager.java` uses the words "if the Java virtual > machine **is started with** the java.security.manager system property..." to > describe this. Thank you for that clarification @wangweij. - PR: https://git.openjdk.java.net/jdk/pull/5204
Re: RFR: 8266936: Add a finalization JFR event [v7]
> Greetings, > > Object.finalize() was deprecated in JDK9. There is an ongoing effort to > replace and mitigate Object.finalize() uses in the JDK libraries; please see > https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. > > We also like to assist users in replacing and mitigating uses in non-JDK code. > > Hence, this changeset adds a periodic JFR event to help identify which > classes are overriding Object.finalize(). > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision: lock ranking - Changes: - all: https://git.openjdk.java.net/jdk/pull/4731/files - new: https://git.openjdk.java.net/jdk/pull/4731/files/d8b829e8..7d63693f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=05-06 Stats: 26 lines in 3 files changed: 6 ins; 11 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/4731.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731 PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: JDK-8272639: jpackaged applications using microphone on mac
On Thu, 19 Aug 2021 22:46:18 GMT, Sergey Bylokhov wrote: >> JDK-8272639: jpackaged applications using microphone on mac > > src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/Info-lite.plist.template > line 37: > >> 35: true >> 36: NSMicrophoneUsageDescription >> 37: The application is requesting access to the >> microphone. > > Should not it include the "application name" here? This is different from the > java tool, where we do not know that info in advance. Yes - I will revise with name substituted here. I also considered localizing this string, but we only support localization of the tool, which would put string in the local that jpackage was run in, not the local the generated artifact was run in. - PR: https://git.openjdk.java.net/jdk/pull/5186
Re: RFR: 8272836: Limit run time for java/lang/invoke/LFCaching tests
On Mon, 23 Aug 2021 11:33:35 GMT, Aleksey Shipilev wrote: > See the RFE for discussion. > > Current PR improves the test time like this: > > > $ make run-test TEST=java/lang/invoke/LFCaching/ > > # Before > real 3m51.608s > user 5m21.612s > sys 0m5.391s > > # After > real 1m13.606s > user 2m26.827s > sys 0m4.761s Can be "Limit", sure. - PR: https://git.openjdk.java.net/jdk/pull/5214
Re: RFR: 8266936: Add a finalization JFR event [v8]
> Greetings, > > Object.finalize() was deprecated in JDK9. There is an ongoing effort to > replace and mitigate Object.finalize() uses in the JDK libraries; please see > https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. > > We also like to assist users in replacing and mitigating uses in non-JDK code. > > Hence, this changeset adds a periodic JFR event to help identify which > classes are overriding Object.finalize(). > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision: enqueued data point - Changes: - all: https://git.openjdk.java.net/jdk/pull/4731/files - new: https://git.openjdk.java.net/jdk/pull/4731/files/7d63693f..5da3bb89 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=06-07 Stats: 62 lines in 5 files changed: 46 ins; 8 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/4731.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731 PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: JDK-8272639: jpackaged applications using microphone on mac [v2]
> JDK-8272639: jpackaged applications using microphone on mac Andy Herrick has updated the pull request incrementally with one additional commit since the last revision: JDK-8272639: jpackaged applications using microphone on mac - Changes: - all: https://git.openjdk.java.net/jdk/pull/5186/files - new: https://git.openjdk.java.net/jdk/pull/5186/files/e1f460e1..337e1fa1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5186&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5186&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5186.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5186/head:pull/5186 PR: https://git.openjdk.java.net/jdk/pull/5186
RFR: 8272473: Parsing epoch seconds at a DST transition with a non-UTC parser is wrong
Please review the fix to the subject issue. When instant seconds and zone co-exist in parsed data, instant seconds was not resolved correctly from them. - Commit messages: - 8272473: Parsing epoch seconds at a DST transition with a non-UTC parser is wrong Changes: https://git.openjdk.java.net/jdk/pull/5225/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5225&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272473 Stats: 14 lines in 2 files changed: 8 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/5225.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5225/head:pull/5225 PR: https://git.openjdk.java.net/jdk/pull/5225
RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null
Please review this simple PR along with the associated CSR. The PR is basically adding a line the the specification of method `java.lang.runtime.ObjectMethods::bootstrap` stating under what conditions a NPE will be thrown. TIA link to the [CSR](https://bugs.openjdk.java.net/browse/JDK-8272852) - Commit messages: - 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null Changes: https://git.openjdk.java.net/jdk/pull/5226/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5226&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272347 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5226.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5226/head:pull/5226 PR: https://git.openjdk.java.net/jdk/pull/5226
Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null
+1 On 8/23/2021 2:22 PM, Vicente Romero wrote: Please review this simple PR along with the associated CSR. The PR is basically adding a line the the specification of method `java.lang.runtime.ObjectMethods::bootstrap` stating under what conditions a NPE will be thrown. TIA link to the [CSR](https://bugs.openjdk.java.net/browse/JDK-8272852) - Commit messages: - 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null Changes: https://git.openjdk.java.net/jdk/pull/5226/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5226&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272347 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5226.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5226/head:pull/5226 PR: https://git.openjdk.java.net/jdk/pull/5226
Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null
Actually, it will not NPE if `names` is null and you have selected equals/hashCode as the name. Might be better to do requiresNonNull() up front for all the arguments, just to make such analysis simpler: requireNonNull(methodName); requireNonNull(type); requireNonNull(recordClass); requireNonNull(names); requireNonNull(getters); On 8/23/2021 4:04 PM, Brian Goetz wrote: +1 On 8/23/2021 2:22 PM, Vicente Romero wrote: Please review this simple PR along with the associated CSR. The PR is basically adding a line the the specification of method `java.lang.runtime.ObjectMethods::bootstrap` stating under what conditions a NPE will be thrown. TIA link to the [CSR](https://bugs.openjdk.java.net/browse/JDK-8272852) - Commit messages: - 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null Changes: https://git.openjdk.java.net/jdk/pull/5226/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5226&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272347 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5226.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5226/head:pull/5226 PR: https://git.openjdk.java.net/jdk/pull/5226
Re: RFR: JDK-8272639: jpackaged applications using microphone on mac [v2]
On Mon, 23 Aug 2021 14:34:52 GMT, Andy Herrick wrote: >> JDK-8272639: jpackaged applications using microphone on mac > > Andy Herrick has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8272639: jpackaged applications using microphone on mac Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5186
Re: [(Much) faster java.util.LinkedList]
Hi, Thanks for sharing. I browsed through the code (not a review) and gave it a test drive, generally it looks of good quality. Over time, since LinkedList was added, there are less reasons to use it, thereby making such improvements you propose less impactful. CPU caches have got much better, so traversal of ArrayList is really good, whereas traversal of LinkedList is not cache efficient and nor is it easily amenable to runtime compiler optimizations like auto-vectorization. ArrayDeque (almost if not as good as ArrayList) was added that supports efficient first/last addition. I would argue that ArrayList or ArrayDeque are better choices in the majority of cases. In other cases where insertion is more generally required and there is a relation between elements a TreeSet or PriorityQueue may be a better choice. So while I think you have done a good job implementing an enhanced LinkedList implementation I am not convinced it's worth adding to the JDK. There are other areas we could enhance that might have more impact. For example, can we improve how ArrayList and ArrayDeque grow in size? When we added the Java Stream API we added an internal data structure SpinedBuffer, which behaves like an append-only array list with better growth characteristics, using an array of arrays (the spine). Maybe such a technique is more broadly applicable to ArrayList and perhaps ArrayDeque? Paul. > On Aug 18, 2021, at 8:30 PM, Rodion Efremov wrote: > > Hello, > > I have implemented a heuristic, indexed doubly-linked list data structure > [1][2] implementing java.util.List and java.util.Deque interfaces that has > superior performance compared to java.util.LinkedList and > java.util.ArrayList. > > I would like to propose it into the upcoming versions of OpenJDK, but, > first, I need to know what do you think about it? > > Best regards, > rodde > > [1] https://github.com/coderodde/LinkedList > [2] http://coderodde.github.io/weblog/#eill
Re: RFR: 8269559: AArch64: Implement string_compare intrinsic in SVE [v2]
On Tue, 17 Aug 2021 07:14:24 GMT, Nick Gasson wrote: >> TatWai Chong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Restore the removal of vtmp3 (=V2) as it is still used by the non-SVE >> compare-long-strings stub. >> >> And remove the assertion in `string_compare` since it won't help as the >> registers >> used in the stub are fixed. > > src/hotspot/cpu/aarch64/aarch64.ad line 16515: > >> 16513: instruct string_compareUL(iRegP_R1 str1, iRegI_R2 cnt1, iRegP_R3 >> str2, iRegI_R4 cnt2, >> 16514: iRegI_R0 result, iRegP_R10 tmp1, iRegL_R11 >> tmp2, >> 16515: vRegD_V0 vtmp1, vRegD_V1 vtmp2, vRegD_V2 >> vtmp3, rFlagsReg cr) > > I think vtmp3 (=V2) is still used by the non-SVE compare-long-strings stub? > (see `generate_compare_long_string_different_encoding`) Thanks, Nick. Yes. I remove this vtmp3 from input arguments wrongly. I roll back this mistake in the following patch. - PR: https://git.openjdk.java.net/jdk/pull/5129
Re: RFR: 8269559: AArch64: Implement string_compare intrinsic in SVE [v2]
> This patch implements string_compare intrinsic in SVE. > It supports all LL, LU, UL and UU comparisons. > > As we haven't found an existing benchmark to measure performance impact, > we created a benchmark derived from the test [1] for this evaluation. > This benchmark is attached to this patch. > > Besides, remove the unused temporary register `vtmp3` from the existing > match rules for StrCmp. > > The result below shows all varients can be benefited largely. > Command: make exploded-test TEST="micro:StringCompareToDifferentLength" > > Benchmark(size) Mode Cnt Score Speedup Units > compareToLL 24 avgt 10 1.0x ms/op > compareToLL 36 avgt 10 1.0x ms/op > compareToLL 72 avgt 10 1.0x ms/op > compareToLL 128 avgt 10 1.4x ms/op > compareToLL 256 avgt 10 1.8x ms/op > compareToLL 512 avgt 10 2.7x ms/op > compareToLU 24 avgt 10 1.6x ms/op > compareToLU 36 avgt 10 1.8x ms/op > compareToLU 72 avgt 10 2.3x ms/op > compareToLU 128 avgt 10 3.8x ms/op > compareToLU 256 avgt 10 4.7x ms/op > compareToLU 512 avgt 10 6.3x ms/op > compareToUL 24 avgt 10 1.6x ms/op > compareToUL 36 avgt 10 1.7x ms/op > compareToUL 72 avgt 10 2.2x ms/op > compareToUL 128 avgt 10 3.3x ms/op > compareToUL 256 avgt 10 4.4x ms/op > compareToUL 512 avgt 10 6.1x ms/op > compareToUU 24 avgt 10 1.0x ms/op > compareToUU 36 avgt 10 1.0x ms/op > compareToUU 72 avgt 10 1.4x ms/op > compareToUU 128 avgt 10 2.2x ms/op > compareToUU 256 avgt 10 2.6x ms/op > compareToUU 512 avgt 10 3.7x ms/op > > [1] > https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/compiler/intrinsics/string/TestStringCompareToDifferentLength.java TatWai Chong has updated the pull request incrementally with one additional commit since the last revision: Restore the removal of vtmp3 (=V2) as it is still used by the non-SVE compare-long-strings stub. And remove the assertion in `string_compare` since it won't help as the registers used in the stub are fixed. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5129/files - new: https://git.openjdk.java.net/jdk/pull/5129/files/bae862e1..2ae102d0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5129&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5129&range=00-01 Stats: 31 lines in 6 files changed: 6 ins; 2 del; 23 mod Patch: https://git.openjdk.java.net/jdk/pull/5129.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5129/head:pull/5129 PR: https://git.openjdk.java.net/jdk/pull/5129
Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null
On Mon, 23 Aug 2021 18:08:02 GMT, Vicente Romero wrote:
> Please review this simple PR along with the associated CSR. The PR is
> basically adding a line the the specification of method
> `java.lang.runtime.ObjectMethods::bootstrap` stating under what conditions a
> NPE will be thrown.
>
> TIA
>
> link to the [CSR](https://bugs.openjdk.java.net/browse/JDK-8272852)
Does the existing `ObjectMethodsTest` test verify all NPE cases?
src/java.base/share/classes/java/lang/runtime/ObjectMethods.java line 327:
> 325: * @throws IllegalArgumentException if the bootstrap arguments are
> invalid
> 326: * or inconsistent
> 327: * @throws NullPointerException if any argument but {@code lookup}
> is {@code null}
`names` may be null if the {@code methodName} is {@code "equals"} or {@code
"hashCode"}.This should be captured here.
-
PR: https://git.openjdk.java.net/jdk/pull/5226
RFR: 8272861: Add a micro benchmark for vector api
This pull request adds a micro benchmark for Vector API. The Black Scholes algorithm is implemented with and without Vector API. We see about ~6x gain with Vector API for this micro benchmark using 256 bit vectors. - Commit messages: - whitespace - 8272861: Add a micro benchmark for vector api Changes: https://git.openjdk.java.net/jdk/pull/5234/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5234&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272861 Stats: 190 lines in 1 file changed: 190 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5234/head:pull/5234 PR: https://git.openjdk.java.net/jdk/pull/5234
Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null
On 8/23/21 4:07 PM, Brian Goetz wrote: Actually, it will not NPE if `names` is null and you have selected equals/hashCode as the name. Might be better to do requiresNonNull() up front for all the arguments, just to make such analysis simpler: requireNonNull(methodName); requireNonNull(type); requireNonNull(recordClass); requireNonNull(names); requireNonNull(getters); will do, thanks, Vicente On 8/23/2021 4:04 PM, Brian Goetz wrote: +1 On 8/23/2021 2:22 PM, Vicente Romero wrote: Please review this simple PR along with the associated CSR. The PR is basically adding a line the the specification of method `java.lang.runtime.ObjectMethods::bootstrap` stating under what conditions a NPE will be thrown. TIA link to the [CSR](https://bugs.openjdk.java.net/browse/JDK-8272852) - Commit messages: - 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null Changes: https://git.openjdk.java.net/jdk/pull/5226/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5226&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272347 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5226.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5226/head:pull/5226 PR: https://git.openjdk.java.net/jdk/pull/5226
Re: RFR: 8264322: Generate CDS archive when creating custom JDK image [v2]
On Fri, 20 Aug 2021 02:04:52 GMT, Mandy Chung wrote: >> I'd prefer to leave it as is for now since it's in a test case and I don't >> think the code is complex. > > This is fragile by setting `os.name`. I've filed [JDK-8272868](https://bugs.openjdk.java.net/browse/JDK-8272868) to follow-up the above issue. - PR: https://git.openjdk.java.net/jdk/pull/5174
Re: RFR: 8264322: Generate CDS archive when creating custom JDK image [v2]
On Fri, 20 Aug 2021 00:09:01 GMT, Calvin Cheung wrote: >> Please review this change for adding a `jlink` command line option >> `--generate-cds-archive` for generating CDS archives as a post processing >> step during the creation of a custom JDK image. >> >> Details can be found in the corresponding CSR >> [JDK-8269178](https://bugs.openjdk.java.net/browse/JDK-8269178). >> >> Testing: >> >> - [x] tiers 1,2 (including the new test) > > Calvin Cheung has updated the pull request incrementally with one additional > commit since the last revision: > > @mlchung comments Thanks for creating a JBS issue. I'm okay to follow up the test improvement as a separate issue. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5174
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v4]
On Wed, 28 Jul 2021 08:51:38 GMT, Andrew Haley wrote: >> I don't think we want to keep two copies of the compareTo intrinsic. If >> there are no cases where the LDP version is worse than the original version >> then we should just delete the old one and replace it with this. > >> I don't think we want to keep two copies of the compareTo intrinsic. If >> there are no cases where the LDP version is worse than the original version >> then we should just delete the old one and replace it with this. > > I agree. The trouble is, what does "worse" mean? I'm looking at SDEN-1982442, > Neoverse N2 errata, 2001293, and I see that LDP has to be slowed down on > streaming workloads, which will affect this. (That's just an example: I'm > making the point that implementations differ.) > > The trouble with this patch is that it (probably) makes things better for > long strings, which are very rare. What we actually need to care about is > performance for a large number of typical-sized strings, which are names, > identifiers, passwords, and so on: about 10-30 characters. Hi, @theRealAph @nick-arm, The test data looks OK on Raspberry Pi 4B and Hisilicon, do you have any other questions about this patch? - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null [v2]
> Please review this simple PR along with the associated CSR. The PR is > basically adding a line the the specification of method > `java.lang.runtime.ObjectMethods::bootstrap` stating under what conditions a > NPE will be thrown. > > TIA > > link to the [CSR](https://bugs.openjdk.java.net/browse/JDK-8272852) Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: addressing review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/5226/files - new: https://git.openjdk.java.net/jdk/pull/5226/files/b7489b41..89086ca1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5226&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5226&range=00-01 Stats: 30 lines in 2 files changed: 17 ins; 1 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/5226.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5226/head:pull/5226 PR: https://git.openjdk.java.net/jdk/pull/5226
Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null [v2]
On Mon, 23 Aug 2021 23:13:58 GMT, Mandy Chung wrote:
>> Vicente Romero has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> addressing review comments
>
> src/java.base/share/classes/java/lang/runtime/ObjectMethods.java line 327:
>
>> 325: * @throws IllegalArgumentException if the bootstrap arguments are
>> invalid
>> 326: * or inconsistent
>> 327: * @throws NullPointerException if any argument but {@code lookup}
>> is {@code null}
>
> `names` may be null if the {@code methodName} is {@code "equals"} or {@code
> "hashCode"}.This should be captured here.
Hi Mandy, I have changed the implementation of the method to explicitly require
all arguments but lookup to be non-null as suggested by Brian. I have also
covered, I think, all the missing test cases in test `ObjectMethodsTest`,
thanks for your comments.
-
PR: https://git.openjdk.java.net/jdk/pull/5226
Re: RFR: 8272861: Add a micro benchmark for vector api
On Mon, 23 Aug 2021 23:18:28 GMT, Sandhya Viswanathan wrote: > This pull request adds a micro benchmark for Vector API. > The Black Scholes algorithm is implemented with and without Vector API. > We see about ~6x gain with Vector API for this micro benchmark using 256 bit > vectors. test/micro/org/openjdk/bench/jdk/incubator/vector/BlackScholes.java line 138: > 136: } > 137: > 138: static final VectorSpecies fsp = FloatVector.SPECIES_256; Can we use a more portable way instead of fixed size SPECIES_256? - PR: https://git.openjdk.java.net/jdk/pull/5234
Re: RFR: 8269559: AArch64: Implement string_compare intrinsic in SVE [v2]
On Mon, 23 Aug 2021 21:48:01 GMT, TatWai Chong wrote: >> This patch implements string_compare intrinsic in SVE. >> It supports all LL, LU, UL and UU comparisons. >> >> As we haven't found an existing benchmark to measure performance impact, >> we created a benchmark derived from the test [1] for this evaluation. >> This benchmark is attached to this patch. >> >> Besides, remove the unused temporary register `vtmp3` from the existing >> match rules for StrCmp. >> >> The result below shows all varients can be benefited largely. >> Command: make exploded-test TEST="micro:StringCompareToDifferentLength" >> >> Benchmark(size) Mode Cnt Score Speedup Units >> compareToLL 24 avgt 10 1.0x ms/op >> compareToLL 36 avgt 10 1.0x ms/op >> compareToLL 72 avgt 10 1.0x ms/op >> compareToLL 128 avgt 10 1.4x ms/op >> compareToLL 256 avgt 10 1.8x ms/op >> compareToLL 512 avgt 10 2.7x ms/op >> compareToLU 24 avgt 10 1.6x ms/op >> compareToLU 36 avgt 10 1.8x ms/op >> compareToLU 72 avgt 10 2.3x ms/op >> compareToLU 128 avgt 10 3.8x ms/op >> compareToLU 256 avgt 10 4.7x ms/op >> compareToLU 512 avgt 10 6.3x ms/op >> compareToUL 24 avgt 10 1.6x ms/op >> compareToUL 36 avgt 10 1.7x ms/op >> compareToUL 72 avgt 10 2.2x ms/op >> compareToUL 128 avgt 10 3.3x ms/op >> compareToUL 256 avgt 10 4.4x ms/op >> compareToUL 512 avgt 10 6.1x ms/op >> compareToUU 24 avgt 10 1.0x ms/op >> compareToUU 36 avgt 10 1.0x ms/op >> compareToUU 72 avgt 10 1.4x ms/op >> compareToUU 128 avgt 10 2.2x ms/op >> compareToUU 256 avgt 10 2.6x ms/op >> compareToUU 512 avgt 10 3.7x ms/op >> >> [1] >> https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/compiler/intrinsics/string/TestStringCompareToDifferentLength.java > > TatWai Chong has updated the pull request incrementally with one additional > commit since the last revision: > > Restore the removal of vtmp3 (=V2) as it is still used by the non-SVE > compare-long-strings stub. > > And remove the assertion in `string_compare` since it won't help as the > registers > used in the stub are fixed. This looks ok to me now but please wait to see if @theRealAph has any comments. I suppose the the short-string comparison code which is expanded at the call site could also benefit from SVE, if only to make it shorter. - Marked as reviewed by ngasson (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5129
RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules
Collections.sort is just a wrapper, so it is better to use an instance method directly. - Commit messages: - [PATCH] Replace usages of Collections.sort with List.sort call in public java modules Changes: https://git.openjdk.java.net/jdk/pull/5229/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5229&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272863 Stats: 27 lines in 10 files changed: 0 ins; 8 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/5229.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5229/head:pull/5229 PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules
On Mon, 23 Aug 2021 21:01:48 GMT, Andrey Turbanov wrote: > Collections.sort is just a wrapper, so it is better to use an instance method > directly. The changes in the src/java.desktop/ looks fine. Filed: https://bugs.openjdk.java.net/browse/JDK-8272863 - Marked as reviewed by serb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5229
Dangling class-level javadoc comments in JDK
Hello. I found a few internal classes in the JDK codebase which don't have proper javadoc, but have dangling javadoc-like comments. Dangling Javadoc comments are ignored by the javadoc tool and IDE. Perhaps it was intentional to not add proper javadoc to them? I believe it's better to convert them to proper javadoc to make developing JDK a bit more friendly within IDE. What do you think? List of classes: com.sun.beans.editors.BooleanEditor com.sun.beans.editors.ByteEditor com.sun.beans.editors.DoubleEditor com.sun.beans.editors.FloatEditor com.sun.beans.editors.IntegerEditor com.sun.beans.editors.LongEditor com.sun.beans.editors.NumberEditor com.sun.beans.editors.ShortEditor com.sun.jndi.toolkit.dir.ContainmentFilter com.sun.jndi.toolkit.dir.LazySearchEnumerationImpl com.sun.security.auth.module.Crypt java.math.MutableBigInteger java.net.DefaultInterface javax.swing.GraphicsWrapper jdk.internal.access.JavaAWTFontAccess jdk.javadoc.internal.doclets.toolkit.CommentUtils sun.awt.X11.XAtom sun.awt.X11.XAwtState sun.java2d.xr.XRBackend sun.java2d.xr.XRDrawLine sun.jvm.hotspot.debugger.PageCache sun.jvm.hotspot.runtime.JavaThreadFactory sun.jvm.hotspot.utilities.Interval sun.jvm.hotspot.utilities.MessageQueueBackend sun.jvm.hotspot.utilities.RBTree sun.launcher.LauncherHelper sun.net.www.content.text.plain sun.net.www.protocol.file.FileURLConnection sun.nio.ch.Interruptible sun.security.pkcs.ParsingException sun.security.provider.SeedGenerator sun.security.util.ByteArrayTagOrder sun.security.util.IOUtils Andrey Turbanov
