Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 9 Apr 2024 19:20:22 GMT, Kim Barrett wrote: >> Joachim Kern has updated the pull request incrementally with one additional >> commit since the last revision: >> >> version check not needed anymore > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 36: > >> 34: #if defined(_AIX) >> 35: #include >> 36: #endif > > I would much rather see this include added in the few places it was actually > needed, rather than being > added here. Do we even need to include ? >From the Linux man page for alloca: By necessity, alloca() is a compiler built-in, also known as __builtin_alloca(). By default, modern compilers automatically translate all uses of alloca() into the built-in, but this is forbidden if standards conformance is requested (-ansi, -std=c*), in which case is required, lest a symbol dependency be emitted. There are uses of it in shared code where there isn't an applicable include, other than from globalDefinitions_xlc.hpp. So it appears all other supported compilers do treat it as a built-in with the options we are providing, and don't need the include. Maybe that's true for the new xlc compiler too? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558565268
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 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? - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2045972249
RFR: 8329970: Update autoconf build-aux files with latest from 2024-01-01
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. - Commit messages: - JDK-8329970 Changes: https://git.openjdk.org/jdk/pull/18704/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18704=00 Issue: https://bugs.openjdk.org/browse/JDK-8329970 Stats: 224 lines in 2 files changed: 126 ins; 24 del; 74 mod Patch: https://git.openjdk.org/jdk/pull/18704.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18704/head:pull/18704 PR: https://git.openjdk.org/jdk/pull/18704
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 16:14:12 GMT, Joachim Kern wrote: >> 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: > > version check not needed anymore Changes requested by kbarrett (Reviewer). src/hotspot/share/utilities/byteswap.hpp line 2: > 1: /* > 2: * Copyright (c) 2023, Google and/or its affiliates. All rights reserved. Don't drop the creation year. src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 36: > 34: #if defined(_AIX) > 35: #include > 36: #endif I would much rather see this include added in the few places it was actually needed, rather than being added here. - PR Review: https://git.openjdk.org/jdk/pull/18536#pullrequestreview-1989864573 PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558124034 PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558172309
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 9 Apr 2024 17:01:59 GMT, Thomas Stuefe wrote: >> @suchismith1993 >> Hi Suchi, can you please tell me when you will raise your build environment >> from AIX 7.2 TL5 SP5 to SP7? >> I' am asking you, because I want to get rid of this nasty workaround. > > Pinging @sxa - what build environment does temurin use for AIX? Currently XLC16 but looking to upgrade to XLC17 on the minimum supported level for it (So it wouldn't be SP7 at present). Thanks for the ping - we have no current plans to increase to SP7. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558053537
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 09:19:16 GMT, Joachim Kern wrote: >> Hi Thomas, >> I would like to get totally rid of this, because as I mentioned IBM already >> modified the `stdlib.h` header not using `#define malloc vec_malloc` any >> more (and all the other vec_... defines). We have to ask the adoptium >> colleagues at IBM if they already have raised their build environment by the >> 2 SP levels needed. >> In principle we had to do the same workaround for `calloc, free,...` too, >> but they didn't show up as errors in the logging files. >> These lines where never meant to stay for long. Just to be able to compile >> until IBM fixes the issue, which is done now. > > @suchismith1993 > Hi Suchi, can you please tell me when you will raise your build environment > from AIX 7.2 TL5 SP5 to SP7? > I' am asking you, because I want to get rid of this nasty workaround. Pinging @sxa - what build environment does temurin use for AIX? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558020493
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 16:14:12 GMT, Joachim Kern wrote:
>> 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:
>
> version check not needed anymore
src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp line 440:
> 438: st->print("pc =" INTPTR_FORMAT " ", (unsigned
> long)uc->uc_mcontext.jmp_context.iar);
> 439: st->print("lr =" INTPTR_FORMAT " ", (unsigned
> long)uc->uc_mcontext.jmp_context.lr);
> 440: st->print("ctr=" INTPTR_FORMAT " ", (unsigned
> long)uc->uc_mcontext.jmp_context.ctr);
p2i
src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp line 443:
> 441: st->cr();
> 442: for (int i = 0; i < 32; i++) {
> 443: st->print("r%-2d=" INTPTR_FORMAT " ", i, (unsigned
> long)uc->uc_mcontext.jmp_context.gpr[i]);
p2i
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558017408
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558017827
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 10:26:42 GMT, Joachim Kern wrote:
>> src/hotspot/os/aix/os_aix.cpp line 314:
>>
>>> 312: ErrnoPreserver ep;
>>> 313: log_trace(os, map)("disclaim failed: " RANGEFMT " errno=(%s)",
>>> 314: RANGEFMTARGS(p, (long)maxDisclaimSize),
>>
>> Wait, why are these casts needed? maxDisclaimSize is size_t, RANGEFMT uses
>> SIZE_FORMAT. That should work without cast.
>
> Hi Thomas, `maxDisclaimSize` is of type `unsigned int`; therefore I get the
> following warning:
>
> os/aix/os_aix.cpp:314:42: error: format specifies type 'unsigned long' but
> the argument has type 'unsigned int' [-Werror,-Wformat]
> RANGEFMTARGS(p, maxDisclaimSize),
> ^~~
>
> Should I keep the casts, or change the type of `maxDisclaimSize,
> numFullDisclaimsNeeded, lastDisclaimSize` to `const unsigned long`?
I would change them to size_t. Thanks for doing this.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558012122
Re: RFR: 8317376: Minor improvements to the 'this' escape analyzer [v4]
> Please review several fixes and improvements to the `this-escape` lint > warning analyzer. > > The goal here is to apply some relatively simple logical fixes that improve > the precision and accuracy of the analyzer, and capture the remaining > low-hanging fruit so we can consider the analyzer relatively complete with > respect to what's feasible with its current design. > > Although the changes are small from a logical point of view, they generate a > fairly large patch due to impact of refactoring (sorry!). Most of the patch > derives from the first two changes listed below. > > The changes are summarized here: > > 1. Generalize how we categorize references > > The `Ref` class hierarchy models the various ways in which, at any point > during the execution of a constructor or some other method/constructor that > it invokes, there can be live references to the original object under > construction lying around. We then look for places where one of these `Ref`'s > might be passed to a subclass method. In other words, the analyzer keeps > track of these references and watches what happens to them as the code > executes so it can catch them trying to "escape". > > Previously the `Ref` categories were: > * `ThisRef` - The current instance of the (non-static) method or constructor > being analyzed > * `OuterRef` - The current outer instance of the (non-static) method or > constructor being analyzed > * `VarRef` - A local variable or method parameter currently in scope > * `ExprRef` - An object reference sitting on top of the Java execution stack > * `YieldRef` - The current switch expression's yield value(s) > * `ReturnRef` - The current method's return value(s) > > For each of those types, we further classified the "indirection" of the > reference, i.e., whether the reference was direct (from the thing itself) or > indirect (from something the thing referenced). > > The problem with that hierarchy is that we could only track outer instance > references that happened to be associated with the current instance. So we > might know that `this` had an outer instance reference, but if we said `var x > = this` we wouldn't know that `x` had an outer instance reference. > > In other words, we should be treating "via an outer instance" as just another > flavor of indirection along with "direct" and "indirect". > > As a result, with this patch the `OuterRef` class goes away and a new > `Indirection` enum has been created, with values `DIRECT`, `INDIRECT`, and > `OUTER`. > > 2. Track the types of all references > > By keeping track of the actual type of... Archie Cobbs has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits: - Merge branch 'master' into JDK-8317376 - Merge branch 'master' into JDK-8317376 - Merge branch 'master' into JDK-8317376 - Merge branch 'master' into JDK-8317376 - Merge branch 'master' into JDK-8317376 - Javadoc++ - Merge branch 'master' into JDK-8317376 - Several improvements to the 'this' escape analyzer. - Track direct, indirect, and outer references for all Ref types. - Keep type information about all references to improve tracking precision. - Track enhanced for() invocations of iterator(), hasNext(), and next(). - Don't report an escape of a non-public outer instances as a leak. - Fix omitted tracking of references from newly instantiated instances. - Fix omitted tracking of leaks via lambda return values. - Remove unneccesary suppressions of this-escape lint warning. - Changes: https://git.openjdk.org/jdk/pull/16208/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16208=03 Stats: 869 lines in 19 files changed: 513 ins; 146 del; 210 mod Patch: https://git.openjdk.org/jdk/pull/16208.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16208/head:pull/16208 PR: https://git.openjdk.org/jdk/pull/16208
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 Just a quick update, this pr introduces some performance regression compared with previous version (https://github.com/openjdk/jdk/pull/18294) for some math functions (e.g. Double256Vector.COS), and no regression for some others (e.g. Double256Vector.ACOS). I'm investigating. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2045495626
Integrated: 8328614: hsdis: dlsym can't find decode symbol
On Wed, 20 Mar 2024 16:17:36 GMT, Robbin Ehn wrote: > Hi, please consider. > > [8327045](https://bugs.openjdk.org/browse/JDK-8327045) hide these symbols. > Tested with gcc and clang, and llvm and binutils backend. > > I didn't find any use of the "DLL_ENTRY", so I removed it. > > Thanks, Robbin This pull request has now been integrated. Changeset: 1e02a13a Author:Robbin Ehn URL: https://git.openjdk.org/jdk/commit/1e02a13a7f02a6fe9aac38b93935bcc238f7d227 Stats: 18 lines in 5 files changed: 8 ins; 8 del; 2 mod 8328614: hsdis: dlsym can't find decode symbol Reviewed-by: ihse, luhenry, mli - PR: https://git.openjdk.org/jdk/pull/18400
Integrated: 8324673: javacserver failed during build: RejectedExecutionException
On Mon, 8 Apr 2024 09:20:44 GMT, Daniel Jeliński wrote: > The RejectedExecutionException was thrown when the thread executing > `Server.start` managed to shut down the `compilerThreadPool` before the > thread executing `Server.handleRequest` submitted the compilation task. > > This patch removes the extra thread used for `Server.handleRequest`, and > executes that method directly in the thread pool. All `compilerThreadPool` > uses happen on the `Server.start` thread now, and no new tasks are submitted > after the thread pool is shut down. > > In order to verify the fix, I modified `IdleMonitor.KEEPALIVE` to 1 second. > With that change the problem was occasionally reproducible without the patch > from this PR. With the patch, the `RejectedExecutionException` problem did > not reproduce. > > No new regression test. Existing langtools tests continue to pass. This pull request has now been integrated. Changeset: 3b6629ce Author:Daniel Jeliński URL: https://git.openjdk.org/jdk/commit/3b6629cec7a2ecec8dcb5b94d8ed3e169483aa97 Stats: 19 lines in 2 files changed: 1 ins; 10 del; 8 mod 8324673: javacserver failed during build: RejectedExecutionException Reviewed-by: cstein, erikj - PR: https://git.openjdk.org/jdk/pull/18672
Re: RFR: 8324673: javacserver failed during build: RejectedExecutionException
On Mon, 8 Apr 2024 09:20:44 GMT, Daniel Jeliński wrote: > The RejectedExecutionException was thrown when the thread executing > `Server.start` managed to shut down the `compilerThreadPool` before the > thread executing `Server.handleRequest` submitted the compilation task. > > This patch removes the extra thread used for `Server.handleRequest`, and > executes that method directly in the thread pool. All `compilerThreadPool` > uses happen on the `Server.start` thread now, and no new tasks are submitted > after the thread pool is shut down. > > In order to verify the fix, I modified `IdleMonitor.KEEPALIVE` to 1 second. > With that change the problem was occasionally reproducible without the patch > from this PR. With the patch, the `RejectedExecutionException` problem did > not reproduce. > > No new regression test. Existing langtools tests continue to pass. Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/18672#issuecomment-2045276658
RFR: JDK-8329617: Update stylesheet for specs and tool documentation
Please review an update to the `jdk-default.css` stylesheet used for specifications and tool guides. The original purpose was to make use of the Dejavu web fonts provided by the API docs and to update the navigation bar to match the one in the API docs. However, the updates include some other fixes and improvements also described below. - The change to use the DejaVu web fonts consists only of the `@import` statement in line 16 as the stylesheet already used DejaVu web fonts as first choice in its `font-family` rules. - The changes to make the navigation bar match the one in the API docs are mostly located at the end of the file (beyond line 160). However, this also includes setting the `margin` property to '0' in the `body` element and adding a `margin` in the `main` and `footer` elements instead. - To set the horizontal margin for page content elements outside the `main` element which occur in some pages, a margin is set explicitly on those elements in lines 48-50. While this is a bit awkward, I think it's still better than working with negative margins in the header to offset the margin in the `body` element. - Most of the remaining changes (lines 53-110) are changes are to redefine the styles in simpler terms, such as leaving out declarations that are equal to browser defaults, and removing the units from `0`-length values. The changes are intended to preserve the layout of the pages, including the body font size which is slightly different from the one used in API docs (`10pt` vs `14px`). I can provide before/after snapshots of the rendered documentation if desired. - Commit messages: - JDK-8329617: Update stylesheet for specs and tool documentation Changes: https://git.openjdk.org/jdk/pull/18694/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18694=00 Issue: https://bugs.openjdk.org/browse/JDK-8329617 Stats: 73 lines in 1 file changed: 27 ins; 19 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/18694.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18694/head:pull/18694 PR: https://git.openjdk.org/jdk/pull/18694
Re: RFR: JDK-8298405: Implement JEP 467: Markdown Documentation Comments [v55]
On Mon, 8 Apr 2024 21:12:42 GMT, Jonathan Gibbons wrote:
>> 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:
>
> add support for JDK-8329296: Update Elements for '///' documentation
> comments
Changes for 21f5b00 mostly look good, but I'm not a compiler person.
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)));
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)));
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocCommentTable.java
line 55:
> 53:
> 54: /**
> 55: * Get the plain text of the doc comment, if any, for a tree node.
This is likely a copy-pasted comment.
-
PR Review: https://git.openjdk.org/jdk/pull/16388#pullrequestreview-1988489764
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1557248565
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1557249290
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1557444619
Re: RFR: 8328614: hsdis: dlsym can't find decode symbol [v3]
On Fri, 5 Apr 2024 10:45:24 GMT, Robbin Ehn wrote: >> Hi, please consider. >> >> [8327045](https://bugs.openjdk.org/browse/JDK-8327045) hide these symbols. >> Tested with gcc and clang, and llvm and binutils backend. >> >> I didn't find any use of the "DLL_ENTRY", so I removed it. >> >> Thanks, Robbin > > Robbin Ehn has updated the pull request incrementally with one additional > commit since the last revision: > > Use JNIEXPORT Looks good. Although I'm not the right person to review this, thanks for explanation and discussion. - Marked as reviewed by mli (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18400#pullrequestreview-1988652730
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v23]
On Wed, 3 Apr 2024 22:31:39 GMT, Mandy Chung wrote: >> Severin Gehwolf has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Move CreateLinkableRuntimePlugin to build folder >> >> Keep runtime link supporting classes in package >> jdk.tools.jlink.internal.runtimelink > > Thanks for the investigation w.r.t. extending jimage. It does not seem worth > the effort in pursuing the support of adding resources to an existing jimage > file. To me, putting the diff data under `lib` directory as a private file > is a simpler and acceptable solution. It allows the second jlink invocation > to avoid the plugin pipeline if the per-module metadata is generated in > normal jlink invocation. > > (An observation: The current implementation requires the diffs to be > generated as a plugin. For this approach, it may be possible to implement > with a separate main class that calls JLink API and generates the diffs.) @mlchung @AlanBateman Any thoughts on this latest version? Is this going into the direction you had envisioned? Any more blockers? The CSR should be up-to-date and is open for review as well. If no more blockers I'll go over this patch once more and clean it up to prepare for integration. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2044593102
Re: RFR: 8328614: hsdis: dlsym can't find decode symbol [v3]
On Fri, 5 Apr 2024 10:45:24 GMT, Robbin Ehn wrote: >> Hi, please consider. >> >> [8327045](https://bugs.openjdk.org/browse/JDK-8327045) hide these symbols. >> Tested with gcc and clang, and llvm and binutils backend. >> >> I didn't find any use of the "DLL_ENTRY", so I removed it. >> >> Thanks, Robbin > > Robbin Ehn has updated the pull request incrementally with one additional > commit since the last revision: > > Use JNIEXPORT Yea, I know, but if hsdis is not an external library I considered it part of hotspot as it is the only use, hence I want two reviews. :) - PR Comment: https://git.openjdk.org/jdk/pull/18400#issuecomment-2044575768
Re: RFR: 8328614: hsdis: dlsym can't find decode symbol [v3]
On Fri, 5 Apr 2024 10:45:24 GMT, Robbin Ehn wrote: >> Hi, please consider. >> >> [8327045](https://bugs.openjdk.org/browse/JDK-8327045) hide these symbols. >> Tested with gcc and clang, and llvm and binutils backend. >> >> I didn't find any use of the "DLL_ENTRY", so I removed it. >> >> Thanks, Robbin > > Robbin Ehn has updated the pull request incrementally with one additional > commit since the last revision: > > Use JNIEXPORT FYI: The 2-reviewer rule only applies to Hotspot (and some other specific parts of the code base), and not generally for JDK fixes. So you had been fine to push with just one reviewer. - PR Comment: https://git.openjdk.org/jdk/pull/18400#issuecomment-2044521203
Re: RFR: 8328614: hsdis: dlsym can't find decode symbol [v3]
On Tue, 9 Apr 2024 08:06:19 GMT, Ludovic Henry wrote: >> Robbin Ehn has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Use JNIEXPORT > > Marked as reviewed by luhenry (Committer). Thank you @luhenry ! - PR Comment: https://git.openjdk.org/jdk/pull/18400#issuecomment-2044496543
Re: RFR: 8328614: hsdis: dlsym can't find decode symbol [v3]
On Fri, 5 Apr 2024 10:45:24 GMT, Robbin Ehn wrote: >> Hi, please consider. >> >> [8327045](https://bugs.openjdk.org/browse/JDK-8327045) hide these symbols. >> Tested with gcc and clang, and llvm and binutils backend. >> >> I didn't find any use of the "DLL_ENTRY", so I removed it. >> >> Thanks, Robbin > > Robbin Ehn has updated the pull request incrementally with one additional > commit since the last revision: > > Use JNIEXPORT Marked as reviewed by luhenry (Committer). - PR Review: https://git.openjdk.org/jdk/pull/18400#pullrequestreview-1988392854
