Re: RFR: JDK-8298405: Implement JEP 467: Markdown Documentation Comments [v50]

2024-03-16 Thread Jonathan Gibbons
> 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 with a new target base due to a 
merge or a rebase. The pull request now contains 69 commits:

 - Merge with upstream/master
 - Merge with upstream/master
 - improve first-sentence break in Markdown comments
 - add test cases for links to anchors
 - fix `textOf` method to accommodate Markdown content.
 - avoid relying on unspecified behavior `List.toString`
 - fix test after merge
 - Merge remote-tracking branch 'upstream/master' into 
8298405.doclet-markdown-v3
 - Revise `Markdown.update` to better handle container blocks.
 - Refactor most of TestMarkdown.java into separate tests, grouped by 
functionality
 - ... and 59 more: https://git.openjdk.org/jdk/compare/65a84c26...635af77d

-

Changes: https://git.openjdk.org/jdk/pull/16388/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=16388=49
  Stats: 23526 lines in 208 files changed: 22955 ins; 228 del; 343 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: 8325163: Enable -Wpedantic on clang [v2]

2024-03-16 Thread Magnus Ihse Bursie
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie  wrote:

>> Inspired by (the later backed-out) 
>> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to 
>> enable `-Wpedantic` for clang. This has already found some irregularities in 
>> the code, like mistakenly using `#import` instead of `#include`. In this 
>> patch, I disable warnings for these individual buggy or badly written files, 
>> but I intend to post follow-up issues on the respective teams to have them 
>> properly fixed.
>> 
>> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since 
>> individual warnings in `-Wpedantic` cannot be disabled. This means that code 
>> like this:
>> 
>> 
>> #define DEBUG_ONLY(code) code;
>> 
>> DEBUG_ONLY(foo());
>> 
>> 
>> will result in a `; ;`. This breaks the C standard, but is benign, and we 
>> use it all over the place. On clang, we can ignore this by 
>> `-Wno-extra-semi`, but this is not available on gcc.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   FIx dtrace build

Keep it for a bit more, bot.

-

PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-2000843635


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v20]

2024-03-16 Thread Mandy Chung
On Fri, 15 Mar 2024 10:53:29 GMT, Severin Gehwolf  wrote:

> Wasn't the intention to make "creating a linkable runtime image" a build only 
> decision and make the relevant infrastructure classes build-only artefacts?

Build-only decision means that the linkable runtime image is only produced by 
JDK build.   Supporting classes of this tool like the code generating the diffs 
can reside in `src/jdk.jlink` if possible as that helps keeping in other jlink 
code.

I misinterpreted that this is no longer a build tool approach as I was confused 
when seeing the source files are moved to `src/jdk.jlink`.   The build tool can 
stay in unnamed module as before but the tool would need to run on the newly 
built or interim JDK (not the boot JDK) which explains why the code of 
generating the diffs can be part of jlink.

Regarding my comment to "only compile those classes when JDK is configured to 
build linkable images" was premature.  I thought of some complexity last night 
and if it could be filtered easily, it would be a bonus.   If not, also no 
issue.

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2000188399


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v20]

2024-03-16 Thread Mandy Chung
On Fri, 15 Mar 2024 09:55:15 GMT, Severin Gehwolf  wrote:

> > If `--enable-runtime-link-image` is enabled, the JDK image does not include 
> > the packaged modules.
> 
> That's not true. Right now `--enable-runtime-link-image` modifies how the 
> `lib/modules` image looks like (adds a bunch of extra resources). That's it. 
> It doesn't modify the setup of packaged modules. 

It is true that they are orthogonal.   jlink does allow to produce a linkable 
image with `--keep-packaged-modules` and the resulting JDK image would work.  

However, the goal of this work is to produce a JDK image with smaller 
footprint.   This is a question to JDK build to allow configuring building a 
linkable image with packaged modules.

In addition, `--enable-keep-packaged-modules` is enabled by default.Do you 
want the linkable image includes `jmods` as it's currently implemented in this 
PR?   I assume not.

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2000322485


Re: RFR: 8326964: Remove Eclipse Shared Workspaces [v4]

2024-03-16 Thread Julian Waters
On Fri, 15 Mar 2024 15:44:48 GMT, Erik Joelsson  wrote:

>> Julian Waters has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Address review comments in CreateWorkspace.gmk
>
> Marked as reviewed by erikj (Reviewer).

@erikj79 Sorry it seems I forgot to remove the SHARED definition from the 
Makefile, could I get a re-review? Thanks
@magicus Also waiting for your approval

-

