Re: RFR: JDK-8256844: Make NMT late-initializable
On Thu, 22 Jul 2021 14:58:47 GMT, Thomas Stuefe wrote: > Short: this patch makes NMT available in custom-launcher scenarios and during > gtests. It simplifies NMT initialization. It adds a lot of NMT-specific > testing, cleans them up and makes them sideeffect-free. > > - > > NMT continues to be an extremely useful tool for SAP to tackle memory > problems in the JVM. > > However, NMT is of limited use due to the following restrictions: > > - NMT cannot be used if the hotspot is embedded into a custom launcher unless > the launcher actively cooperates. Just creating and invoking the JVM is not > enough, it needs to do some steps prior to loading the hotspot. This > limitation is not well known (nor, do I believe, documented). Many products > don't do this, e.g., you cannot use NMT with IntelliJ. For us at SAP this > problem limits NMT usefulness greatly since our VMs are often embedded into > custom launchers and modifying every launcher is impossible. > - Worse, if that custom launcher links the libjvm *statically* there is just > no way to activate NMT at all. This is the reason NMT cannot be used in the > `gtestlauncher`. > - Related to that is that we cannot pass NMT options via `JAVA_TOOL_OPTIONS` > and `-XX:Flags=`. > - The fact that NMT cannot be used in gtests is really a pity since it would > allow us to both test NMT itself more rigorously and check for memory leaks > while testing other stuff. > > The reason for all this is that NMT initialization happens very early, on the > first call to `os::malloc()`. And those calls happen already during dynamic > C++ initialization - a long time before the VM gets around parsing arguments. > So, regular VM argument parsing is too late to parse NMT arguments. > > The current solution is to pass NMT arguments via a specially prepared > environment variable: `NMT_LEVEL_=`. That environment > variable has to be set by the embedding launcher, before it loads the libjvm. > Since its name contains the PID, we cannot even set that variable in the > shell before starting the launcher. > > All that means that every launcher needs to especially parse and process the > NMT arguments given at the command line (or via whatever method) and prepare > the environment variable. `java` itself does this. This only works before the > libjvm.so is loaded, before its dynamic C++ initialization. For that reason, > it does not work if the launcher links statically against the hotspot, since > in that case C++ initialization of the launcher and hotspot are folded into > one phase with no possibility of executing code beforehand. > > And since it bypasses argument handling in the VM, it bypasses a number of > argument processing ways, e.g., `JAVA_TOOL_OPTIONS`. > > -- > > This patch fixes these shortcomings by making NMT late-initializable: it can > now be initialized after normal VM argument parsing, like all other parts of > the VM. This greatly simplifies NMT initialization and makes it work > automagically for every third party launcher, as well as within our gtests. > > The glaring problem with late-initializing NMT is the NMT malloc headers. If > we rule out just always having them (unacceptable in terms of memory > overhead), there is no safe way to determine, in os::free(), if an allocation > came from before or after NMT initialization ran, and therefore what to do > with its malloc headers. For a more extensive explanation, please see the > comment block `nmtPreInit.hpp` and the discussion with @kimbarrett and > @zhengyu123 in the JBS comment section. > > The heart of this patch is a new way to track early, pre-NMT-init > allocations. These are tracked via a lookup table. This was a suggestion by > Kim and it worked out well. > > Changes in detail: > > - pre-NMT-init handling: > - the new files `nmtPreInit.hpp/cpp` take case of NMT pre-init > handling. They contain a small global lookup table managing C-heap blocks > allocated in the pre-NMT-init phase. > - `os::malloc()/os::realloc()/os::free()` defer to this code before > doing anything else. > - Please see the extensive comment block at the start of > `nmtPreinit.hpp` explaining the details. > > - Changes to NMT: > - Before, NMT initialization was spread over two phases, `initialize()` > and `late_initialize()`. Those were merged into one and simplified - there is > only one initialization now which happens after argument parsing. > - Minor changes were needed for the `NMT_TrackingLevel` enum - to > simplify code, I changed NMT_unknown to be numerically 0. A new comment block > in `nmtCommon.hpp` now clearly specifies what's what, including allowed level > state transitions. > - New utility functions to translate tracking level from/to strings > added to `NMTUtil` > - NMT has never been able to handle virtual memory allocations before > initialization, which is fine since os::reserve_memory() is not
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package
On Thu, 29 Jul 2021 01:30:37 GMT, Vladimir Kozlov wrote: >> Hi all, >> >> could you please review this big tedious and trivial(-ish) patch which moves >> `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? >> >> the majority of the patch is the following substitutions: >> - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` >> - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` >> - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` >> - `s/sun.hotspot.code/jdk.test.whitebox.code/g` >> - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` >> - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` >> >> testing: tier1-4 >> >> Thanks, >> -- Igor > > I know that tests fixes could be pushed during RDP2 without approval. > But these one is very big and it is not a fix - it is enhancement. > > What is the reason you want to push it into JDK 17 just few days before first > Release Candidate? Instead of pushing it into JDK 18. > > And I can't even review it because GutHub UI hangs on these big changes. I agree with @vnkozlov this too big and disruptive for 17 at this stage of the release. I also think a better approach to this cleanup would have been to copy the WhiteBox class to the new package structure, then update the tests in chunks, then remove the old WhiteBox classes when that is complete. Thanks, David - PR: https://git.openjdk.java.net/jdk17/pull/290
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package
On Wed, 28 Jul 2021 17:13:49 GMT, Igor Ignatyev wrote: > Hi all, > > could you please review this big tedious and trivial(-ish) patch which moves > `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? > > the majority of the patch is the following substitutions: > - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` > - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` > - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` > - `s/sun.hotspot.code/jdk.test.whitebox.code/g` > - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` > - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` > > testing: tier1-4 > > Thanks, > -- Igor I know that tests fixes could be pushed during RDP2 without approval. But these one is very big and it is not a fix - it is enhancement. What is the reason you want to push it into JDK 17 just few days before first Release Candidate? Instead of pushing it into JDK 18. And I can't even review it because GutHub UI hangs on these big changes. - PR: https://git.openjdk.java.net/jdk17/pull/290
[jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package
Hi all, could you please review this big tedious and trivial(-ish) patch which moves `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? the majority of the patch is the following substitutions: - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` - `s/sun.hotspot.code/jdk.test.whitebox.code/g` - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` testing: tier1-4 Thanks, -- Igor - Commit messages: - copyright year - update TEST.ROOT - jdk: s/sun\.hotspot\.gc/jdk.test.whitebox.gc/g - jdk: s/sun\.hotspot\.code/jdk.test.whitebox.code/g - jdk: s/sun\.hotspot\.WhiteBox/jdk.test.whitebox.WhiteBox/g - hotspot: 's~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g' - hotspot: s/sun\.hotspot\.parser/jdk.test.whitebox.parser/g - hotspot: s/sun\.hotspot\.cpuinfo/jdk.test.whitebox.cpuinfo/g - hotspot: s/sun\.hotspot\.code/jdk.test.whitebox.code/g - hotspot: 's/sun\.hotspot\.gc/jdk.test.whitebox.gc/g' - ... and 9 more: https://git.openjdk.java.net/jdk17/compare/c8ae7e5b...8f12f2cf Changes: https://git.openjdk.java.net/jdk17/pull/290/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=290=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8067223 Stats: 2310 lines in 949 files changed: 0 ins; 0 del; 2310 mod Patch: https://git.openjdk.java.net/jdk17/pull/290.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/290/head:pull/290 PR: https://git.openjdk.java.net/jdk17/pull/290
Re: RFR: 8266936: Add a finalization JFR event [v4]
On Tue, 27 Jul 2021 15:14:29 GMT, Markus Grönlund wrote: >> Greetings, >> >> Object.finalize() was deprecated in JDK9. There is an ongoing effort to >> replace and mitigate Object.finalize() uses in the JDK libraries; please see >> https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. >> >> We also like to assist users in replacing and mitigating uses in non-JDK >> code. >> >> Hence, this changeset adds a periodic JFR event to help identify which >> classes are overriding Object.finalize(). >> >> Thanks >> Markus > > Markus Grönlund has updated the pull request incrementally with one > additional commit since the last revision: > > remove build directive Hi, Bernd The JFR event can complement static bytecode analysis, reporting which classes with finalizers actually get loaded at runtime. I agree that it would be useful to add a count for finalizers that get run, either as part of this PR, or as a follow-up. I would hesitate to add timing, though. It's important to keep the overhead of this event low, and finalize() methods should already show up in current profilers. Thanks, -Brent > _Mailing list message from [Bernd Eckenfels](mailto:e...@zusammenkunft.net) > on [core-libs-dev](mailto:core-libs-...@mail.openjdk.java.net):_ > > Hello, > > I know I am a bit late, but just wanted to mention, that since finding > finalizers with Bytecode analysis is doable (and probably easier to deal with > such scan reports), I don?t see much value in a JFR event, especially > considering it even has native code executed. (Not so sure about dynamically > loaded classes, would the event content help to identify sources?) > > Having said that, this event would be more useful from a runtime perspective > if It would actually record execution counts and time per class. Then one can > concentrate on the worst offenders, first and even use it for runtime > monitoring. Is there already a finalizer profiler? > > Gruss > Bernd - PR: https://git.openjdk.java.net/jdk/pull/4731
[jdk17] Integrated: 8271412: ProblemList javax/sound/midi/Sequencer/Looping.java
On Wed, 28 Jul 2021 17:51:31 GMT, Daniel D. Daugherty wrote: > 8271412: ProblemList javax/sound/midi/Sequencer/Looping.java This pull request has now been integrated. Changeset: 7bf72ce3 Author:Daniel D. Daugherty URL: https://git.openjdk.java.net/jdk17/commit/7bf72ce301de80f4126607c2ef51d6df8c5849cf Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod 8271412: ProblemList javax/sound/midi/Sequencer/Looping.java 8271413: ProblemList 2 locale tests on macOS-x64 Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk17/pull/291
Re: [jdk17] RFR: 8271412: ProblemList javax/sound/midi/Sequencer/Looping.java
On Wed, 28 Jul 2021 18:42:11 GMT, Naoto Sato wrote: >> 8271412: ProblemList javax/sound/midi/Sequencer/Looping.java > > Marked as reviewed by naoto (Reviewer). @naotoj - Thanks for the fast review! - PR: https://git.openjdk.java.net/jdk17/pull/291
Re: [jdk17] RFR: 8271412: ProblemList javax/sound/midi/Sequencer/Looping.java
On Wed, 28 Jul 2021 17:51:31 GMT, Daniel D. Daugherty wrote: > 8271412: ProblemList javax/sound/midi/Sequencer/Looping.java Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/291
[jdk17] RFR: 8271412: ProblemList javax/sound/midi/Sequencer/Looping.java
8271412: ProblemList javax/sound/midi/Sequencer/Looping.java - Commit messages: - 8271413: ProblemList 2 locale tests on macOS-x64 - 8271412: ProblemList javax/sound/midi/Sequencer/Looping.java Changes: https://git.openjdk.java.net/jdk17/pull/291/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=291=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271412 Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk17/pull/291.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/291/head:pull/291 PR: https://git.openjdk.java.net/jdk17/pull/291
Re: RFR: 8271396: Spelling errors
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg wrote: > This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed jdi, jvmti, and dcmd related changes look good. - Marked as reviewed by cjplummer (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8271396: Spelling errors
On Wed, 28 Jul 2021 17:08:01 GMT, Emmanuel Bourg wrote: >> I'm happy to sponsor this change, but could you please update the copyright >> year where necessary, e.g. in >> src/java.desktop/unix/classes/sun/awt/X11/XMSelection.java: >> `Copyright (c) 2003, 2018, Oracle...` -> `Copyright (c) 2003, 2021, >> Oracle...` > > @FrauBoes thank you, the PR has been updated to modify the copyright year @ebourg for future PRs please do not force push after the PR is out for review. Just push incremental commits normally. The Skara tooling will squash them all into a single commit. - PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8271396: Spelling errors [v2]
On Wed, 28 Jul 2021 17:12:04 GMT, Emmanuel Bourg wrote: >> This PR fixes the following spelling errors: >> >> choosen -> chosen >> commad -> command >> hiearchy -> hierarchy >> leagacy -> legacy >> minium -> minimum >> subsytem -> subsystem >> unamed -> unnamed > > Emmanuel Bourg 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 one additional > commit since the last revision: > > 8271396: Fix spelling errors > >choosen -> chosen >commad -> command >hiearchy -> hierarchy >leagacy -> legacy >minium -> minimum >subsytem -> subsystem >unamed -> unnamed Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8271396: Spelling errors
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg wrote: > This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed Thanks for awt/swing correction. - Marked as reviewed by psadhukhan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8271396: Spelling errors
On Wed, 28 Jul 2021 17:20:37 GMT, Kevin Rushforth wrote: >> @FrauBoes thank you, the PR has been updated to modify the copyright year > > @ebourg for future PRs please do not force push after the PR is out for > review. Just push incremental commits normally. The Skara tooling will squash > them all into a single commit. @kevinrushforth I'll do that, thank you for the hint - PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8271396: Spelling errors [v2]
> This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed Emmanuel Bourg 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 one additional commit since the last revision: 8271396: Fix spelling errors choosen -> chosen commad -> command hiearchy -> hierarchy leagacy -> legacy minium -> minimum subsytem -> subsystem unamed -> unnamed - Changes: - all: https://git.openjdk.java.net/jdk/pull/2385/files - new: https://git.openjdk.java.net/jdk/pull/2385/files/31cfcba7..6e1be4f6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2385=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2385=00-01 Stats: 61642 lines in 1361 files changed: 29147 ins; 26026 del; 6469 mod Patch: https://git.openjdk.java.net/jdk/pull/2385.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2385/head:pull/2385 PR: https://git.openjdk.java.net/jdk/pull/2385
Re: jpackage MacOS Notarization
Not really enough info given here to act on. Exactly what java version/build are you using? As Kevin suggested it best to try JDK17 EA first, but I can notarize simple test app with JDK16 , staple the notarization, and then download it and run it on other machines without the quarantine hacks. While implementing support for the Mac App Store in JDK17, we had to change the way signing works (we now unsign the java runtime and then re-sign it's components together with the app's components, where we previously used the signing already present in the released jdk.) For this reason I think it is better to look only at problem that persist in JDK17 at this time. /Andy On 7/28/2021 11:27 AM, Daniel Peintner wrote: All, I am trying to notarize an app (built with jpackage) for MacOS. jpackage at first *seems* to properly sign all resources with the available --mac-sign options et cetera. Having said that, there are still remaining issues 1. The app cannot be properly installed (without hacks like xattr -d com.apple.quarantine /Applications/myAPP.app ). This indicates the app is not notarized or the notarization is not properly stapled. 2. I am also not able to properly notarize the app. According to online resources there seem to exist issues in *past* about notarization but according to [1, 2] the issues are fixed. As mentioned, I still have issues :-( Am I really the only one still having problems? Java Version: AdoptOpenJDK-16.0.1+9 (tried Oracle JDK also without success) The issue seems to boil down to 2 errors (attached the error log from Apple notarization process). * app Folder * libjli.dylib From below it looks like you are trying to sign a dmg. Notarization or a jpackage artifact requires either a signed pkg or a zipped signed app image. It looks like notarizing a signed dmg is now supported by Apple, but this is not something that was available when we initially implemented this in jpackage. Can you try the same thing with a "pkg" instead of a "dmg". We will have to look into what is needed to sign "dmg" artifacts now. /Andy
Re: RFR: 8271396: Spelling errors
On Wed, 28 Jul 2021 16:26:49 GMT, Julia Boes wrote: >> This PR fixes the following spelling errors: >> >> choosen -> chosen >> commad -> command >> hiearchy -> hierarchy >> leagacy -> legacy >> minium -> minimum >> subsytem -> subsystem >> unamed -> unnamed > > I'm happy to sponsor this change, but could you please update the copyright > year where necessary, e.g. in > src/java.desktop/unix/classes/sun/awt/X11/XMSelection.java: > `Copyright (c) 2003, 2018, Oracle...` -> `Copyright (c) 2003, 2021, Oracle...` @FrauBoes thank you, the PR has been updated to modify the copyright year - PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8266054: VectorAPI rotate operation optimization [v13]
On Wed, 28 Jul 2021 04:48:35 GMT, Vladimir Kozlov wrote: >> Looks good to me. > > @sviswa7 and @jatin-bhateja jatin-bhateja > The push caused https://bugs.openjdk.java.net/browse/JDK-8271366 > I am strongly suggest in a future to ask an Oracle's engineer to test Intel's > changes before pushing. @vnkozlov @PaulSandoz Sorry for the inconvenience. @jatin-bhateja Please don't be in a hurry to push and reach out to Oracle engineers for testing before pushing. - PR: https://git.openjdk.java.net/jdk/pull/3720
Re: RFR: 8271396: Spelling errors
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg wrote: > This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed I'm happy to sponsor this change, but could you please update the copyright year where necessary, e.g. in src/java.desktop/unix/classes/sun/awt/X11/XMSelection.java: `Copyright (c) 2003, 2018, Oracle...` -> `Copyright (c) 2003, 2021, Oracle...` - PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8271396: Spelling errors
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg wrote: > This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2385
Re: jpackage MacOS Notarization
Full support for notarization in jpackage was added in JDK 17. Can you
try an early access build of JDK 17 [1] and see if that works for you?
-- Kevin
[1] https://jdk.java.net/17
On 7/28/2021 8:27 AM, Daniel Peintner wrote:
All,
I am trying to notarize an app (built with jpackage) for MacOS.
jpackage at first *seems* to properly sign all resources with the available
--mac-sign options et cetera.
Having said that, there are still remaining issues
1. The app cannot be properly installed
(without hacks like xattr -d com.apple.quarantine /Applications/myAPP.app
).
2. I am also not able to properly notarize the app.
According to online resources there seem to exist issues in *past* about
notarization but according to [1, 2] the issues are fixed.
As mentioned, I still have issues :-(
Am I really the only one still having problems?
Java Version: AdoptOpenJDK-16.0.1+9 (tried Oracle JDK also without success)
The issue seems to boil down to 2 errors (attached the error log from Apple
notarization process).
* app Folder
* libjli.dylib
I thought I better ask first on the mailing list before creating an actual
bug report.
Note1: I used to use the *old* javapackager that worked with the same
signature/credentials.
Note2: running jpackage without --mac-sign options causes many more errors
in notarization (Hence, jpackage signs most resources but fails with some)
Any help / hint appreciated.
Thanks,
-- Daniel
[1] https://bugs.openjdk.java.net/browse/JDK-8257488
[2] https://bugs.openjdk.java.net/browse/JDK-8251892
## APPLE LOG ##
{
"logFormatVersion": 1,
"jobId": "...",
"status": "Invalid",
"statusSummary": "Archive contains critical validation errors",
"statusCode": 4000,
"archiveFilename": "myAPP-21.07.28.dmg",
"uploadDate": "2021-07-28T14:31:24Z",
"sha256": "...",
"ticketContents": null,
"issues": [
{
"severity": "error",
"code": null,
"path": "myAPP-21.07.28.dmg/myAPP.app/Contents/MacOS/myAPP",
"message": "The signature of the binary is invalid.",
"docUrl": null,
"architecture": "x86_64"
},
{
"severity": "error",
"code": null,
"path":
"myAPP-21.07.28.dmg/myAPP.app/Contents/runtime/Contents/MacOS/libjli.dylib",
"message": "The signature of the binary is invalid.",
"docUrl": null,
"architecture": "x86_64"
}
]
}
Re: RFR: 8271396: Spelling errors
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg wrote: > This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed . . Thank you! The PR has been updated - PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8271396: Spelling errors
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg wrote: > This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed Hi, I've filed https://bugs.openjdk.java.net/browse/JDK-8271396 for this PR, you can change the title to "8271396: Spelling errors", openjdk bot will link this PR to the corresponding issue. Also you should resolve conflicts and pass jcheck before integrating it. - PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8271396: Spelling errors
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg wrote: > This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed Trivially, looks ok to me. - Marked as reviewed by chegar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2385
Re: RFR: 8271396: Spelling errors
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg wrote: > This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed Lgtm. - Marked as reviewed by tschatzl (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2385
RFR: 8271396: Spelling errors
This PR fixes the following spelling errors: choosen -> chosen commad -> command hiearchy -> hierarchy leagacy -> legacy minium -> minimum subsytem -> subsystem unamed -> unnamed - Commit messages: - 8271396: Fix spelling errors Changes: https://git.openjdk.java.net/jdk/pull/2385/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2385=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271396 Stats: 78 lines in 34 files changed: 0 ins; 0 del; 78 mod Patch: https://git.openjdk.java.net/jdk/pull/2385.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2385/head:pull/2385 PR: https://git.openjdk.java.net/jdk/pull/2385
jpackage MacOS Notarization
All,
I am trying to notarize an app (built with jpackage) for MacOS.
jpackage at first *seems* to properly sign all resources with the available
--mac-sign options et cetera.
Having said that, there are still remaining issues
1. The app cannot be properly installed
(without hacks like xattr -d com.apple.quarantine /Applications/myAPP.app
).
2. I am also not able to properly notarize the app.
According to online resources there seem to exist issues in *past* about
notarization but according to [1, 2] the issues are fixed.
As mentioned, I still have issues :-(
Am I really the only one still having problems?
Java Version: AdoptOpenJDK-16.0.1+9 (tried Oracle JDK also without success)
The issue seems to boil down to 2 errors (attached the error log from Apple
notarization process).
* app Folder
* libjli.dylib
I thought I better ask first on the mailing list before creating an actual
bug report.
Note1: I used to use the *old* javapackager that worked with the same
signature/credentials.
Note2: running jpackage without --mac-sign options causes many more errors
in notarization (Hence, jpackage signs most resources but fails with some)
Any help / hint appreciated.
Thanks,
-- Daniel
[1] https://bugs.openjdk.java.net/browse/JDK-8257488
[2] https://bugs.openjdk.java.net/browse/JDK-8251892
## APPLE LOG ##
{
"logFormatVersion": 1,
"jobId": "...",
"status": "Invalid",
"statusSummary": "Archive contains critical validation errors",
"statusCode": 4000,
"archiveFilename": "myAPP-21.07.28.dmg",
"uploadDate": "2021-07-28T14:31:24Z",
"sha256": "...",
"ticketContents": null,
"issues": [
{
"severity": "error",
"code": null,
"path": "myAPP-21.07.28.dmg/myAPP.app/Contents/MacOS/myAPP",
"message": "The signature of the binary is invalid.",
"docUrl": null,
"architecture": "x86_64"
},
{
"severity": "error",
"code": null,
"path":
"myAPP-21.07.28.dmg/myAPP.app/Contents/runtime/Contents/MacOS/libjli.dylib",
"message": "The signature of the binary is invalid.",
"docUrl": null,
"architecture": "x86_64"
}
]
}
Re: RFR: 8266054: VectorAPI rotate operation optimization [v13]
On Tue, 27 Jul 2021 18:31:20 GMT, Sandhya Viswanathan wrote: >> Jatin Bhateja has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 19 commits: >> >> - 8266054: Re-designing benchmark to remove noise. >> - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8266054 >> - 8266054: Formal argument name change to be more appropriate. >> - 8266054: Review comments resolution. >> - 8266054: Incorporating styling changes based on reviews. >> - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8266054 >> - Merge http://github.com/openjdk/jdk into JDK-8266054 >> - Merge http://github.com/openjdk/jdk into JDK-8266054 >> - Merge http://github.com/openjdk/jdk into JDK-8266054 >> - Merge branch 'JDK-8266054' of http://github.com/jatin-bhateja/jdk into >> JDK-8266054 >> - ... and 9 more: >> https://git.openjdk.java.net/jdk/compare/a8f15427...b20404e2 > > Looks good to me. > @sviswa7 and @jatin-bhateja jatin-bhateja > The push caused https://bugs.openjdk.java.net/browse/JDK-8271366 > I am strongly suggest in a future to ask an Oracle's engineer to test Intel's > changes before pushing. Yes, as discussed before please request that we perform internal tests before integrating e.g. CC me. Unfortunately the pre-commit PR tests don't cover all the tests cases and we don't yet have a way to expand that set. - PR: https://git.openjdk.java.net/jdk/pull/3720
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v4]
On Wed, 28 Jul 2021 09:29:25 GMT, Wu Yan wrote: > > We are testing on HiSilicon TSV110, maybe we can enable this optimization by > default on the verified platforms. We don't really want to have different implementations for each microarchitecture, that would be a testing nightmare. The existing stub uses prefetch instructions if `SoftwarePrefetchHintDistance >= 0` but the new LDP version doesn't. Did you find there's no benefit to that? Adding prefetches was one of the reasons to introduce the separate stub for long strings, see the mail below: https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2018-April/02.html It seems the existing code was tuned for Thunder X/X2 so perhaps that's why Andrew sees little improvement there with the new version. What testing have you done besides benchmarking? The patch linked above had at least two subtle bugs in corner cases. - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v4]
On Wed, 28 Jul 2021 08:51:38 GMT, Andrew Haley wrote: > The trouble is, what does "worse" mean? I'm looking at SDEN-1982442, Neoverse > N2 errata, 2001293, and I see that LDP has to be slowed down on streaming > workloads, which will affect this. (That's just an example: I'm making the > point that implementations differ.) We are testing on HiSilicon TSV110, maybe we can enable this optimization by default on the verified platforms. - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v4]
On Wed, 28 Jul 2021 08:25:08 GMT, Nick Gasson wrote: > I don't think we want to keep two copies of the compareTo intrinsic. If there > are no cases where the LDP version is worse than the original version then we > should just delete the old one and replace it with this. I agree. The trouble is, what does "worse" mean? I'm looking at SDEN-1982442, Neoverse N2 errata, 2001293, and I see that LDP has to be slowed down on streaming workloads, which will affect this. The trouble with this patch is that it (probably) makes things better for long strings, which are very rare. What we actually need to care about is performance for a large number of typical-sized strings, which are names, identifiers, passwords, and so on: about 10-30 characters. - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v4]
On Thu, 15 Jul 2021 03:30:46 GMT, Wang Huang wrote: >> Dear all, >> Can you do me a favor to review this patch. This patch use `ldp` to >> implement String.compareTo. >> >> * We add a JMH test case >> * Here is the result of this test case >> >> Benchmark |(size)| Mode| Cnt|Score | Error |Units >> -|--|-||--||- >> StringCompare.compareLL | 64 | avgt| 5 |7.992 | ± 0.005|us/op >> StringCompare.compareLL | 72 | avgt| 5 |15.029| ± 0.006|us/op >> StringCompare.compareLL | 80 | avgt| 5 |14.655| ± 0.011|us/op >> StringCompare.compareLL | 91 | avgt| 5 |16.363| ± 0.12 |us/op >> StringCompare.compareLL | 101 | avgt| 5 |16.966| ± 0.007|us/op >> StringCompare.compareLL | 121 | avgt| 5 |19.276| ± 0.006|us/op >> StringCompare.compareLL | 181 | avgt| 5 |19.002| ± 0.417|us/op >> StringCompare.compareLL | 256 | avgt| 5 |24.707| ± 0.041|us/op >> StringCompare.compareLLWithLdp| 64 | avgt| 5 |8.001 | ± >> 0.121|us/op >> StringCompare.compareLLWithLdp| 72 | avgt| 5 |11.573| ± 0.003|us/op >> StringCompare.compareLLWithLdp| 80 | avgt| 5 |6.861 | ± 0.004|us/op >> StringCompare.compareLLWithLdp| 91 | avgt| 5 |12.774| ± 0.201|us/op >> StringCompare.compareLLWithLdp| 101 | avgt| 5 |8.691 | ± 0.004|us/op >> StringCompare.compareLLWithLdp| 121 | avgt| 5 |11.091| ± 1.342|us/op >> StringCompare.compareLLWithLdp| 181 | avgt| 5 |14.64 | ± 0.581|us/op >> StringCompare.compareLLWithLdp| 256 | avgt| 5 |25.879| ± 1.775|us/op >> StringCompare.compareUU | 64 | avgt| 5 |13.476| ± 0.01 |us/op >> StringCompare.compareUU | 72 | avgt| 5 |15.078| ± 0.006|us/op >> StringCompare.compareUU | 80 | avgt| 5 |23.512| ± 0.011|us/op >> StringCompare.compareUU | 91 | avgt| 5 |24.284| ± 0.008|us/op >> StringCompare.compareUU | 101 | avgt| 5 |20.707| ± 0.017|us/op >> StringCompare.compareUU | 121 | avgt| 5 |29.302| ± 0.011|us/op >> StringCompare.compareUU | 181 | avgt| 5 |39.31 | ± >> 0.016|us/op >> StringCompare.compareUU | 256 | avgt| 5 |54.592| ± 0.392|us/op >> StringCompare.compareUUWithLdp| 64 | avgt| 5 |16.389| ± 0.008|us/op >> StringCompare.compareUUWithLdp| 72 | avgt| 5 |10.71 | ± 0.158|us/op >> StringCompare.compareUUWithLdp| 80 | avgt| 5 |11.488| ± 0.024|us/op >> StringCompare.compareUUWithLdp| 91 | avgt| 5 |13.412| ± 0.006|us/op >> StringCompare.compareUUWithLdp| 101 | avgt| 5 |16.245| ± 0.434|us/op >> StringCompare.compareUUWithLdp| 121 | avgt| 5 |16.597| ± 0.016|us/op >> StringCompare.compareUUWithLdp| 181 | avgt| 5 |27.373| ± 0.017|us/op >> StringCompare.compareUUWithLdp| 256 | avgt| 5 |41.74 | ± 3.5 |us/op >> >> From this table, we can see that in most cases, our patch is better than old >> one. >> >> Thank you for your review. Any suggestions are welcome. > > Wang Huang has updated the pull request incrementally with one additional > commit since the last revision: > > fix style and add unalign test case I don't think we want to keep two copies of the compareTo intrinsic. If there are no cases where the LDP version is worse than the original version then we should just delete the old one and replace it with this. - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v2]
On Mon, 12 Jul 2021 15:36:29 GMT, Andrew Haley wrote: >> Wang Huang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> draft of refactor > > And with longer strings, M1 and ThunderX2: > > > Benchmark (diff_pos) (size) Mode Cnt > Score Error Units > StringCompare.compareLLDiffStrings10231024 avgt3 > 50.849 ± 0.087 us/op > StringCompare.compareLLDiffStringsWithLdp 10231024 avgt3 > 23.676 ± 0.015 us/op > StringCompare.compareLLDiffStringsWithRefactor10231024 avgt3 > 28.967 ± 0.168 us/op > > > StringCompare.compareLLDiffStrings10231024 avgt3 > 98.681 ± 0.026 us/op > StringCompare.compareLLDiffStringsWithLdp 10231024 avgt3 > 82.576 ± 0.656 us/op > StringCompare.compareLLDiffStringsWithRefactor10231024 avgt3 > 98.801 ± 0.321 us/op > > LDP wins on M1 here, but on ThunderX2 it makes almost no difference at all. > And how often are we comparing such long strings? > I don't know what to think, really. It seems that we're near to a place where > we're optimizing for micro-architecture, and I don't want to see that here. > On the other hand, using LDP is not worse anywhere, so we should allow it. Could you do me a favor to review the patch? @theRealAph @nick-arm Thanks. - PR: https://git.openjdk.java.net/jdk/pull/4722
Integrated: 8271368: [BACKOUT] JDK-8266054 VectorAPI rotate operation optimization
On Wed, 28 Jul 2021 05:35:59 GMT, Vladimir Kozlov wrote: > Backout the following changes due to vector tests failures in tier 2 and > later: > [JDK-8266054](https://bugs.openjdk.java.net/browse/JDK-8266054) VectorAPI > rotate operation optimization > > Changes also caused copyright header validation failure in Tier1 due to > missing `,` after copyright year in new test. > > Currently running testing. This pull request has now been integrated. Changeset: d7b5cb68 Author:Vladimir Kozlov URL: https://git.openjdk.java.net/jdk/commit/d7b5cb688956ce79443ef3cd080c36028fcfb19d Stats: 4438 lines in 57 files changed: 58 ins; 4219 del; 161 mod 8271368: [BACKOUT] JDK-8266054 VectorAPI rotate operation optimization Reviewed-by: dholmes, iklam - PR: https://git.openjdk.java.net/jdk/pull/4915
Re: RFR: 8271368: [BACKOUT] JDK-8266054 VectorAPI rotate operation optimization
On Wed, 28 Jul 2021 06:24:20 GMT, Jatin Bhateja wrote: > * Thanks for reporting it, should it be ok to move those tests to > ProblemList.txt and let me fix this as a follow up issue instead of a revert ? @jatin-bhateja We did not test original changes in our testing infra. There could be other issues in high tiers which we don't know. I prefer that you use 8271366 to prepare changes with fixed reported failure, file PR and let me run testing. - PR: https://git.openjdk.java.net/jdk/pull/4915
Re: RFR: 8271368: [BACKOUT] JDK-8266054 VectorAPI rotate operation optimization
On Wed, 28 Jul 2021 05:35:59 GMT, Vladimir Kozlov wrote: > Backout the following changes due to vector tests failures in tier 2 and > later: > [JDK-8266054](https://bugs.openjdk.java.net/browse/JDK-8266054) VectorAPI > rotate operation optimization > > Changes also caused copyright header validation failure in Tier1 due to > missing `,` after copyright year in new test. > > Currently running testing. - Thanks for reporting it, should it be ok to move those tests to ProblemList.txt and let me fix this as a follow up issue instead of a revert ? - PR: https://git.openjdk.java.net/jdk/pull/4915
Re: RFR: 8271368: [BACKOUT] JDK-8266054 VectorAPI rotate operation optimization
On Wed, 28 Jul 2021 05:35:59 GMT, Vladimir Kozlov wrote: > Backout the following changes due to vector tests failures in tier 2 and > later: > [JDK-8266054](https://bugs.openjdk.java.net/browse/JDK-8266054) VectorAPI > rotate operation optimization > > Changes also caused copyright header validation failure in Tier1 due to > missing `,` after copyright year in new test. > > Currently running testing. LGTM - Marked as reviewed by iklam (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4915
