Re: RFR: 8321713: Harmonize executeTestJvm with create[Limited]TestJavaProcessBuilder [v5]
> [JDK-8315097](https://bugs.openjdk.org/browse/JDK-8315097): 'Rename > createJavaProcessBuilder' changed the name of the ProcessTools helper > functions used to create `ProcessBuilder`s used to spawn new java test > processes. > > We now have `createTestJavaProcessBuilder` and > `createLimitedTestJavaProcess`. The former prepends jvm options from jtreg, > while the latter doesn't. > > With these functions it is common to see the following pattern in tests: > > ProcessBuilder pb = ProcessTools.createTestJavaProcessBuilder(...); > OutputAnalyzer output = executeProcess(pb); > > > We have a couple of thin wrapper in `ProcessTools` that does exactly this, so > that the code can be written as a one-liner: > > OutputAnalyzer output = ProcessTools.executeTestJvm(); > > > I propose that we name this functions using the same naming scheme we used > for `createTestJavaProcessBuilder` and `createLimitedTestJavaProcessBuilder`. > That is, we change `executeTestJvm` to `executeTestJava` and add a new > `executeLimitedTestJava` function. Stefan Karlsson 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 five additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into rename_executeTestJvm - Merge remote-tracking branch 'upstream/master' into rename_executeTestJvm - Test cleanup - Fix impl and add test - 8321713: Harmonize executeTestJvm with create[Limited]TestJavaProcessBuilder - Changes: - all: https://git.openjdk.org/jdk/pull/17049/files - new: https://git.openjdk.org/jdk/pull/17049/files/486dc6d5..755d925d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17049=04 - incr: https://webrevs.openjdk.org/?repo=jdk=17049=03-04 Stats: 875 lines in 70 files changed: 577 ins; 58 del; 240 mod Patch: https://git.openjdk.org/jdk/pull/17049.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17049/head:pull/17049 PR: https://git.openjdk.org/jdk/pull/17049
Re: RFR: 8265372: Simplify PKCS9Attribute
On Tue, 2 Jan 2024 18:22:51 GMT, Ben Perez wrote: >> src/java.base/share/classes/sun/security/pkcs/PKCS9Attribute.java line 186: >> >>> 184: /** >>> 185: * Array of attribute OIDs defined in PKCS9, by number. >>> 186: */ >> >> I don't think `PKCS9_OIDS` is useful now. It's used in >> `PKCS9Attributes.getAttributes()` but this method is used nowhere. It's also >> used in `PKCS9Attributes.toString` but we can just iterate through >> `attributes` there. I don't see a reason to print the attributes in this >> order. If we want to print them in the order they appear in the data, we can >> use `LinkedHashMap` to in `PKCS9Attributes`. `Hashtable` is a little stale. > > Do you think we can remove `PKCS9Attributes.getAttributes()` entirely or > should we just modify it to not use `PKCS9_OIDS` anymore? I think we can remove both. There is no need to keep a useless method as long as it's not an exported API. - PR Review Comment: https://git.openjdk.org/jdk/pull/17132#discussion_r1439897759
Re: RFR: 8265372: Simplify PKCS9Attribute
On Thu, 21 Dec 2023 16:27:43 GMT, Weijun Wang wrote: >> Refactored PKCS9Attribute to use a hash map instead of multiple arrays. The >> key for the hash map is an `ObjectIdentifier` and the values are a record >> `AttributeInfo` that stores the information previously contained in the >> arrays `PKCS9_VALUE_TAGS`, `VALUE_CLASSES`, and `SINGLE_VALUED`. >> >> It seems as though we should be able to get rid of constants such as >> `EMAIL_ADDRESS_OID` since they aren't heavily used with the hash map >> approach, but since the values are public it might cause compatibility >> issues. >> >> Another question is how to handle `RSA DSI`, `S/MIME`, >> `Extended-certificate`, and `Issuer Serial Number` OIDs. The prior version >> threw an error but in this refactor they are treated as an "unknown OID" and >> only throw a debug warning. This was addressed in >> https://bugs.openjdk.org/browse/JDK-8011867 but prior to this refactor the >> aforementioned OIDs were treated differently than unknown OIDs. > > src/java.base/share/classes/sun/security/pkcs/PKCS9Attribute.java line 186: > >> 184: /** >> 185: * Array of attribute OIDs defined in PKCS9, by number. >> 186: */ > > I don't think `PKCS9_OIDS` is useful now. It's used in > `PKCS9Attributes.getAttributes()` but this method is used nowhere. It's also > used in `PKCS9Attributes.toString` but we can just iterate through > `attributes` there. I don't see a reason to print the attributes in this > order. If we want to print them in the order they appear in the data, we can > use `LinkedHashMap` to in `PKCS9Attributes`. `Hashtable` is a little stale. Do you think we can remove `PKCS9Attributes.getAttributes()` entirely or should we just modify it to not use `PKCS9_OIDS` anymore? - PR Review Comment: https://git.openjdk.org/jdk/pull/17132#discussion_r1439704152
Re: RFR: 8321713: Harmonize executeTestJvm with create[Limited]TestJavaProcessBuilder [v4]
> [JDK-8315097](https://bugs.openjdk.org/browse/JDK-8315097): 'Rename > createJavaProcessBuilder' changed the name of the ProcessTools helper > functions used to create `ProcessBuilder`s used to spawn new java test > processes. > > We now have `createTestJavaProcessBuilder` and > `createLimitedTestJavaProcess`. The former prepends jvm options from jtreg, > while the latter doesn't. > > With these functions it is common to see the following pattern in tests: > > ProcessBuilder pb = ProcessTools.createTestJavaProcessBuilder(...); > OutputAnalyzer output = executeProcess(pb); > > > We have a couple of thin wrapper in `ProcessTools` that does exactly this, so > that the code can be written as a one-liner: > > OutputAnalyzer output = ProcessTools.executeTestJvm(); > > > I propose that we name this functions using the same naming scheme we used > for `createTestJavaProcessBuilder` and `createLimitedTestJavaProcessBuilder`. > That is, we change `executeTestJvm` to `executeTestJava` and add a new > `executeLimitedTestJava` function. Stefan Karlsson 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 four additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into rename_executeTestJvm - Test cleanup - Fix impl and add test - 8321713: Harmonize executeTestJvm with create[Limited]TestJavaProcessBuilder - Changes: - all: https://git.openjdk.org/jdk/pull/17049/files - new: https://git.openjdk.org/jdk/pull/17049/files/5d488f42..486dc6d5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17049=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17049=02-03 Stats: 5249 lines in 348 files changed: 3069 ins; 973 del; 1207 mod Patch: https://git.openjdk.org/jdk/pull/17049.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17049/head:pull/17049 PR: https://git.openjdk.org/jdk/pull/17049
Re: RFR: 8321713: Harmonize executeTestJvm with create[Limited]TestJavaProcessBuilder [v3]
On Mon, 11 Dec 2023 14:06:43 GMT, Stefan Karlsson wrote: >> [JDK-8315097](https://bugs.openjdk.org/browse/JDK-8315097): 'Rename >> createJavaProcessBuilder' changed the name of the ProcessTools helper >> functions used to create `ProcessBuilder`s used to spawn new java test >> processes. >> >> We now have `createTestJavaProcessBuilder` and >> `createLimitedTestJavaProcess`. The former prepends jvm options from jtreg, >> while the latter doesn't. >> >> With these functions it is common to see the following pattern in tests: >> >> ProcessBuilder pb = ProcessTools.createTestJavaProcessBuilder(...); >> OutputAnalyzer output = executeProcess(pb); >> >> >> We have a couple of thin wrapper in `ProcessTools` that does exactly this, >> so that the code can be written as a one-liner: >> >> OutputAnalyzer output = ProcessTools.executeTestJvm(); >> >> >> I propose that we name this functions using the same naming scheme we used >> for `createTestJavaProcessBuilder` and >> `createLimitedTestJavaProcessBuilder`. That is, we change `executeTestJvm` >> to `executeTestJava` and add a new `executeLimitedTestJava` function. > > Stefan Karlsson has updated the pull request incrementally with one > additional commit since the last revision: > > Test cleanup Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/17049#issuecomment-1874176578
Re: RFR: 8310813: Simplify and modernize equals, hashCode, and compareTo for BigInteger [v10]
> Please review this PR to use modern APIs and language features to simplify > equals, hashCode, and compareTo for BigInteger. If you have any performance > concerns, please raise them. > > This PR is cherry-picked from a bigger, not-yet-published PR, to test the > waters. That latter PR will be published soon. Pavel Rappo 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 17 additional commits since the last revision: - Merge branch 'master' into 8310813 - Merge branch 'master' into 8310813 - Merge branch 'master' into 8310813 - Merge branch 'master' into 8310813 - Fix bugs in Shared.createSingle - Merge branch 'master' into 8310813 - Group params coarser (suggested by @cl4es) - Splits 20 params into 3 groups: (S)mall, (M)edium and (L)arge. Every testXYZ method invokes M operations, where M is the maximum number of elements in a group. Shorter groups are cyclically padded. - Uses the org.openjdk.jmh.infra.Blackhole API and increases benchmark time. - Fixes a bug in Shared that precluded 0 from being in a pair. - Use better overloads (suggested by @cl4es) - Uses simpler, more suitable overloads for the subrange starting from 0 - Improve benchmarks - Merge branch 'master' into 8310813 - ... and 7 more: https://git.openjdk.org/jdk/compare/cd913e32...252b7378 - Changes: - all: https://git.openjdk.org/jdk/pull/14630/files - new: https://git.openjdk.org/jdk/pull/14630/files/ef8b0c46..252b7378 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14630=09 - incr: https://webrevs.openjdk.org/?repo=jdk=14630=08-09 Stats: 4826 lines in 316 files changed: 2898 ins; 812 del; 1116 mod Patch: https://git.openjdk.org/jdk/pull/14630.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14630/head:pull/14630 PR: https://git.openjdk.org/jdk/pull/14630
Re: RFR: JDK-8319122: Improve documentation of various Zip-file related APIs [v2]
On Fri, 10 Nov 2023 15:44:19 GMT, Yakov Shafranovich wrote: >> The various Zip/Jar-file related Java APIs have some long-standing >> differences or peculiarities with respect to the ZIP-file specification or >> compared to other implementations which should be documented in the API-doc. >> This documents the following: >> - Cache of JAR files in JarURLConnection class >> - Cache of JAR/ZIP files in JarFile and ZipFile classes >> - Unexpected behavior when parsing ZIP files with duplicate entries in >> JarFile and ZipFile classes, as well as the zipfs provider >> - Directories and filenames with the same name considered to be the same in >> ZipFile class >> - Possible issues when local and central headers conflict in ZipInputStream >> class >> >> Related JBS report: >> https://bugs.openjdk.org/browse/JDK-8319122 > > Yakov Shafranovich has updated the pull request incrementally with two > additional commits since the last revision: > > - Fixed more line breaks > - fixed line breaks Still working on this - PR Comment: https://git.openjdk.org/jdk/pull/16424#issuecomment-1874073990
Re: RFR: 8322766: Micro bench SSLHandshake should use default algorithms [v2]
On Tue, 2 Jan 2024 13:43:27 GMT, Sean Mullan wrote: >> John Jiang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Use default algorithms > > Can you change the bug summary to "Micro bench SSLHandshake should use modern > algorithms"? @seanjmullan The last commit uses the default algorithms, so I just changed the summary to "Micro bench SSLHandshake should use default algorithms". - PR Comment: https://git.openjdk.org/jdk/pull/17202#issuecomment-1874067097
Re: RFR: 8322766: Micro bench SSLHandshake would use modern algorithms [v2]
On Sat, 30 Dec 2023 00:48:00 GMT, John Jiang wrote: >> test/micro/org/openjdk/bench/java/security/SSLHandshake.java is using >> keystore type `JKS` and TrustManagerFactory/KeyManagerFactory algorithm >> `SunX509`. >> It may be better to use `PKCS12` and `PKIX` respectively. > > John Jiang has updated the pull request incrementally with one additional > commit since the last revision: > > Use default algorithms Can you change the bug summary to "Micro bench SSLHandshake should use modern algorithms"? - PR Comment: https://git.openjdk.org/jdk/pull/17202#issuecomment-1874042757