PR Comment: https://git.openjdk.org/jdk/pull/18046#issuecomment-2001849914


Re: RFR: 8326964: Remove Eclipse Shared Workspaces [v5]

2024-03-16 Thread Julian Waters
> Eclipse Shared Workspaces share the same directory as the JDK itself, which 
> does not make much sense since there can be multiple different configurations 
> of the JDK which Eclipse does depend on (as minimalistic as it is), and there 
> is no one true JDK configuration, unlike what this setting suggests. This 
> feature was created when I was much less familiar with the structure of how 
> the JDK handles different builds, and should be removed

Julian Waters has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove SHARED definition in Main.gmk

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18046/files
  - new: https://git.openjdk.org/jdk/pull/18046/files/212fc3c1..4287082a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18046=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=18046=03-04

  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/18046.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18046/head:pull/18046

PR: https://git.openjdk.org/jdk/pull/18046


Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]

2024-03-16 Thread Andrew Haley
On Fri, 15 Mar 2024 11:35:05 GMT, Hamlin Li  wrote:

> > I'm running the tests, but I have no way to confirm that SLEEF or SVE is in 
> > use. Exactly what did you do, and how did you confirm it?
> 
> Thanks for running test. I just turn on some log, and check the output.

The problem I see is that J. Random Java User has no way to know if SLEEF is 
making their program faster without running benchmarks. They'll put SLEEF 
somewhere and hope that Java uses it.

-

PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-276965


Re: RFR: 8328074: Add jcheck whitespace checking for assembly files [v4]

2024-03-16 Thread Sandhya Viswanathan
On Wed, 13 Mar 2024 11:59:36 GMT, Magnus Ihse Bursie  wrote:

>> As part of the ongoing effort to enable jcheck whitespace checking to all 
>> text files, it is now time to address assembly (.S) files. The hotspot 
>> assembly files were fixed as part of the hotspot mapfile removal, so only a 
>> single incorrect jdk library remains.
>> 
>> The main issue with `libjsvml` was inconsistent line starts, sometimes using 
>> tabs. I used the `expand` unix command line tool to replace these with 
>> spaces.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Make all ALIGN/.align aligned

Looks good to me.

-

Marked as reviewed by sviswanathan (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18268#pullrequestreview-1940824156


Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]

2024-03-16 Thread Andrew Haley
On Fri, 15 Mar 2024 13:58:05 GMT, Hamlin Li  wrote:

>> Hi,
>> Can you help to review this patch?
>> Thanks
>> 
>> This is a continuation of work based on [1] by @XiaohongGong, most work was 
>> done in that pr. In this new pr, just rebased the code in [1], then added 
>> some minor changes (renaming of calling method, add libsleef as extra lib in 
>> CI cross-build on aarch64 in github workflow); I aslo tested the combination 
>> of following scenarios:
>> * at build time
>>   * with/without sleef
>>   * with/without sve support 
>> * at runtime
>>   * with/without sleef
>>   * with/without sve support 
>> 
>> [1] https://github.com/openjdk/jdk/pull/16234
>> 
>> ## Regression Test
>> * test/jdk/jdk/incubator/vector/
>> * test/hotspot/jtreg/compiler/vectorapi/
>> 
>> ## Performance Test
>> Previously, @XiaohongGong has shared the data: 
>> https://github.com/openjdk/jdk/pull/16234#issuecomment-1767727028
>
> Hamlin Li has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix jni includes

> > What is the relevance of SVE support at build time? Should it matter what 
> > the build machine is?
> > Its important to realize that almost no one except the JDK devs builds 
> > their own JDK, and having SLEEF dependencies at build time will mean that 
> > almost no one will use it. All this work you've done will be for nothing.
> 
> I agree. On the other hand, I see previously there was discussion about this 
> already, and current solution got some points [#16234 
> (comment)](https://github.com/openjdk/jdk/pull/16234#issuecomment-1836266314),
>  [#16234 
> (comment)](https://github.com/openjdk/jdk/pull/16234#issuecomment-1836449876)
> 
> So, currently maybe we could go with this solution first, and as an 
> incremental optimization, we could introduce a dynamic loading without 
> dependency on the bridge furnctions in libvmath at runtime 
> ([pr](https://bugs.openjdk.org/browse/JDK-8328268)). How do you think about 
> it?

If there was some kind of plan, with evidence of the intention to do something 
to get this valuable tech into people's hands in a form they can use, sure. But 
as you can tell, I think this may rot because no one will be able use it. If 
SLEEF were included in the JDK it'd be fine. if SLEEF were a common Linux 
library it'd be fine.

-

PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-273532