Re: RFR: 8303689: javac -Xlint could/should report on "dangling" doc comments [v2]
> Please review the updates to support a proposed new
> `-Xlint:dangling-doc-comments` option.
>
> The work can be thought of as in 3 parts:
>
> 1. An update to the `javac` internal class `DeferredLintHandler` so that it
> is possible to specify the appropriately configured `Lint` object when it is
> time to consider whether to generate the diagnostic or not.
>
> 2. Updates to the `javac` front end to record "dangling docs comments" found
> near the beginning of a declaration, and to report them using an instance of
> `DeferredLintHandler`. This allows the warnings to be enabled or disabled
> using the standard mechanisms for `-Xlint` and `@SuppressWarnings`. The
> procedure for handling dangling doc comments is described in this comment in
> `JavacParser`.
>
> * Dangling documentation comments are handled as follows.
> * 1. {@code Scanner} adds all doc comments to a queue of
> * recent doc comments. The queue is flushed whenever
> * it is known that the recent doc comments should be
> * ignored and should not cause any warnings.
> * 2. The primary documentation comment is the one obtained
> * from the first token of any declaration.
> * (using {@code token.getDocComment()}.
> * 3. At the end of the "signature" of the declaration
> * (that is, before any initialization or body for the
> * declaration) any other "recent" comments are saved
> * in a map using the primary comment as a key,
> * using this method, {@code saveDanglingComments}.
> * 4. When the tree node for the declaration is finally
> * available, and the primary comment, if any,
> * is "attached", (in {@link #attach}) any related
> * dangling comments are also attached to the tree node
> * by registering them using the {@link #deferredLintHandler}.
> * 5. (Later) Warnings may be genereated for the dangling
> * comments, subject to the {@code -Xlint} and
> * {@code @SuppressWarnings}.
>
>
> 3. Updates to the make files to disable the warnings in modules for which
> the
> warning is generated. This is often because of the confusing use of `/**` to
> create box or other standout comments.
Jonathan Gibbons has updated the pull request incrementally with one additional
commit since the last revision:
adjust call for `saveDanglingDocComments` for enum members
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18527/files
- new: https://git.openjdk.org/jdk/pull/18527/files/3d6f1f95..56d6dcac
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=18527=01
- incr: https://webrevs.openjdk.org/?repo=jdk=18527=00-01
Stats: 5 lines in 1 file changed: 3 ins; 2 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/18527.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18527/head:pull/18527
PR: https://git.openjdk.org/jdk/pull/18527
Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v56]
> Please review a patch to add support for Markdown syntax in documentation > comments, as described in the associated JEP. > > Notable features: > > * support for `///` documentation comments in `JavaTokenizer` > * new module `jdk.internal.md` -- a private copy of the `commonmark-java` > library > * updates to `DocCommentParser` to treat `///` comments as Markdown > * updates to the standard doclet to render Markdown comments in HTML Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: address review feedback for updates to Elements and friends - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/21f5b004..d4c2c73b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=55 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=54-55 Stats: 9 lines in 2 files changed: 0 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v55]
On Tue, 9 Apr 2024 08:53:10 GMT, Pavel Rappo wrote:
>> Jonathan Gibbons has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> add support for JDK-8329296: Update Elements for '///' documentation
>> comments
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/model/JavacElements.java
> line 443:
>
>> 441: @DefinedBy(Api.LANGUAGE_MODEL)
>> 442: public CommentKind getDocCommentKind(Element e) {
>> 443: return getDocCommentItem(e, ((docCommentTable, tree) ->
>> docCommentTable.getCommentKind(tree)));
>
> Nit:
> Suggestion:
>
> return getDocCommentItem(e, ((docCommentTable, tree) ->
> docCommentTable.getCommentKind(tree)));
Fixed
> src/jdk.compiler/share/classes/com/sun/tools/javac/model/JavacElements.java
> line 443:
>
>> 441: @DefinedBy(Api.LANGUAGE_MODEL)
>> 442: public CommentKind getDocCommentKind(Element e) {
>> 443: return getDocCommentItem(e, ((docCommentTable, tree) ->
>> docCommentTable.getCommentKind(tree)));
>
> Again:
> Suggestion:
>
> return getDocCommentItem(e, ((docCommentTable, tree) ->
> docCommentTable.getCommentKind(tree)));
Fixed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1561382358
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1561382835
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v2]
On Wed, 10 Apr 2024 18:02:38 GMT, Volodymyr Paprotski wrote: > > In `ECOperations.java`, if I understand this correctly, it is to replace > > the existing `PointMultiplier` with montgomery-based PointMuliplier. But > > when I look at the code, I see both are still options. If I read this > > correctly, it checks for the old `IntegerFieldModuloP`, then looks for the > > new `IntegerMontgomeryFieldModuloP`. It appears to use the new one always. > > Why doesn't it just replace the old implementation entry in the `fields` > > Map? Is there a reason to keep it around? > > Hmm, thats a good point I haven't fully considered; i.e. (if I read > correctly) "for `CurveDB.P_256` remove the fallback path to non-montgomery > entirely".. that might also help in cleaning a few things up in the > construction. Maybe even get rid of this nested ECOperations inside > ECOperations.. Perhaps nesting isnt a big deal, but all attempts to make the > ECC stack clearer is positive! > > One functional reason that might justify keeping it as-is, is fuzz-testing; > with the fallback available, I am able to write the included Fuzz tests and > have them check the values against the existing implementation. While I also > included a few KAT tests using openssl-generated values, the fuzz tests check > millions of values and it does add a lot more certainty about correctness of > this code. I hadn't looked at your fuzz test until you mentioned it. I see you are using reflection to change the values. Is that what you mean by "fallback"? I'm assuming there is no to access the older implementation without reflection. > > Can it be removed? For the operations that do not involve multiplication > (i.e. `setSum(*)`), montgomery is expensive. I think I did go through the > uses of this code some time back (i.e. ECDHE, ECDSA and KeyGeneration) and > existing IntegerPolynomialP256 is no longer used (I should verify that again) > and only P256OrderField remains non-montgomery. So removing references to > IntegerPolynomialP256 in ECOperations should be possible and cleaner. > Removing IntegerPolynomialP256 from MontgomeryIntegerPolynomialP256 is harder > (fromMontgomery() uses IntegerPolynomialP256) but perhaps also worth some > thought.. > > I tend to like `ECOperationsFuzzTest.java` and would prefer to keep it, but > it could also be chucked up as part of 'scaffolding' and removed in name of > code quality? I wouldn't rip out the old implementation. I have been wondering if we should make the older implementation available, maybe by security property. I was looking at the static Maps at the top of `ECOperations`, `forParameters`, and the constructors where it checks if the `montgomeryOps` was null or set. It would be nice if we could have one set of `fields` Maps by putting the montgomery entry into the `fields` to replace it. I think that should work because `IntegerMontgomeryFieldModuloP` extends `IntegerFieldModuloP`. `instanceof` or other `montgomeryOps` checks would still need to exist because not all the `fields` support mongomery, and the older implementation would still be accessible for your fuzz tester. At least that is my theory. > > Thanks @ascarpino > > PS: Perhaps there is some middle ground, remove the `ECOperations > montgomeryOps` nesting, and construct (somehow?? singleton makes most things > inaccessible..) the reference ECOperations in the fuzz test instead.. not > sure how yet, but perhaps worth a further thought.. It would be nice to remove the nesting and it would be nice to be a singleton. Maybe some combination of what I mentioned above chance can help that. I haven't fully thought this out either. - PR Comment: https://git.openjdk.org/jdk/pull/18583#issuecomment-2050148475
Re: RFR: 8329970: Update autoconf build-aux files with latest from 2024-01-01
On Tue, 9 Apr 2024 19:43:27 GMT, Erik Joelsson wrote: > Since we are now able to update the autoconf build-aux files, I think it's > prudent to semi regularly do just that. I'm not aware of any particular > platform that has been improved that would affect OpenJDK and I don't think > we can further trim down our wrappers this time. This is more about staying > with the latest. As of the filing of this bug, that is 2024-01-01, found here: > > https://git.savannah.gnu.org/cgit/config.git/ > > I have verified this with the platforms we build for at Oracle. I would > encourage other OpenJDK distributors to verify on any platforms not covered > by us. I will leave this open for a few days. SAP testing didn't show problems. - Marked as reviewed by clanger (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18704#pullrequestreview-1994700270
Integrated: 8326947: Minimize MakeBase.gmk
On Wed, 28 Feb 2024 11:24:06 GMT, Magnus Ihse Bursie wrote: > This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. This pull request has now been integrated. Changeset: 16061874 Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/16061874ffdd1b018fe1cad7e6d8ba8bdbdbbee1 Stats: 981 lines in 43 files changed: 499 ins; 404 del; 78 mod 8326947: Minimize MakeBase.gmk Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/18041
Re: RFR: 8330107: Split Awt2dLibraries.gmk into separate AWT and 2D files
On Thu, 11 Apr 2024 13:53:23 GMT, Magnus Ihse Bursie wrote: > The file to build most of the java.desktop native libraries, t > Awt2dLibraries.gmk, is large and unstructured, making it hard to navigate. > > I want to split it into two parts, one for the AWT libraries, and one for the > 2D libraries. I also used this opportunity to change the order to be more > logical (e.g. grouping "image" libraries and "font" libraries in 2D). @prrace I will need your assistance in confirming that my understanding about the AWT vs 2D split is correct. In particular, `libosxui` gave me some headache, but after trying to dig into the code my understanding ended up being that this is part of Swing which is considered part of AWT, not 2D. I also looked at `libosx` and `libosxapp` which are located in the general `Lib.gmk` file. I could not easily see that they should have been placed in either 2d or Awt, so my assumption is that they are correctly placed outside these two new files. - PR Comment: https://git.openjdk.org/jdk/pull/18743#issuecomment-2049760109
Re: RFR: 8330107: Split Awt2dLibraries.gmk into separate AWT and 2D files
On Thu, 11 Apr 2024 13:53:23 GMT, Magnus Ihse Bursie wrote: > The file to build most of the java.desktop native libraries, t > Awt2dLibraries.gmk, is large and unstructured, making it hard to navigate. > > I want to split it into two parts, one for the AWT libraries, and one for the > 2D libraries. I also used this opportunity to change the order to be more > logical (e.g. grouping "image" libraries and "font" libraries in 2D). The default github review screen makes this a bit difficult to understand, especially AwtLibraries which it sees as a diff from the old Awt2d one. I recommend viewing the new file in its entirety instead: https://github.com/openjdk/jdk/blob/8180274092fa955cb7ff002bbdf3c32053dd337e/make/modules/java.desktop/lib/AwtLibraries.gmk Also to clarify: I have only moved the individual library compilations around, and made no other changes. - PR Comment: https://git.openjdk.org/jdk/pull/18743#issuecomment-2049752669
RFR: 8330107: Split Awt2dLibraries.gmk into separate AWT and 2D files
The file to build most of the java.desktop native libraries, t Awt2dLibraries.gmk, is large and unstructured, making it hard to navigate. I want to split it into two parts, one for the AWT libraries, and one for the 2D libraries. I also used this opportunity to change the order to be more logical (e.g. grouping "image" libraries and "font" libraries in 2D). - Commit messages: - 8330107: Split Awt2dLibraries.gmk into separate AWT and 2D files Changes: https://git.openjdk.org/jdk/pull/18743/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18743=00 Issue: https://bugs.openjdk.org/browse/JDK-8330107 Stats: 1707 lines in 4 files changed: 865 ins; 841 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18743.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18743/head:pull/18743 PR: https://git.openjdk.org/jdk/pull/18743
Re: RFR: 8326947: Minimize MakeBase.gmk [v6]
On Thu, 11 Apr 2024 12:48:01 GMT, Magnus Ihse Bursie wrote: >> This is part of a general "spring cleaning" of the build system, addressing >> old code that has bit-rotted, been subject to lava flow, or just had bad or >> smelly code that we've never gotten around to fix. >> >> This particular patch tries to make MakeBase truly minimal; only including >> the core parts of the build system that all makefiles will need. This is now >> limited to essential functionality for named parameter functions, variable >> dependency, tool execution, logging and fixpath functionality. MakeBase >> still includes Utils.gmk and FileUtils.gmk, and thus "provides" this >> functionality as well. Separating these out as well will be the subject of a >> future patch. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Fix dependency problem from inlining BaseUtils Looks good now. - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18041#pullrequestreview-1994127692
Re: RFR: 8326947: Minimize MakeBase.gmk [v7]
> This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix test-make - Changes: - all: https://git.openjdk.org/jdk/pull/18041/files - new: https://git.openjdk.org/jdk/pull/18041/files/8c25c87c..b6059e1a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18041=06 - incr: https://webrevs.openjdk.org/?repo=jdk=18041=05-06 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18041.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18041/head:pull/18041 PR: https://git.openjdk.org/jdk/pull/18041
Re: RFR: 8326947: Minimize MakeBase.gmk [v6]
> This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix dependency problem from inlining BaseUtils - Changes: - all: https://git.openjdk.org/jdk/pull/18041/files - new: https://git.openjdk.org/jdk/pull/18041/files/bba3672d..8c25c87c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18041=05 - incr: https://webrevs.openjdk.org/?repo=jdk=18041=04-05 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18041.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18041/head:pull/18041 PR: https://git.openjdk.org/jdk/pull/18041
Re: RFR: 8201183: sjavac build failures: "Connection attempt failed: Connection refused"
On Wed, 10 Apr 2024 11:32:43 GMT, Daniel Jeliński wrote: > The "Connection attempt failed: Connection refused" error may happen if the > client tries to connect to a server that is no longer listening, which in > turn may happen if the server shuts down without removing the port file. I > added a check that the delete operation succeeded, and retry as necessary. > > I removed the comment about asynchronous deletes on Windows. I don't think > it's correct; it's more likely that the file existed because the delete > operation failed. > > I added a 1 second delay after deleting the port file; this delay is intended > to allow any clients that managed to read the port file before it was deleted > to finish connecting. It should also take care of the "IOException caught > during compilation: Connection reset" issue. > > And finally, the portfile is now closed when not in use. This was necessary > to fix the failures on Windows, where the file cannot be deleted as long as > it is open in any process. > > In order to verify the fix, I modified `IdleMonitor.KEEPALIVE` to 1 second. > Without the changes from this PR this resulted in at least a few failures in > every mach5 run. With this PR I was able to build tier1-5 with no failures. Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/18712#issuecomment-2049609810
Integrated: 8201183: sjavac build failures: "Connection attempt failed: Connection refused"
On Wed, 10 Apr 2024 11:32:43 GMT, Daniel Jeliński wrote: > The "Connection attempt failed: Connection refused" error may happen if the > client tries to connect to a server that is no longer listening, which in > turn may happen if the server shuts down without removing the port file. I > added a check that the delete operation succeeded, and retry as necessary. > > I removed the comment about asynchronous deletes on Windows. I don't think > it's correct; it's more likely that the file existed because the delete > operation failed. > > I added a 1 second delay after deleting the port file; this delay is intended > to allow any clients that managed to read the port file before it was deleted > to finish connecting. It should also take care of the "IOException caught > during compilation: Connection reset" issue. > > And finally, the portfile is now closed when not in use. This was necessary > to fix the failures on Windows, where the file cannot be deleted as long as > it is open in any process. > > In order to verify the fix, I modified `IdleMonitor.KEEPALIVE` to 1 second. > Without the changes from this PR this resulted in at least a few failures in > every mach5 run. With this PR I was able to build tier1-5 with no failures. This pull request has now been integrated. Changeset: ecc603ca Author:Daniel Jeliński URL: https://git.openjdk.org/jdk/commit/ecc603ca9b441cbb7ad27fbc2529fcb0b1da1992 Stats: 32 lines in 1 file changed: 12 ins; 8 del; 12 mod 8201183: sjavac build failures: "Connection attempt failed: Connection refused" Reviewed-by: erikj, ihse - PR: https://git.openjdk.org/jdk/pull/18712
Re: RFR: 8326947: Minimize MakeBase.gmk [v5]
> This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Restore copyright year for untouched file - Changes: - all: https://git.openjdk.org/jdk/pull/18041/files - new: https://git.openjdk.org/jdk/pull/18041/files/18bcf569..bba3672d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18041=04 - incr: https://webrevs.openjdk.org/?repo=jdk=18041=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18041.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18041/head:pull/18041 PR: https://git.openjdk.org/jdk/pull/18041
Re: RFR: 8326947: Minimize MakeBase.gmk [v4]
> This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Inline BaseUtils.gmk into Utils.gmk - Changes: - all: https://git.openjdk.org/jdk/pull/18041/files - new: https://git.openjdk.org/jdk/pull/18041/files/47278b69..18bcf569 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18041=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18041=02-03 Stats: 357 lines in 3 files changed: 163 ins; 194 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18041.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18041/head:pull/18041 PR: https://git.openjdk.org/jdk/pull/18041
Re: RFR: 8326947: Minimize MakeBase.gmk [v3]
> This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Move SOURCE_REVISION_TRACKER to spec.gmk - Changes: - all: https://git.openjdk.org/jdk/pull/18041/files - new: https://git.openjdk.org/jdk/pull/18041/files/36bb38f0..47278b69 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18041=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18041=01-02 Stats: 12 lines in 3 files changed: 4 ins; 8 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18041.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18041/head:pull/18041 PR: https://git.openjdk.org/jdk/pull/18041
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]
On Thu, 11 Apr 2024 10:36:03 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review the patch? >> This pr is based on previous work and discussion in [pr >> 16234](https://github.com/openjdk/jdk/pull/16234), [pr >> 18294](https://github.com/openjdk/jdk/pull/18294). >> >> Compared with previous prs, the major change in this pr is to integrate the >> source of sleef (for the steps, please check >> `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than >> depends on external sleef things (header or lib) at build or run time. >> Besides of this change, also modify the previous changes accordingly, e.g. >> remove some uncessary files or changes especially in make dir of jdk. >> >> Besides of the code changes, one important task is to handle the legal >> process. >> >> Thanks! >> >> ## Performance >> NOTE: >> * `Src` means implementation in this pr, i.e. without depenency on external >> sleef. >> * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` >> * `system_sleef` means implementation in [previous pr >> 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk >> with depenency on external sleef. >> >> Basically, the perf data below shows that >> * this implementation has better performance than previous version in [pr >> 18294](https://github.com/openjdk/jdk/pull/18294), >> * and both sleef versions has much better performance compared with >> non-sleef version. >> >> |Benchmark |(size)|Src >> |Units|system_sleef|(system_sleef-Src)/Src|Diabled |(Disable-Src)/Src| >> |--|--|-|-||--|-|-| >> |3472:Double128Vector.ACOS |1024 |8546.842 |ns/op|8516.007|-0.004 >> |16799.273|0.966| >> |3473:Double128Vector.ASIN |1024 |6864.656 |ns/op|6987.328|0.018 >> |16602.442|1.419| >> |3474:Double128Vector.ATAN |1024 |11489.255|ns/op|12261.800 |0.067 >> |26329.320|1.292| >> |3475:Double128Vector.ATAN2|1024 |16661.170|ns/op|17234.472 |0.034 >> |42084.100|1.526| >> |3476:Double128Vector.CBRT |1024 |18999.387|ns/op|20298.458 |0.068 >> |35998.688|0.895| >> |3477:Double128Vector.COS |1024 |14081.857|ns/op|14846.117 |0.054 >> |24420.692|0.734| >> |3478:Double128Vector.COSH |1024 |12202.306|ns/op|12237.772 |0.003 >> |21343.863|0.749| >> |3479:Double128Vector.EXP |1024 |4553.108 |ns/op|4777.638 ... > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix performance issue I've also updated the pr description with performance data, it shows that * this implementation has better performance than previous version in https://github.com/openjdk/jdk/pull/18294, * and both sleef versions has much better performance compared with non-sleef version. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2049482861
Re: RFR: 8326947: Minimize MakeBase.gmk [v2]
> This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits: - Merge branch 'master' into minimize-makebase - Whitespace fix - MakeBase.gmk should not include MakeIO.gmk anymore - MakeBase.gmk should not include CopyFiles.gmk anymore - Reorder BaseUtils.gmk to make more sense - Move some more functionality to BaseUtils.gmk - Create BaseUtils.gmk with the most basic stuff - Move all file stuff from Utils.gmk to FileUtils.gmk - Document the purpose of MakeBase - Move SOURCE_REVISION_TRACKER to where it is used. This unfortunately creates a duplication, but there is no suitable place to put this information, and it does not belong in MakeBase. - ... and 4 more: https://git.openjdk.org/jdk/compare/9acce7a6...36bb38f0 - Changes: https://git.openjdk.org/jdk/pull/18041/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18041=01 Stats: 1099 lines in 43 files changed: 607 ins; 480 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/18041.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18041/head:pull/18041 PR: https://git.openjdk.org/jdk/pull/18041
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]
On Thu, 11 Apr 2024 10:36:03 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review the patch? >> This pr is based on previous work and discussion in [pr >> 16234](https://github.com/openjdk/jdk/pull/16234), [pr >> 18294](https://github.com/openjdk/jdk/pull/18294). >> >> Compared with previous prs, the major change in this pr is to integrate the >> source of sleef (for the steps, please check >> `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than >> depends on external sleef things (header or lib) at build or run time. >> Besides of this change, also modify the previous changes accordingly, e.g. >> remove some uncessary files or changes especially in make dir of jdk. >> >> Besides of the code changes, one important task is to handle the legal >> process. >> >> Thanks! > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > fix performance issue Thanks everyone for discussion about the direction (integrate source or lib). We did have some implementation for integrating sleef lib into jdk, but seems previously the most strong opinion is to integrate the sleef source into jdk. I know there are cons and pros for every solution, but I will stick to current solution unless everyone can reach another agreement. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2049410731
Re: RFR: 8329970: Update autoconf build-aux files with latest from 2024-01-01
On Tue, 9 Apr 2024 19:43:27 GMT, Erik Joelsson wrote: > Since we are now able to update the autoconf build-aux files, I think it's > prudent to semi regularly do just that. I'm not aware of any particular > platform that has been improved that would affect OpenJDK and I don't think > we can further trim down our wrappers this time. This is more about staying > with the latest. As of the filing of this bug, that is 2024-01-01, found here: > > https://git.savannah.gnu.org/cgit/config.git/ > > I have verified this with the platforms we build for at Oracle. I would > encourage other OpenJDK distributors to verify on any platforms not covered > by us. I will leave this open for a few days. Marked as reviewed by ihse (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18704#pullrequestreview-1993854457
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Tue, 9 Apr 2024 20:10:36 GMT, Mikael Vidstedt wrote: >> Hamlin Li has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - disable unused-function warnings; add log msg >> - minor > > Thank you for the update and for working on this in general. > > I've started working on JDK-8329816, preparing the change for the SLEEF > specific part of the change. Specifically, I'm currently planning on > including the three SLEEF header files, the README and a legal/sleef.md file > in that change. Let me know if you have any thoughts/concerns. > > Also, just for my understanding, would love to understand your thoughts on > the future here (I apologize if this was already discussed elsewhere): > > It seem like SLEEF is (sort of) limited to linux at this point (the SLEEF > README mentions that "Due to limited test capacities, SLEEF is currently only > officially supported on Linux with gcc or llvm/clang." ). That same README > does, however, indicate good test coverage on several architectures in > addition to aarch64 (including x86_64, PPC, RISC-V). With that in mind, it > looks like we could potentially use SLEEF for other architectures on linux in > the future? And potentially additional operating systems as well? Hey, @vidmik I've fixed the performance issue, and update the sleef inline headers and README. It's good for you to integrate these files via JDK-8329816. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2049400793
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]
> Hi, > Can you help to review the patch? > This pr is based on previous work and discussion in [pr > 16234](https://github.com/openjdk/jdk/pull/16234), [pr > 18294](https://github.com/openjdk/jdk/pull/18294). > > Compared with previous prs, the major change in this pr is to integrate the > source of sleef (for the steps, please check > `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than > depends on external sleef things (header or lib) at build or run time. > Besides of this change, also modify the previous changes accordingly, e.g. > remove some uncessary files or changes especially in make dir of jdk. > > Besides of the code changes, one important task is to handle the legal > process. > > Thanks! Hamlin Li has updated the pull request incrementally with one additional commit since the last revision: fix performance issue - Changes: - all: https://git.openjdk.org/jdk/pull/18605/files - new: https://git.openjdk.org/jdk/pull/18605/files/34529ff1..cd70f5a9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18605=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18605=01-02 Stats: 4 lines in 4 files changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605 PR: https://git.openjdk.org/jdk/pull/18605
Re: RFR: 8201183: sjavac build failures: "Connection attempt failed: Connection refused"
On Wed, 10 Apr 2024 11:32:43 GMT, Daniel Jeliński wrote: > The "Connection attempt failed: Connection refused" error may happen if the > client tries to connect to a server that is no longer listening, which in > turn may happen if the server shuts down without removing the port file. I > added a check that the delete operation succeeded, and retry as necessary. > > I removed the comment about asynchronous deletes on Windows. I don't think > it's correct; it's more likely that the file existed because the delete > operation failed. > > I added a 1 second delay after deleting the port file; this delay is intended > to allow any clients that managed to read the port file before it was deleted > to finish connecting. It should also take care of the "IOException caught > during compilation: Connection reset" issue. > > And finally, the portfile is now closed when not in use. This was necessary > to fix the failures on Windows, where the file cannot be deleted as long as > it is open in any process. > > In order to verify the fix, I modified `IdleMonitor.KEEPALIVE` to 1 second. > Without the changes from this PR this resulted in at least a few failures in > every mach5 run. With this PR I was able to build tier1-5 with no failures. Thank you very much for spending the time and effort of fixing these intermittent javac server issues! - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18712#pullrequestreview-1993815955
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v8]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about > ill formatted printf Joachim Kern has updated the pull request incrementally with one additional commit since the last revision: my_disclaim64 already removed by other PR - Changes: - all: https://git.openjdk.org/jdk/pull/18536/files - new: https://git.openjdk.org/jdk/pull/18536/files/a8d85924..030de164 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18536=07 - incr: https://webrevs.openjdk.org/?repo=jdk=18536=06-07 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18536/head:pull/18536 PR: https://git.openjdk.org/jdk/pull/18536
Integrated: 8329704: Implement framework for proper handling of JDK_LIBS
On Fri, 5 Apr 2024 09:56:48 GMT, Magnus Ihse Bursie wrote: > This is the pinnacle of the recent stream of refactorings in the build > system. This patch introduces a more abstract concept of "JDK_LIBS", where > only the library name (e.g. "java" or "java.desktop:jawt") is specified, and > the build system turns this into suitable linker flags: `-ljawt -L path>` or `jawt.lib -libpath:`, depending on linker. It will > also automatically create proper dependencies. This pull request has now been integrated. Changeset: f0cd866a Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/f0cd866a375082e14c69ccd3bf5e3d4d18edaebf Stats: 578 lines in 38 files changed: 211 ins; 266 del; 101 mod 8329704: Implement framework for proper handling of JDK_LIBS Reviewed-by: erikj, jwaters - PR: https://git.openjdk.org/jdk/pull/18649
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Fri, 5 Apr 2024 12:17:17 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review the patch? >> This pr is based on previous work and discussion in [pr >> 16234](https://github.com/openjdk/jdk/pull/16234), [pr >> 18294](https://github.com/openjdk/jdk/pull/18294). >> >> Compared with previous prs, the major change in this pr is to integrate the >> source of sleef (for the steps, please check >> `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than >> depends on external sleef things (header or lib) at build or run time. >> Besides of this change, also modify the previous changes accordingly, e.g. >> remove some uncessary files or changes especially in make dir of jdk. >> >> Besides of the code changes, one important task is to handle the legal >> process. >> >> Thanks! > > Hamlin Li has updated the pull request incrementally with two additional > commits since the last revision: > > - disable unused-function warnings; add log msg > - minor > Nice work, Hamlin and Xiaohong. I'm glad to see progress on incorporating > SLEEF library into the JDK. (Somehow I > From engineering perspective, I > believe that bundling vector math library with the JDK is the right thing to > do, but it doesn't imply the sources should be part of JDK. There are already > examples of optional dependencies on external native libraries in HotSpot > (e.g., hsdis tool w/ binutils, capstone, and llvm backends). No, it doesn't imply that the sources should be part of JDK, but practical reasons to do with the way that OpenJDK is built and shipped by various parties strongly suggests that we should integrate the SLEEF library into the JDK source tree. If we don't, there will be skew between OpenJDK versions shipped by different vendors. Also, I believe that there is less work for all of us if we integrate rather than having communicate to everyone building the JDK. And finally, Mark Reinhold has stated that the JDK is not downstream of any other project. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2049256172
