Re: RFR: 8284932: [Vector API] Incorrect implementation of LSHR operator for negative byte/short elements
On Tue, 19 Apr 2022 02:43:33 GMT, Quan Anh Mai wrote: > I see, however, I preserve the opinion that the API doc implies the extended > unsigned right shift not the original `>>>` (or the output types would be > wrong). So, I think you can create another operator that perform the scalar > `>>>` if it is needed. > > Thank you very much. Thanks @merykitty for your understanding. After the discussion, I got the point that the original implementation of `LSHR` for bytes/shorts is useful and needed. So let's just keep it. Yes, we think the operator for scalar `>>>` is needed for several reasons: 1. We do have scalar `>>>` upon bytes/shorts in real programs. 2. There is usually no guarantee that all the operands would be non-negative for `>>>`. 3. Make it to be programmed more easily and also reduce the possibility to make mistakes. Java developers would be happy and appreciated with that operator I believe. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8276
Re: RFR: 8283870: jdeprscan --help causes an exception when the locale is ja, zh_CN or de [v2]
On Tue, 12 Apr 2022 06:36:29 GMT, Koichi Sakata wrote: >> # Summary >> Running jdeprscan with --help option causes an exception on any OSs when the >> locale is ja, zh_CN or de. >> >> # How to reproduce this issue >> >> $ jdeprscan -J-Duser.language=ja --help >> Exception in thread "main" java.lang.IllegalArgumentException: can't parse >> argument number: dir|jar|class >> at >> java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1454) >> at >> java.base/java.text.MessageFormat.applyPattern(MessageFormat.java:492) >> at java.base/java.text.MessageFormat.(MessageFormat.java:371) >> at java.base/java.text.MessageFormat.format(MessageFormat.java:860) >> at jdk.jdeps/com.sun.tools.jdeprscan.Messages.get(Messages.java:62) >> at jdk.jdeps/com.sun.tools.jdeprscan.Main.printHelp(Main.java:706) >> at jdk.jdeps/com.sun.tools.jdeprscan.Main.run(Main.java:529) >> at jdk.jdeps/com.sun.tools.jdeprscan.Main.call(Main.java:717) >> at jdk.jdeps/com.sun.tools.jdeprscan.Main.main(Main.java:725) >> Caused by: java.lang.NumberFormatException: For input string: "dir|jar|class" >> at >> java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67) >> at java.base/java.lang.Integer.parseInt(Integer.java:668) >> at java.base/java.lang.Integer.parseInt(Integer.java:786) >> at >> java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1452) >> ... 8 more >> >> $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help >> (Same as above) >> >> $ jdeprscan -J-Duser.language=de --help >> (Same as above) >> >> Of course, it works well when the locale is anything other than those >> locales. >> >> # Details >> I found **''**{dir|jar|class}**''** in both ja and zh_CN properties files. >> Doubled single quotes mean a single quote in MessageFormat class, so the >> class recognizes {dir|jar|class} as a FormatElement and throws the >> exception. >> >> I removed extra single quotes. >> >> # Test >> It seems there is no test class for it. So I run commands as I mentioned >> before. >> >> $ jdeprscan -J-Duser.language=ja --help >> 使用方法: jdeprscan [options] {dir|jar|class} ... >> >> オプション: >> --class-path PATH >> --for-removal >> --full-version >> -? -h --help >> -l--list >> --release 7|8|9|10|11|12|13|14|15|16|17|18|19 >> -v--verbose >> --version >> >> 非推奨APIの使用について各引数をスキャンします。引数には、 >> パッケージ階層のルートを指定するディレクトリ、JARファイル、 >> クラス・ファイルまたはクラス名を使用できます。クラス名は、 >> 完全修飾クラス名を使用して指定する必要があります。ネストされた >> クラスは$で区切ります。例: >> >> java.lang.Thread$State >> >> --class-pathオプションは、依存するクラスの解決のための >> 検索パスを指定します。 >> >> --for-removalオプションは、スキャンとリスト化を削除予定で非推奨のAPIに >> 限定します。リリース値が6、7または8の場合は使用できません。 >> >> --full-versionオプションはツールのバージョン文字列の全体を出力します。 >> >> --help (-? -h)オプションは、ヘルプ・メッセージ全体を出力します。 >> >> --list (-l)オプションは非推奨APIセットを出力します。スキャンは行われないため、 >> ディレクトリ、JARまたはクラス引数を指定する必要はありません。 >> >> --releaseオプションは、スキャンする非推奨APIのセットを提供するJava SE >> リリースを指定します。 >> >> --verbose (-v)オプションを使用すると、処理中に追加のメッセージを出力できます。 >> >> --versionオプションは、簡略化されたツールのバージョン文字列を出力します。 >> >> $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help >> 用法:jdeprscan [options] {dir|jar|class} ... >> >> 选项: >> --class-path PATH >> --for-removal >> --full-version >> -? -h --help >> -l--list >> --release 7|8|9|10|11|12|13|14|15|16|17|18|19 >> -v--verbose >> --version >> >> 扫描每个参数以了解是否使用了过时的 API。 >> 参数可以是指定程序包分层结构、JAR 文件、 >> 类文件或类名的根的目录。类名必须 >> 使用全限定类名指定,并使用 $ 分隔符 >> 指定嵌套类,例如, >> >> java.lang.Thread$State >> >> --class-path 选项提供了用于解析从属类的 >> 搜索路径。 >> >> --for-removal 选项限制扫描或列出已过时并待删除 >> 的 API。不能与发行版值 6、7 或 8 一起使用。 >> >> --full-version 选项输出工具的完整版本字符串。 >> >> --help (-? -h) 选项输出完整的帮助消息。 >> >> --list (-l) 选项输出一组已过时的 API。不执行扫描, >> 因此不应提供任何目录、jar 或类参数。 >> >> --release 选项指定提供要扫描的已过时 API 集 >> 的 Java SE 发行版。 >> >> --verbose (-v) 选项在处理期间启用附加消息输出。 >> >> --version 选项输出工具的缩写版本字符串。 >> >> $ jdeprscan -J-Duser.language=de --help >> Verwendung: jdeprscan [Optionen] {dir|jar|class} ... >> >> Optionen: >> --class-path PATH >> --for-removal >> --full-version >> -? -h --help >> -l--list >> --release 7|8|9|10|11|12|13|14|15|16|17|18|19 >> -v--verbose >> --version >> >> Scannt jedes Argument auf die Verwendung veralteter APIs. Ein Argument >> kann ein Verzeichnis, das die Root einer Packagehierarchie angibt, >> eine JAR-Datei, eine Klassendatei oder ein Klassenname sein. Der Klassenname >> muss durch einen vollständig qualifizierten Klassennamen mit $ als >> Trennzeichen >> für verschachtelte Klassen angegeben werden. Beispiel: >> >> java.lang.Thread$State >> >> Die Option --class-path liefert einen Suchpfad für die Auflösung >> von abhängigen Klassen. >> >> Die Option --for-removal begrenzt das Scannen oder Auflisten auf APIs, die >> veraltet sind >> und entfernt
Re: RFR: 8284856: Add a test case for checking UnicodeScript entity numbers [v3]
> Added the test case, and eliminated the immediate hashmap value, replaced > with the ordinal number of `Character.UnicodeScript.UNKNOWN`. Naoto Sato has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 29 commits: - Merge branch 'master' into JDK-8284856 - Fixed a typo - 8284856: Add a test case for checking UnicodeScript entity numbers - add `@LastModified: Apr 2022` to DocumentCache - revert changes on ProcessEnvironment - fix usage in XSAttributeChecker - update LastModified - Copyright latest year to 2022 - revert changes in: src/java.desktop src/java.management src/jdk.internal.vm.ci src/jdk.jfr src/jdk.management.jfr src/jdk.management src/utils/IdealGraphVisualizer - Add apiNote to appropriate constructors of HM, LHM, and WHM. - ... and 19 more: https://git.openjdk.java.net/jdk/compare/145dfed0...2a5f3f72 - Changes: https://git.openjdk.java.net/jdk/pull/8253/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8253=02 Stats: 13 lines in 2 files changed: 8 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/8253.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8253/head:pull/8253 PR: https://git.openjdk.java.net/jdk/pull/8253
Re: RFR: 8284950: Swappiness disables swap space usage
On Mon, 18 Apr 2022 16:25:17 GMT, Ioi Lam wrote: >> src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1Subsystem.java >> line 155: >> >>> 153: long memswBytes = getLongValue(controller, >>> "memory.memsw.limit_in_bytes"); >>> 154: long swappiness = getLongValue(controller, >>> "memory.swappiness"); >>> 155: return (memswBytes > 0 && swappiness > 0); >> >> Does this also need to be changed in the test? >> >> https://github.com/openjdk/jdk/blob/c63fabe3d582ce0828b04b0224cea49aab5fedf3/test/lib/jdk/test/lib/containers/cgroup/MetricsTesterCgroupV1.java#L291-L296 > > There's also corresponding code in HotSpot: > > https://github.com/openjdk/jdk/blob/c63fabe3d582ce0828b04b0224cea49aab5fedf3/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp#L129-L150 > Does this also need to be changed in the test? > > https://github.com/openjdk/jdk/blob/c63fabe3d582ce0828b04b0224cea49aab5fedf3/test/lib/jdk/test/lib/containers/cgroup/MetricsTesterCgroupV1.java#L291-L296 This condition returns false,The following code is skipped https://github.com/openjdk/jdk/blob/c63fabe3d582ce0828b04b0224cea49aab5fedf3/test/lib/jdk/test/lib/containers/cgroup/MetricsTesterCgroupV1.java#L284 - PR: https://git.openjdk.java.net/jdk/pull/8285
Re: RFR: 8284932: [Vector API] Incorrect implementation of LSHR operator for negative byte/short elements
On Sun, 17 Apr 2022 14:35:14 GMT, Jie Fu wrote: > Hi all, > > According to the Vector API doc, the `LSHR` operator computes > `a>>>(n&(ESIZE*8-1))`. > However, current implementation is incorrect for negative bytes/shorts. > > The background is that one of our customers try to vectorize `urshift` with > `urshiftVector` like the following. > > 13 public static void urshift(byte[] src, byte[] dst) { > 14 for (int i = 0; i < src.length; i++) { > 15 dst[i] = (byte)(src[i] >>> 3); > 16 } > 17 } > 18 > 19 public static void urshiftVector(byte[] src, byte[] dst) { > 20 int i = 0; > 21 for (; i < spec.loopBound(src.length); i +=spec.length()) { > 22 var va = ByteVector.fromArray(spec, src, i); > 23 var vb = va.lanewise(VectorOperators.LSHR, 3); > 24 vb.intoArray(dst, i); > 25 } > 26 > 27 for (; i < src.length; i++) { > 28 dst[i] = (byte)(src[i] >>> 3); > 29 } > 30 } > > > Unfortunately and to our surprise, code@line28 computes different results > with code@line23. > It took quite a long time to figure out this bug. > > The root cause is that current implemenation of Vector API can't compute the > unsigned right shift results as what is done for scalar `>>>` for negative > byte/short elements. > Actually, current implementation will do `(a & 0xFF) >>> (n & 7)` [1] for all > bytes, which is unable to compute the vectorized `>>>` for negative bytes. > So this seems unreasonable and unfriendly to Java developers. > It would be better to fix it. > > The key idea to support unsigned right shift of negative bytes/shorts is just > to replace the unsigned right shift operation with the signed right shift > operation. > This logic is: > - For byte elements, unsigned right shift is equal to signed right shift if > the shift_cnt <= 24. > - For short elements, unsigned right shift is equal to signed right shift if > the shift_cnt <= 16. > - For Vector API, the shift_cnt will be masked to shift_cnt <= 7 for bytes > and shift_cnt <= 15 for shorts. > > I just learned this idea from https://github.com/openjdk/jdk/pull/7979 . > And many thanks to @fg1417 . > > > Thanks. > Best regards, > Jie > > [1] > https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java#L935 I see, however, I preserve the opinion that the API doc implies the extended unsigned right shift not the original `>>>` (or the output types would be wrong). So, I think you can create another operator that perform the scalar `>>>` if it is needed. Thank you very much. - PR: https://git.openjdk.java.net/jdk/pull/8276
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v4]
On Sun, 17 Apr 2022 16:03:30 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh src/java.base/share/classes/java/lang/System.java line 2173: > 2171: > 2172: // start Finalizer and Reference Handler threads > 2173: SharedSecrets.getJavaLangRefAccess().startThreads(); I think it would avoid any confusion if `VM.initLevel(1)` is the last step in `initPhase1`, i.e. call after this line. Previously, the finalizer starts very early and it has to wait until initLevel is set to level 1 which guarantees that `JavaLangAccess` is available. With this change, `JavaLangAccess` is already set. src/java.base/share/classes/java/lang/ref/ReferenceQueue.java line 61: > 59: private final Condition notEmpty; > 60: > 61: void signal() { notEmpty.signalAll(); } `signal()`, `await()` and `await(long)` methods are only used by `ReferenceQueue`. Good to make them private. src/jdk.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java line 138: > 136: * > 137: * @param outputFile the path to the file to create > 138: * @param format the format to use (TEXT_PLAIN or JSON) It's useful to link to `ThreadDumpFormat#TEXT_PLAN` and `ThreadDumpFormat#JSON` - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284642: Unexpected behavior of -XX:MaxDirectMemorySize=0
On Wed, 13 Apr 2022 12:24:46 GMT, Harold Seigel wrote: > Please review this small fix for JDK-8284642. The fix was tested by running > Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-5 on Linux > x64. Additionally, the modified test and the test in the bug report were run > locally. > > Thanks, Harold See main comment - Changes requested by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8222
Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes
On Mon, 18 Apr 2022 20:42:48 GMT, Remi Forax wrote: > The third parameter of defineProxy() is a lambda which is called for each > method that can be overriden, either toString/equals/hashCode but also any > default methods, if the lambda return true, the method is overriden, otherwise the default implementation is used. Not quite, I mean calling default implementations in the super interface, consider: interface Root { void cleanUp(); } interface FeatureOne extends Root { default void cleanUp() { /* do something */ } } interface FeatureTwo extends Root { default void cleanUp() { /* do something */ } } My proxy implements both feature one and two, but in your API, there is no way for my `cleanUp` to call both `FeatureOne.super.cleanUp();` and `FeatureTwo.super.cleanUp();`. You should probably expose the lookup in your linker too, especially that you enabled nest access and you can just use that lookup to resolve, say, an implementation static method in the proxy user class. Also the "delegate" in your API would significantly benefit from https://bugs.openjdk.java.net/browse/JDK-8282798 (#7744), too. - PR: https://git.openjdk.java.net/jdk/pull/8278
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v4]
On Sun, 17 Apr 2022 16:03:30 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh src/java.base/share/classes/java/lang/ref/ReferenceQueue.java line 206: > 204: throws IllegalArgumentException, InterruptedException { > 205: if (timeout < 0) throw new IllegalArgumentException("Negative > timeout value"); > 206: if (timeout == 0) return remove(); Nit: use the same formatting as in `NativeReferenceQueue::remove` and in this file. if (timeout < 0) throw new IllegalArgumentException("Negative timeout value"); if (timeout == 0) return remove(); - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284856: Add a test case for checking UnicodeScript entity numbers [v2]
On Thu, 14 Apr 2022 22:27:20 GMT, Naoto Sato wrote: >> Added the test case, and eliminated the immediate hashmap value, replaced >> with the ordinal number of `Character.UnicodeScript.UNKNOWN`. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed a typo The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork: git checkout fix_8186958 git fetch https://git.openjdk.java.net/jdk master git merge FETCH_HEAD # if there are conflicts, follow the instructions given by git merge git commit -m "Merge master" git push - PR: https://git.openjdk.java.net/jdk/pull/8253
Integrated: 8186958: Need method to create pre-sized HashMap
On Wed, 23 Mar 2022 18:41:59 GMT, XenoAmess wrote: > 8186958: Need method to create pre-sized HashMap This pull request has now been integrated. Changeset: 87faa85c Author:XenoAmess Committer: Stuart Marks URL: https://git.openjdk.java.net/jdk/commit/87faa85c59e94d66c3c61d997eacdd2dbe5a1772 Stats: 212 lines in 30 files changed: 139 ins; 4 del; 69 mod 8186958: Need method to create pre-sized HashMap Reviewed-by: chegar, naoto, joehw, lancea, wetmore, smarks - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v22]
On Thu, 14 Apr 2022 21:27:16 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > java.xml.crypto's usage downgrade grammar to 1.8 I've also written a release note for this change. Please review. https://bugs.openjdk.java.net/browse/JDK-8284975 - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: [External] : Re: jpackage usage problems
On Mon, 2022-04-18 at 18:41 -0400, Alexey Semenyuk wrote: > > I've filed [1] and [2] CRs to track the issues. > > [1] https://bugs.openjdk.java.net/browse/JDK-8284973 > [2] https://bugs.openjdk.java.net/browse/JDK-8284974 > > - Alexey Sounds great. Thank you. While we are at improving JPackage, there are a few more items I stumbled over: a) When running jpackage on Linux, a freedesktop.org style starter file is created - but one more line in there would make it a lot more usable: https://stackoverflow.com/questions/70420618/jpackage-linux-creates-insufficient-desktop-file b) The solution is to override resource files. Meanwhile I found out the resource files are templates, and jpackage replaces specific strings in these files. This however is nowhere mentioned in the documentation. c) Running jpackage in two phases as I do (1: create app-image, 2: create final package) I learned that --name has to be specified in both runs. When running jpackage on MacOS however the second run needs the application name postfixed with .app otherwise jpackage won't find the directory it created in the first run. See the logs https://github.com/HiranChaudhuri/settlers-installer/runs/6055932278?check_suite_focus=true and https://github.com/HiranChaudhuri/settlers-installer/runs/6055948346?check_suite_focus=true Hiran
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v4]
On Sun, 17 Apr 2022 16:03:30 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh src/java.management/share/classes/java/lang/management/ThreadMXBean.java line 655: > 653: * synchronization control. It might be an expensive operation. > 654: * Its behavior with cycles involving virtual threads is not defined > 655: * in this release. What does the current implementation return if the virtual threads are involved in a deadlock?The class spec says "ThreadMXBean does not support monitoring or management of virtual threads" while this javadoc says it's undefined.I wonder if it's helpful to include `@implNote` if appropriate. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: [External] : Re: jpackage usage problems
On 4/17/2022 2:22 PM, Hiran Chaudhuri wrote: On Thu, 2022-04-07 at 19:53 -0400, Alexey Semenyuk wrote: I can see two separate issues with jpackage: 1. jpackage reports NPE if it can't figure out the package name from the supplied application image. It should issue a helpful error message instead 2. jpackage fails to populate application image with Java runtime and app specific files when executed in Github but exits with "0" status indicating no error I totally agree these are two separate issues. The major problem in your use case is that jpackage doesn't create correct application image. Instead, it creates only a directory structure of application image, but no files and exits with a "success" status code. This is quite strange. Is there any chance Gradle ignores non-zero exit code from jpackage execution? Can you directly run jpackage command creating application image in the environment where it outputs "empty" application image and check its exit code (please add "--verbose" to jpackage command line to get debug output)? - Alexey Now this is somewhat puzzling for me: I tried to factor out Gradle by putting the JPackage commands into a separate section of the Github Actions script. So now it is invoked through bash. No wonder, the command triggered by Gradle and the one triggered by bash revealed the same result: A lot of files were missing. I verified this by listing the directories through a 'find'. But then I followed your advice and added --verbose to the JPackage command. And find lists a lot more files now. In case you are interested, check out https://urldefense.com/v3/__https://github.com/HiranChaudhuri/settlers-installer/runs/5890636608?check_suite_focus=true__;!!ACWV5N9M2RV99hQ!fMwHRzbnvjiQPsoiuBB1Cnl4RNGxATH4RHnb3Ugy-dlsPTPHLoqOF2uSYcFRkfaSYHQA$ Thank you for sharing. I find this piece of log interesting: --- [19:28:24.553] Command [PID: -1]: jlink --output app/build/app-image/SettlersRemake/lib/runtime --module-path /usr/lib/jvm/temurin-17-jdk-amd64/jmods --add-modules jdk.management.jfr,java.rmi,jdk.jdi,jdk.charsets,java.xml,jdk.xml.dom,java.datatransfer,jdk.jstatd,jdk.httpserver,java.desktop,java.security.sasl,jdk.zipfs,java.base,jdk.crypto.ec,jdk.javadoc,jdk.management.agent,jdk.jshell,jdk.editpad,java.sql.rowset,jdk.jsobject,jdk.sctp,java.smartcardio,jdk.unsupported,jdk.jlink,java.security.jgss,java.compiler,jdk.nio.mapmode,jdk.dynalink,jdk.unsupported.desktop,jdk.accessibility,jdk.security.jgss,jdk.incubator.vector,java.sql,java.transaction.xa,java.logging,java.xml.crypto,jdk.jfr,jdk.crypto.cryptoki,jdk.net,jdk.random,java.naming,jdk.internal.ed,java.prefs,java.net.http,jdk.compiler,jdk.naming.rmi,jdk.internal.opt,jdk.jconsole,jdk.attach,jdk.internal.le,java.management,jdk.jdwp.agent,jdk.internal.jvmstat,jdk.incubator.foreign,java.instrument,jdk.management,jdk.security.auth,java.scripting,jdk.jdeps,jdk.jartool,jdk.jpackage,java.management.rmi,jdk.naming.dns,jdk.localedata --strip-native-commands --strip-debug --no-man-pages --no-header-files --- It is not quite right that pid of jlink invocation is reported as "-1". jlnk fills runtime directory in app image. Something if off here, invocation of jlink in non-verbose mode fails. In the verbose mode, jpackage saves command lines and output of all external tools it invokes and prints saved data to the console. At least there is a workaround for the issue - add "--verbose" to jpackage command line. I've filed [1] and [2] CRs to track the issues. [1] https://bugs.openjdk.java.net/browse/JDK-8284973 [2] https://bugs.openjdk.java.net/browse/JDK-8284974 - Alexey So now I believe that --verbose does not only change log output but also impacts app-image creation. BTW, the command used is jpackage --verbose --type app-image --dest app/build/app-image -i app/build/jpackage_input/app-0.1.0-SNAPSHOT/lib --main-jar app-0.1.0- SNAPSHOT.jar --main-class settlers.installer.App --name SettlersRemake --app-version 0.1.0-SNAPSHOT --description 'Settlers 3 remake - see https://urldefense.com/v3/__https://github.com/__;!!ACWV5N9M2RV99hQ!fMwHRzbnvjiQPsoiuBB1Cnl4RNGxATH4RHnb3Ugy-dlsPTPHLoqOF2uSYcFRkQh1q_el$ ' --vendor Hiran, --icon app/build/resources/main/siedler3-helme-logo.png --resource-dir app/build/resources/jpackage Hiran
Re: RFR: 8186958: Need method to create pre-sized HashMap [v22]
On Thu, 14 Apr 2022 21:27:16 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > java.xml.crypto's usage downgrade grammar to 1.8 Marked as reviewed by smarks (Reviewer). OK, go ahead and integrate! - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes
- Original Message - > From: "liach" > To: "core-libs-dev" , "security-dev" > > Sent: Monday, April 18, 2022 10:01:39 PM > Subject: Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes > On Sun, 17 Apr 2022 16:17:30 GMT, liach wrote: > >> Convert dynamic proxies to hidden classes. Modifies the serialization of >> proxies >> (requires change in "Java Object Serialization Specification"). Makes the >> proxies hidden in stack traces. Removes duplicate logic in proxy building. >> >> The main compatibility changes and their rationales are: >> 1. Modification to the serialization specification: In the "An instance of >> the >> class is allocated... The contents restored appropriately" section, I propose >> explicitly state that handling of proxies are unspecified as to allow >> implementation freedom, though I've seen deliberate attempts for proxies to >> implement interfaces with `readResolve` in order to control their >> serialization >> behavior. >>- This is for the existing generated constructor accessor is >> bytecode-based, >>which cannot refer to hidden classes. >>- An alternative is to preserve the behavior, where the serialization >>constructor calls `invokespecial` on the closest serializable superclass' >>no-arg constructor, like in #1830 by @DasBrain. >>- My rationale against preservation is such super calls are unsafe and >> should be >>discouraged in the long term. Calling the existing constructor with a >> dummy >>argument, as in my implementation, would be more safe. >> 2. The disappearance of proxies in stack traces. >>- Same behavior exists in lambda expressions: For instance, in >> `((Runnable) () >>-> { throw new Error(); }).run();`, the `run` method, implemented by the >>lambda, will not appear in the stack trace, and isn't too problematic. >> >> A summary of the rest of the changes: >> 1. Merged the two passes of determining module and package of the proxy into >> one. This reduced a lot of code and allowed anchor class (for hidden class >> creation) selection be done together as well. >> 2. Exposed internal API for obtaining a full-privileged lookup to the rest of >> `java.base`. This API is intended for implementation of legacy (pre >> `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more >> complex tricks to obtain proper permissions as lookups. >> 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959): >> passes methods computed by proxy generator as class data to the hidden proxy >> class to reduce generated proxy class size and improve performance. >> >> In addition, since this change is somewhat large, should we keep the old >> proxy >> generator as well and have it toggled through a command-line flag (like the >> old >> v49 proxy generator or the old reflection implementation)? >> >> Please feel free to comment or review. This change definitely requires a CSR, >> but I have yet to determine what specifications should be changed. > > I strongly recommend a new API over revamping `Proxy` itself. > https://github.com/forax/hidden_proxy would be a good prototype that serves > most needs of the current Proxy API (except a few, like invoking default > superinterface method). The third parameter of defineProxy() is a lambda which is called for each method that can be overriden, either toString/equals/hashCode but also any default methods, if the lambda return true, the method is overriden, otherwise the default implementation is used. > With hidden classes, I don't see much value in leaving > the new API's produced instance in separate modules; what we have for hidden > classes should be fine for the most part. > > Imo the main obstacle is still the handling of serialization. The annotations > are serializable, but currently hidden classes do not work with serialization > at all and must use `writeReplace` and `readResolve`. And how to migrate > annotations off proxies without breaking serialization is still a question as > well. Maybe we can upgrade invocation handlers to allow them to declare custom > `readResolve` logic for the proxy to facilitate migration away. How the new > API > will treat serialization is still undetermined. yes, i suppose that like with lambdas, we need a special overload of defineProxy that automatically implements writeReplace() and use a specific class SerializableProxy (mirroring how SerializableLambda works). > > - > > PR: https://git.openjdk.java.net/jdk/pull/8278
Re: RFR: 8284638: store skip buffers in InputStream Object [v7]
On Fri, 15 Apr 2022 18:56:37 GMT, XenoAmess wrote: >> @jmehrens what about this then? >> I think it safe now(actually this mechanism is learned from Reader) > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > change to liach operation. > @jmehrens what about this then? I think it safe now(actually this mechanism > is learned from Reader) Reader uses a lock object and it appears that InputStream locks on this (per make/reset) I would assume now that you have some object member fields you need to hold some lock while accessing those. Even though single thread access would be the expected case. Not related but, it looks like the static buffer issue has come up a few times. Maybe time to add a test to: https://github.com/openjdk/jdk/blob/master/test/jdk/java/io/InputStream/Skip.java that would fail if the skip buffer is static? Not really the primary issue but, it seems questionable to me that we don't have to fill the skip buffer with zeros after the last read. That way any sensitive information that was skipped would also be forgotten. Matching with Reader seems more important for now. - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes
On Sun, 17 Apr 2022 16:17:30 GMT, liach wrote: > Convert dynamic proxies to hidden classes. Modifies the serialization of > proxies (requires change in "Java Object Serialization Specification"). Makes > the proxies hidden in stack traces. Removes duplicate logic in proxy building. > > The main compatibility changes and their rationales are: > 1. Modification to the serialization specification: In the "An instance of > the class is allocated... The contents restored appropriately" section, I > propose explicitly state that handling of proxies are unspecified as to allow > implementation freedom, though I've seen deliberate attempts for proxies to > implement interfaces with `readResolve` in order to control their > serialization behavior. >- This is for the existing generated constructor accessor is > bytecode-based, which cannot refer to hidden classes. >- An alternative is to preserve the behavior, where the serialization > constructor calls `invokespecial` on the closest serializable superclass' > no-arg constructor, like in #1830 by @DasBrain. >- My rationale against preservation is such super calls are unsafe and > should be discouraged in the long term. Calling the existing constructor with > a dummy argument, as in my implementation, would be more safe. > 2. The disappearance of proxies in stack traces. >- Same behavior exists in lambda expressions: For instance, in > `((Runnable) () -> { throw new Error(); }).run();`, the `run` method, > implemented by the lambda, will not appear in the stack trace, and isn't too > problematic. > > A summary of the rest of the changes: > 1. Merged the two passes of determining module and package of the proxy into > one. This reduced a lot of code and allowed anchor class (for hidden class > creation) selection be done together as well. > 2. Exposed internal API for obtaining a full-privileged lookup to the rest of > `java.base`. This API is intended for implementation of legacy (pre > `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more > complex tricks to obtain proper permissions as lookups. > 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959): > passes methods computed by proxy generator as class data to the hidden proxy > class to reduce generated proxy class size and improve performance. > > In addition, since this change is somewhat large, should we keep the old > proxy generator as well and have it toggled through a command-line flag (like > the old v49 proxy generator or the old reflection implementation)? > > Please feel free to comment or review. This change definitely requires a CSR, > but I have yet to determine what specifications should be changed. I strongly recommend a new API over revamping `Proxy` itself. https://github.com/forax/hidden_proxy would be a good prototype that serves most needs of the current Proxy API (except a few, like invoking default superinterface method). With hidden classes, I don't see much value in leaving the new API's produced instance in separate modules; what we have for hidden classes should be fine for the most part. Imo the main obstacle is still the handling of serialization. The annotations are serializable, but currently hidden classes do not work with serialization at all and must use `writeReplace` and `readResolve`. And how to migrate annotations off proxies without breaking serialization is still a question as well. Maybe we can upgrade invocation handlers to allow them to declare custom `readResolve` logic for the proxy to facilitate migration away. How the new API will treat serialization is still undetermined. - PR: https://git.openjdk.java.net/jdk/pull/8278
Integrated: JDK-8282008: Incorrect handling of quoted arguments in ProcessBuilder
On Fri, 4 Mar 2022 23:20:21 GMT, Roger Riggs wrote: > Quoting related changes in https://bugs.openjdk.java.net/browse/JDK-8250568 > modified the way that > process builder recognized argument strings, causing some arguments to be > doubly quoted and malformed. > > ProcessBuilder encodes command arguments in two ways, a looser legacy encoding > and stricter encoding that prevents quotes from being misinterpreted. > The system property jdk.lang.Process.allowAmbiguousCommands controls which is > in effect. > > When the property is "true" or not set, arguments are inserted into the > Windows command line > with minimal changes. Arguments containing space or tab are quoted to > prevent them being split. > Arguments that start and end with double-quote are left alone. > Some executables interpret a backslash before the final quote as an escape; > if the argument > contains first and last quotes, backslashes are ignored. > > When the allowAmbigousCommands property is `false`, care is taken to ensure > that > the final quote of an argument is the closing quote for the argument and is > not > interpreted as a literal quote by a preceding quote (or an odd number of > quotes). > > The PR includes a test matrix of the cases where an argument with spaces and > a final backslash > is passed with each combination of `allowAmbiguousCommands = true and false`, > launched executable, java, .cmd, and .vbs and when the argument is surrounded > with double-quotes. > > The priority for allowAmbiguousCommands = false is that no argument is split > or joined to another argument. > In some cases, backslashes are doubled to prevent a double-quote from being > interpreted incorrectly. > The trailing backslash in an argument occurs rarely exception when the > argument is a directory. > In that case, the addition of trailing backslashes is benign when the string > is used as a filesystem path. > > See also PR#7504, for background and a proposal. This pull request has now been integrated. Changeset: 897d6c0d Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/897d6c0dc7cdfb3ad92f864f9ad4b50e642197e4 Stats: 343 lines in 2 files changed: 332 ins; 6 del; 5 mod 8282008: Incorrect handling of quoted arguments in ProcessBuilder Reviewed-by: bchristi - PR: https://git.openjdk.java.net/jdk/pull/7709
Re: RFR: JDK-8282008: Incorrect handling of quoted arguments in ProcessBuilder [v3]
On Tue, 5 Apr 2022 16:39:33 GMT, Roger Riggs wrote: >> Quoting related changes in https://bugs.openjdk.java.net/browse/JDK-8250568 >> modified the way that >> process builder recognized argument strings, causing some arguments to be >> doubly quoted and malformed. >> >> ProcessBuilder encodes command arguments in two ways, a looser legacy >> encoding >> and stricter encoding that prevents quotes from being misinterpreted. >> The system property jdk.lang.Process.allowAmbiguousCommands controls which >> is in effect. >> >> When the property is "true" or not set, arguments are inserted into the >> Windows command line >> with minimal changes. Arguments containing space or tab are quoted to >> prevent them being split. >> Arguments that start and end with double-quote are left alone. >> Some executables interpret a backslash before the final quote as an escape; >> if the argument >> contains first and last quotes, backslashes are ignored. >> >> When the allowAmbigousCommands property is `false`, care is taken to ensure >> that >> the final quote of an argument is the closing quote for the argument and is >> not >> interpreted as a literal quote by a preceding quote (or an odd number of >> quotes). >> >> The PR includes a test matrix of the cases where an argument with spaces and >> a final backslash >> is passed with each combination of `allowAmbiguousCommands = true and false`, >> launched executable, java, .cmd, and .vbs and when the argument is >> surrounded with double-quotes. >> >> The priority for allowAmbiguousCommands = false is that no argument is split >> or joined to another argument. >> In some cases, backslashes are doubled to prevent a double-quote from being >> interpreted incorrectly. >> The trailing backslash in an argument occurs rarely exception when the >> argument is a directory. >> In that case, the addition of trailing backslashes is benign when the string >> is used as a filesystem path. >> >> See also PR#7504, for background and a proposal. > > Roger Riggs has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains five additional > commits since the last revision: > > - Merge branch 'master' into 8282008-quoted-escape > - Add count of skipped tests and improve comments > - Refactored ArgCheck test to be more readable and easier to maintain and > backport > - Cleanup comment and copyright > - JDK-8282008: Incorrect handling of quoted arguments in ProcessBuilder This change required a CSR, now approved. - PR: https://git.openjdk.java.net/jdk/pull/7709
Integrated: 8284922: Fix some doc-comment issues on methods with package access in JDK API
On Fri, 15 Apr 2022 19:34:33 GMT, Pavel Rappo wrote: > People rarely include JDK elements with package access in a javadoc run. That > is why bugs in those elements' doc comments tend to remain unnoticed. > > There are many more bugs in the doc comments of the JDK elements with the > package access than are addressed by this PR; I only included the simplest > ones. This pull request has now been integrated. Changeset: d3d71ea2 Author:Pavel Rappo URL: https://git.openjdk.java.net/jdk/commit/d3d71ea289b7525d3f5c5057d995776be9a0796a Stats: 6 lines in 3 files changed: 1 ins; 0 del; 5 mod 8284922: Fix some doc-comment issues on methods with package access in JDK API Reviewed-by: darcy, iris, bpb - PR: https://git.openjdk.java.net/jdk/pull/8267
Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes
On Sun, 17 Apr 2022 16:17:30 GMT, liach wrote: > Convert dynamic proxies to hidden classes. Modifies the serialization of > proxies (requires change in "Java Object Serialization Specification"). Makes > the proxies hidden in stack traces. Removes duplicate logic in proxy building. > > The main compatibility changes and their rationales are: > 1. Modification to the serialization specification: In the "An instance of > the class is allocated... The contents restored appropriately" section, I > propose explicitly state that handling of proxies are unspecified as to allow > implementation freedom, though I've seen deliberate attempts for proxies to > implement interfaces with `readResolve` in order to control their > serialization behavior. >- This is for the existing generated constructor accessor is > bytecode-based, which cannot refer to hidden classes. >- An alternative is to preserve the behavior, where the serialization > constructor calls `invokespecial` on the closest serializable superclass' > no-arg constructor, like in #1830 by @DasBrain. >- My rationale against preservation is such super calls are unsafe and > should be discouraged in the long term. Calling the existing constructor with > a dummy argument, as in my implementation, would be more safe. > 2. The disappearance of proxies in stack traces. >- Same behavior exists in lambda expressions: For instance, in > `((Runnable) () -> { throw new Error(); }).run();`, the `run` method, > implemented by the lambda, will not appear in the stack trace, and isn't too > problematic. > > A summary of the rest of the changes: > 1. Merged the two passes of determining module and package of the proxy into > one. This reduced a lot of code and allowed anchor class (for hidden class > creation) selection be done together as well. > 2. Exposed internal API for obtaining a full-privileged lookup to the rest of > `java.base`. This API is intended for implementation of legacy (pre > `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more > complex tricks to obtain proper permissions as lookups. > 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959): > passes methods computed by proxy generator as class data to the hidden proxy > class to reduce generated proxy class size and improve performance. > > In addition, since this change is somewhat large, should we keep the old > proxy generator as well and have it toggled through a command-line flag (like > the old v49 proxy generator or the old reflection implementation)? > > Please feel free to comment or review. This change definitely requires a CSR, > but I have yet to determine what specifications should be changed. It's good to see more experiment and prototype for this issue. Like Alan said, the spec change, compatibility risks and security issues in particular on the serialization spec/impl change require lot of discussions and also prototyping of other alternatives. A new API may also be one alternative to consider. The current spec of Proxy class is defined with null protection domain (same protection domain as the bootstrap class loader). Lookup::defineHiddenClass defines the hidden class in the same protection domain as the defining class loader. This would become a non-issue when the security manager is removed. However, before the removal of security manager, we still need to look into the compatibility risks and what and how applications/libraries depend on this behavior. During the development of JEP 371, I had a prototype of converting dynamic proxies to hidden class by adding a shim class (that's what your prototype does). That raises the question "how to get a Lookup on a package for the dynamic proxy class to be injected in without injecting a shim class (i.e. your anchor class)?" This functionality could be considered as a separate RFE. However, frameworks don't always have the access to inject a class in a package defined to a class loader. This is something worth looking into. - PR: https://git.openjdk.java.net/jdk/pull/8278
Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v3]
On Fri, 15 Apr 2022 15:36:35 GMT, Vicente Romero wrote: >> Jan Lahoda has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Cleanup. > > src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1779: > >> 1777: //binding pattern >> 1778: attribExpr(pat, switchEnv); >> 1779: var primary = TreeInfo.primaryPatternType(pat); > > general comment: the handleSwitch method is getting more and more complex, > please consider refactoring it, probably splitting it, for example different > subrutines handling different case kinds. Of course this probably should be > done as a separate effort. Thanks for the comment. I agree the method is fairly complex, but it is also a bit difficult to disentangle into separate methods. We've separated some of the checks to `Check.checkSwitchCaseStructure` before. I'll try to think of splitting it more, but I am not sure where that will lead. - PR: https://git.openjdk.java.net/jdk/pull/8182
Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v3]
On Tue, 12 Apr 2022 13:18:14 GMT, Jan Lahoda wrote: >> This is a (preliminary) patch for javac implementation for the third preview >> of pattern matching for switch (type patterns in switches). >> >> Draft JLS: >> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwitchPlusRecordPatterns-20220407/specs/patterns-switch-jls.html >> >> The changes are: >> -there are no guarded patterns anymore, guards are not bound to the >> CaseElement (JLS 15.28) >> -a new contextual keyword `when` is used to add a guard, instead of `&&` >> -`null` selector value is handled on switch level (if a switch has `case >> null`, it is used, otherwise a NPE is thrown), rather than on pattern >> matching level. >> -total patterns are allowed in `instanceof` >> -`java.lang.MatchException` is added for the case where a switch is >> exhaustive (due to sealed types) at compile-time, but not at runtime. >> >> Feedback is welcome! >> >> Thanks! > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Cleanup. Thanks for the comments so far. I've reflected most of them here: https://github.com/openjdk/jdk/pull/8182/commits/311d68a60200dc4d9928e5ea2854c22fbcf68596 please let me know what you think! - PR: https://git.openjdk.java.net/jdk/pull/8182
Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v4]
> This is a (preliminary) patch for javac implementation for the third preview > of pattern matching for switch (type patterns in switches). > > Draft JLS: > http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwitchPlusRecordPatterns-20220407/specs/patterns-switch-jls.html > > The changes are: > -there are no guarded patterns anymore, guards are not bound to the > CaseElement (JLS 15.28) > -a new contextual keyword `when` is used to add a guard, instead of `&&` > -`null` selector value is handled on switch level (if a switch has `case > null`, it is used, otherwise a NPE is thrown), rather than on pattern > matching level. > -total patterns are allowed in `instanceof` > -`java.lang.MatchException` is added for the case where a switch is > exhaustive (due to sealed types) at compile-time, but not at runtime. > > Feedback is welcome! > > Thanks! Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision: Adjusting to review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8182/files - new: https://git.openjdk.java.net/jdk/pull/8182/files/ef7a7d6a..311d68a6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8182=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8182=02-03 Stats: 27 lines in 6 files changed: 1 ins; 0 del; 26 mod Patch: https://git.openjdk.java.net/jdk/pull/8182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8182/head:pull/8182 PR: https://git.openjdk.java.net/jdk/pull/8182
Re: RFR: 8186958: Need method to create pre-sized HashMap [v21]
On Thu, 14 Apr 2022 20:16:38 GMT, Sean Mullan wrote: >>> Are the changes necessary for this part? >> >> @seanjmullan no, they are just performance refinement. >> >> If you really that wanna 100% sync , >> >> I can use the old 1.8 api to migrate that part, and make a mirror pr to that >> part of https://github.com/apache/santuario-xml-security-java >> >> Is this solution acceptable then? > >> > Are the changes necessary for this part? >> >> @seanjmullan no, they are just performance refinement. >> >> If you really that wanna 100% sync , >> >> I can use the old 1.8 api to migrate that part, and make a mirror pr to that >> part of https://github.com/apache/santuario-xml-security-java >> >> Is this solution acceptable then? > > Yes, that would be preferred. Thanks! > I'd like to see a confirmation from @seanjmullan to make sure the issues with > Santuario are resolved satisfactorily. Other than that I think it's pretty > well covered. I've already run these changes through our internal test system > and they look fine. I'll do a final recheck and let you know. I am fine with this being integrated. @XenoAmess already [submitted a PR to the Santuario Project](https://github.com/apache/santuario-xml-security-java/pull/64) using the existing `HashMap` constructor which I have approved. So the code will be in sync the next time we upgrade the JDK to a newer version of Santuario. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8284950: Swappiness disables swap space usage
On Mon, 18 Apr 2022 16:22:27 GMT, Ioi Lam wrote: >> set memory.swappiness to 0,swap space will not be used >> determine the value of memory.swappiness >> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt >> >> >> Memory Limit: 50.00M >> Memory Soft Limit: Unlimited >> Memory & Swap Limit: 100.00M >> Maximum Processes Limit: 4194305 >> >> => >> >> Memory Limit: 50.00M >> Memory Soft Limit: Unlimited >> Memory & Swap Limit: 50.00M >> Maximum Processes Limit: 4194305 > > src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1Subsystem.java > line 155: > >> 153: long memswBytes = getLongValue(controller, >> "memory.memsw.limit_in_bytes"); >> 154: long swappiness = getLongValue(controller, "memory.swappiness"); >> 155: return (memswBytes > 0 && swappiness > 0); > > Does this also need to be changed in the test? > > https://github.com/openjdk/jdk/blob/c63fabe3d582ce0828b04b0224cea49aab5fedf3/test/lib/jdk/test/lib/containers/cgroup/MetricsTesterCgroupV1.java#L291-L296 There's also corresponding code in HotSpot: https://github.com/openjdk/jdk/blob/c63fabe3d582ce0828b04b0224cea49aab5fedf3/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp#L129-L150 - PR: https://git.openjdk.java.net/jdk/pull/8285
Re: RFR: 8284950: Swappiness disables swap space usage
On Mon, 18 Apr 2022 09:07:31 GMT, xpbob wrote: > set memory.swappiness to 0,swap space will not be used > determine the value of memory.swappiness > https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt > > > Memory Limit: 50.00M > Memory Soft Limit: Unlimited > Memory & Swap Limit: 100.00M > Maximum Processes Limit: 4194305 > > => > > Memory Limit: 50.00M > Memory Soft Limit: Unlimited > Memory & Swap Limit: 50.00M > Maximum Processes Limit: 4194305 src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1Subsystem.java line 155: > 153: long memswBytes = getLongValue(controller, > "memory.memsw.limit_in_bytes"); > 154: long swappiness = getLongValue(controller, "memory.swappiness"); > 155: return (memswBytes > 0 && swappiness > 0); Does this also need to be changed in the test? https://github.com/openjdk/jdk/blob/c63fabe3d582ce0828b04b0224cea49aab5fedf3/test/lib/jdk/test/lib/containers/cgroup/MetricsTesterCgroupV1.java#L291-L296 - PR: https://git.openjdk.java.net/jdk/pull/8285
Re: RFR: 8186958: Need method to create pre-sized HashMap [v21]
On Thu, 14 Apr 2022 20:16:38 GMT, Sean Mullan wrote: >>> Are the changes necessary for this part? >> >> @seanjmullan no, they are just performance refinement. >> >> If you really that wanna 100% sync , >> >> I can use the old 1.8 api to migrate that part, and make a mirror pr to that >> part of https://github.com/apache/santuario-xml-security-java >> >> Is this solution acceptable then? > >> > Are the changes necessary for this part? >> >> @seanjmullan no, they are just performance refinement. >> >> If you really that wanna 100% sync , >> >> I can use the old 1.8 api to migrate that part, and make a mirror pr to that >> part of https://github.com/apache/santuario-xml-security-java >> >> Is this solution acceptable then? > > Yes, that would be preferred. Thanks! I'd like to see a confirmation from @seanjmullan to make sure the issues with Santuario are resolved satisfactorily. Other than that I think it's pretty well covered. I've already run these changes through our internal test system and they look fine. I'll do a final recheck and let you know. - PR: https://git.openjdk.java.net/jdk/pull/7928
Integrated: 8283870: jdeprscan --help causes an exception when the locale is ja, zh_CN or de
On Mon, 11 Apr 2022 05:31:49 GMT, Koichi Sakata wrote: > # Summary > Running jdeprscan with --help option causes an exception on any OSs when the > locale is ja, zh_CN or de. > > # How to reproduce this issue > > $ jdeprscan -J-Duser.language=ja --help > Exception in thread "main" java.lang.IllegalArgumentException: can't parse > argument number: dir|jar|class > at > java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1454) > at > java.base/java.text.MessageFormat.applyPattern(MessageFormat.java:492) > at java.base/java.text.MessageFormat.(MessageFormat.java:371) > at java.base/java.text.MessageFormat.format(MessageFormat.java:860) > at jdk.jdeps/com.sun.tools.jdeprscan.Messages.get(Messages.java:62) > at jdk.jdeps/com.sun.tools.jdeprscan.Main.printHelp(Main.java:706) > at jdk.jdeps/com.sun.tools.jdeprscan.Main.run(Main.java:529) > at jdk.jdeps/com.sun.tools.jdeprscan.Main.call(Main.java:717) > at jdk.jdeps/com.sun.tools.jdeprscan.Main.main(Main.java:725) > Caused by: java.lang.NumberFormatException: For input string: "dir|jar|class" > at > java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67) > at java.base/java.lang.Integer.parseInt(Integer.java:668) > at java.base/java.lang.Integer.parseInt(Integer.java:786) > at > java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1452) > ... 8 more > > $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help > (Same as above) > > $ jdeprscan -J-Duser.language=de --help > (Same as above) > > Of course, it works well when the locale is anything other than those locales. > > # Details > I found **''**{dir|jar|class}**''** in both ja and zh_CN properties files. > Doubled single quotes mean a single quote in MessageFormat class, so the > class recognizes {dir|jar|class} as a FormatElement and throws the exception. > > I removed extra single quotes. > > # Test > It seems there is no test class for it. So I run commands as I mentioned > before. > > $ jdeprscan -J-Duser.language=ja --help > 使用方法: jdeprscan [options] {dir|jar|class} ... > > オプション: > --class-path PATH > --for-removal > --full-version > -? -h --help > -l--list > --release 7|8|9|10|11|12|13|14|15|16|17|18|19 > -v--verbose > --version > > 非推奨APIの使用について各引数をスキャンします。引数には、 > パッケージ階層のルートを指定するディレクトリ、JARファイル、 > クラス・ファイルまたはクラス名を使用できます。クラス名は、 > 完全修飾クラス名を使用して指定する必要があります。ネストされた > クラスは$で区切ります。例: > > java.lang.Thread$State > > --class-pathオプションは、依存するクラスの解決のための > 検索パスを指定します。 > > --for-removalオプションは、スキャンとリスト化を削除予定で非推奨のAPIに > 限定します。リリース値が6、7または8の場合は使用できません。 > > --full-versionオプションはツールのバージョン文字列の全体を出力します。 > > --help (-? -h)オプションは、ヘルプ・メッセージ全体を出力します。 > > --list (-l)オプションは非推奨APIセットを出力します。スキャンは行われないため、 > ディレクトリ、JARまたはクラス引数を指定する必要はありません。 > > --releaseオプションは、スキャンする非推奨APIのセットを提供するJava SE > リリースを指定します。 > > --verbose (-v)オプションを使用すると、処理中に追加のメッセージを出力できます。 > > --versionオプションは、簡略化されたツールのバージョン文字列を出力します。 > > $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help > 用法:jdeprscan [options] {dir|jar|class} ... > > 选项: > --class-path PATH > --for-removal > --full-version > -? -h --help > -l--list > --release 7|8|9|10|11|12|13|14|15|16|17|18|19 > -v--verbose > --version > > 扫描每个参数以了解是否使用了过时的 API。 > 参数可以是指定程序包分层结构、JAR 文件、 > 类文件或类名的根的目录。类名必须 > 使用全限定类名指定,并使用 $ 分隔符 > 指定嵌套类,例如, > > java.lang.Thread$State > > --class-path 选项提供了用于解析从属类的 > 搜索路径。 > > --for-removal 选项限制扫描或列出已过时并待删除 > 的 API。不能与发行版值 6、7 或 8 一起使用。 > > --full-version 选项输出工具的完整版本字符串。 > > --help (-? -h) 选项输出完整的帮助消息。 > > --list (-l) 选项输出一组已过时的 API。不执行扫描, > 因此不应提供任何目录、jar 或类参数。 > > --release 选项指定提供要扫描的已过时 API 集 > 的 Java SE 发行版。 > > --verbose (-v) 选项在处理期间启用附加消息输出。 > > --version 选项输出工具的缩写版本字符串。 > > $ jdeprscan -J-Duser.language=de --help > Verwendung: jdeprscan [Optionen] {dir|jar|class} ... > > Optionen: > --class-path PATH > --for-removal > --full-version > -? -h --help > -l--list > --release 7|8|9|10|11|12|13|14|15|16|17|18|19 > -v--verbose > --version > > Scannt jedes Argument auf die Verwendung veralteter APIs. Ein Argument > kann ein Verzeichnis, das die Root einer Packagehierarchie angibt, > eine JAR-Datei, eine Klassendatei oder ein Klassenname sein. Der Klassenname > muss durch einen vollständig qualifizierten Klassennamen mit $ als > Trennzeichen > für verschachtelte Klassen angegeben werden. Beispiel: > > java.lang.Thread$State > > Die Option --class-path liefert einen Suchpfad für die Auflösung > von abhängigen Klassen. > > Die Option --for-removal begrenzt das Scannen oder Auflisten auf APIs, die > veraltet sind > und entfernt werden sollen. Kann nicht mit den Releasewerten 6, 7, oder 8 > verwendet werden. > > Die Option --full-version gibt die vollständige Versionszeichenfolge des > Tools
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v4]
On Sun, 17 Apr 2022 16:03:30 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh src/java.base/share/classes/jdk/internal/misc/ThreadFlock.java line 266: > 264: * contained in the flock > 265: * @throws jdk.incubator.concurrent.StructureViolationException if > the current > 266: * scope-local bindings are not the same as when the flock was > created Suggestion: * scope-local bindings are not the same as when the flock was created - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284932: [Vector API] Incorrect implementation of LSHR operator for negative byte/short elements
On Mon, 18 Apr 2022 08:29:52 GMT, Jie Fu wrote: >>> @DamonFool >>> >>> I think the issue is that these two cases of yours are not equal >>> semantically. >> >> Why? >> According to the vector api doc, they should compute the same value when the >> shift_cnt is 3, right? >> >>> >>> ``` >>> 13 public static void urshift(byte[] src, byte[] dst) { >>> 14 for (int i = 0; i < src.length; i++) { >>> 15 dst[i] = (byte)(src[i] >>> 3); >>> 16 } >>> 17 } >>> 18 >>> 19 public static void urshiftVector(byte[] src, byte[] dst) { >>> 20 int i = 0; >>> 21 for (; i < spec.loopBound(src.length); i +=spec.length()) { >>> 22 var va = ByteVector.fromArray(spec, src, i); >>> 23 var vb = va.lanewise(VectorOperators.LSHR, 3); >>> 24 vb.intoArray(dst, i); >>> 25 } >>> 26 >>> 27 for (; i < src.length; i++) { >>> 28 dst[i] = (byte)(src[i] >>> 3); >>> 29 } >>> 30 } >>> ``` >>> >>> Besides the unsigned shift, line15 also has a type conversion which is >>> missing in the vector api case. To get the equivalent result, one need to >>> cast the result explicitly at line24, e.g, >>> `((IntVector)vb.castShape(SPECISE_XXX, 0)).intoArray(idst, i);` >> >> Since all the vector operations are already based on byte lane type, I don't >> think we need a `cast` operation here. >> Can we solve this problem by inserting a cast operation? > >> @DamonFool >> >> `>>>` can not apply to sub-word type in Java. `(byte)(src[i] >>> 3)` is >> unsigned right shift in type of INT and transformed the result to BYTE. In >> vector api, it extends the `>>>` to sub-word type with the same semantic >> meaning like `iushr`[1], that is zero extending. >> >> > The vector api docs says it would compute a>>>(n&(ESIZE*8-1)). >> >> I think `>>>` has some extending meanings here. As I said above, no sub-word >> type for `>>>` in Java. >> >> [1] >> https://docs.oracle.com/javase/specs/jvms/se18/html/jvms-6.html#jvms-6.5.iushr > > As discussed above > https://github.com/openjdk/jdk/pull/8276#issuecomment-1101016904 , there > isn't any problem to apply `>>>` upon shorts/bytes. > > What do you think of https://github.com/openjdk/jdk/pull/7979 , which tries > to vectorize unsigned shift right on signed subword types ? > And what do you think of the benchmarks mentioned in that PR? > > The vector api doc clearly states `LSHR` operator would compute > `a>>>(n&(ESIZE*8-1))`. > But it fails to do so when `a` is negative byte/short element. > > So if the doc description is correct, the current implementation would be > wrong, right? > > However, if you think the current implementation is correct, the vector api > doc would be wrong. > Then, we would lack an operator working like the scalar `>>>` since current > implementation fails to do so for negative bytes/shorts. > Hi @DamonFool, the doc does obviously not mean what you think, and actually > seems to indicate the Eric's interpretation instead. Simply because `a >>> (n > & (ESIZE - 1))` does not output the type of `a` for subword-type inputs, > which is clearly wrong. This suggests that the doc here should be interpreted > that `>>>` is the extended shift operation, which is defined on subword types > the same as for words and double-words. Though, I agree that the doc must be > modified to reflect the intention more clearly. > My intention is to make Vector API to be more friendly to Java developers. The so called extended unsigned right shift operation for bytes/shorts actually behave differently with the well-known scalar `>>>`, which may become the source of bugs. > > Then, we would lack an operator working like the scalar >>> since current > > implementation fails to do so for negative bytes/shorts. > > As you have noted, we have `ASHR` for bytes, shorts and `LSHR` for ints, > longs. Thanks a lot. Then people have to be very careful about when to use `AHSR` and when to use `LSHR`, which is really inconvenient and easy to make a mistake. And not all the people are smart enough to know this skill for bytes/shorts. So simply modifying the vector api doc can't solve these problems. Maybe, we can add one more operator to distinguish the semantics of scalar `>>>` with the so called extended unsigned right shift operation for bytes/shorts. - PR: https://git.openjdk.java.net/jdk/pull/8276
Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes
- Original Message - > From: "Johannes Kuhn" > To: "Alan Bateman" , "core-libs-dev" > > Sent: Monday, April 18, 2022 12:53:45 PM > Subject: Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes > On 18-Apr-22 9:36, Alan Bateman wrote: >> On 17/04/2022 17:24, liach wrote: >>> Convert dynamic proxies to hidden classes. Modifies the serialization >>> of proxies (requires change in "Java Object Serialization >>> Specification"). Makes the proxies hidden in stack traces. Removes >>> duplicate logic in proxy building. >>> >>> The main compatibility changes and their rationales are: >>> 1. Modification to the serialization specification: In the "An >>> instance of the class is allocated... The contents restored >>> appropriately" section, I propose explicitly state that handling of >>> proxies are unspecified as to allow implementation freedom, though >>> I've seen deliberate attempts for proxies to implement interfaces with >>> `readResolve` in order to control their serialization behavior. >>> - This is for the existing generated constructor accessor is >>> bytecode-based, which cannot refer to hidden classes. >>> - An alternative is to preserve the behavior, where the >>> serialization constructor calls `invokespecial` on the closest >>> serializable superclass' no-arg constructor, like in #1830 by @DasBrain. >>> - My rationale against preservation is such super calls are unsafe >>> and should be discouraged in the long term. Calling the existing >>> constructor with a dummy argument, as in my implementation, would be >>> more safe. >>> 2. The disappearance of proxies in stack traces. >>> - Same behavior exists in lambda expressions: For instance, in >>> `((Runnable) () -> { throw new Error(); }).run();`, the `run` method, >>> implemented by the lambda, will not appear in the stack trace, and >>> isn't too problematic. >> >> It's great that you have time to experiment in this area but just to set >> expectations that it will likely require significant discussion and >> maybe prototyping of alternatives. It suspect the Reviewer effort will >> end up being many times the effort required to do the actual work >> because of the compatibility and security issues that will need to be >> worked through. >> >> I think you need to add non-discoverability to the list of compatibility >> issues. Do we know for sure that there aren't frameworks and libraries >> that use Class.forName on proxy classes? We've had issues in the past >> with changes in this area. >> >> It's too early to say but it might be that the compatibility risks may >> nudge this one into creating a new API. >> >> -Alan. >> >> >> > > Proxies will have to be rethought at some future point - wrt Valhalla. > > The current specification says: > > > A proxy class implements exactly the interfaces specified at its > creation, in the same order. Invoking getInterfaces on its Class object > will return an array containing the same list of interfaces (in the > order specified at its creation), [...] > > In the current design Valhalla will add two interfaces - IdentityObject > and ValueObject (afaik). One of them have to be implemented as well. > Also, because the superclass is java.lang.reflect.Proxy, and that class > has a protected final field, it can currently not implement ValueObject. Recently, we (the Valhalla EG) have decided that IdentityObject/ValueObject were not the right way to represent identity and value class. So no problem anymore on that side. > > An other thing are annotations - currently they are implemented using > Proxies. This implementation detail surfaces when serializing an > annotation. Other implementation strategies are possible - for example > spinning a record at runtime. > But this leads to the question - how can one migrate away from > serialized proxies in a compatible way? > > In the end - a lot of stuff in the JDK depends on Proxies, and their > limitations now begins to surface. > > A new API may not be a bad idea. :) Yes ! And we can leverage invokedynamic/method handles to avoid boxing/ bad perf. The idea is that first time an implementation of an abstract method is required, an object (implementing an interface similar to InvocationHandler) acting as a bootstrap method is called to provide a method handle that will be used as implementation. see https://github.com/forax/hidden_proxy for a prototype of that idea. > > - Johannes Rémi
Re: RFR: 8284932: [Vector API] Incorrect implementation of LSHR operator for negative byte/short elements
On Mon, 18 Apr 2022 08:29:52 GMT, Jie Fu wrote: >>> @DamonFool >>> >>> I think the issue is that these two cases of yours are not equal >>> semantically. >> >> Why? >> According to the vector api doc, they should compute the same value when the >> shift_cnt is 3, right? >> >>> >>> ``` >>> 13 public static void urshift(byte[] src, byte[] dst) { >>> 14 for (int i = 0; i < src.length; i++) { >>> 15 dst[i] = (byte)(src[i] >>> 3); >>> 16 } >>> 17 } >>> 18 >>> 19 public static void urshiftVector(byte[] src, byte[] dst) { >>> 20 int i = 0; >>> 21 for (; i < spec.loopBound(src.length); i +=spec.length()) { >>> 22 var va = ByteVector.fromArray(spec, src, i); >>> 23 var vb = va.lanewise(VectorOperators.LSHR, 3); >>> 24 vb.intoArray(dst, i); >>> 25 } >>> 26 >>> 27 for (; i < src.length; i++) { >>> 28 dst[i] = (byte)(src[i] >>> 3); >>> 29 } >>> 30 } >>> ``` >>> >>> Besides the unsigned shift, line15 also has a type conversion which is >>> missing in the vector api case. To get the equivalent result, one need to >>> cast the result explicitly at line24, e.g, >>> `((IntVector)vb.castShape(SPECISE_XXX, 0)).intoArray(idst, i);` >> >> Since all the vector operations are already based on byte lane type, I don't >> think we need a `cast` operation here. >> Can we solve this problem by inserting a cast operation? > >> @DamonFool >> >> `>>>` can not apply to sub-word type in Java. `(byte)(src[i] >>> 3)` is >> unsigned right shift in type of INT and transformed the result to BYTE. In >> vector api, it extends the `>>>` to sub-word type with the same semantic >> meaning like `iushr`[1], that is zero extending. >> >> > The vector api docs says it would compute a>>>(n&(ESIZE*8-1)). >> >> I think `>>>` has some extending meanings here. As I said above, no sub-word >> type for `>>>` in Java. >> >> [1] >> https://docs.oracle.com/javase/specs/jvms/se18/html/jvms-6.html#jvms-6.5.iushr > > As discussed above > https://github.com/openjdk/jdk/pull/8276#issuecomment-1101016904 , there > isn't any problem to apply `>>>` upon shorts/bytes. > > What do you think of https://github.com/openjdk/jdk/pull/7979 , which tries > to vectorize unsigned shift right on signed subword types ? > And what do you think of the benchmarks mentioned in that PR? > > The vector api doc clearly states `LSHR` operator would compute > `a>>>(n&(ESIZE*8-1))`. > But it fails to do so when `a` is negative byte/short element. > > So if the doc description is correct, the current implementation would be > wrong, right? > > However, if you think the current implementation is correct, the vector api > doc would be wrong. > Then, we would lack an operator working like the scalar `>>>` since current > implementation fails to do so for negative bytes/shorts. Hi @DamonFool, the doc does obviously not mean what you think, and actually seems to indicate the Eric's interpretation instead. Simply because `a >>> (n & (ESIZE - 1))` does not output the type of `a` for subword-type inputs, which is clearly wrong. This suggests that the doc here should be interpreted that `>>>` is the extended shift operation, which is defined on subword types the same as for words and double-words. Though, I agree that the doc must be modified to reflect the intention more clearly. > Then, we would lack an operator working like the scalar >>> since current > implementation fails to do so for negative bytes/shorts. As you have noted, we have `ASHR` for bytes, shorts and `LSHR` for ints, longs. Thanks a lot. - PR: https://git.openjdk.java.net/jdk/pull/8276
RFR: 8284950: Swappiness disables swap space usage
set memory.swappiness to 0,swap space will not be used determine the value of memory.swappiness https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt Memory Limit: 50.00M Memory Soft Limit: Unlimited Memory & Swap Limit: 100.00M Maximum Processes Limit: 4194305 => Memory Limit: 50.00M Memory Soft Limit: Unlimited Memory & Swap Limit: 50.00M Maximum Processes Limit: 4194305 - Commit messages: - 8284950: Swappiness disables swap space usage Changes: https://git.openjdk.java.net/jdk/pull/8285/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8285=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284950 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8285.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8285/head:pull/8285 PR: https://git.openjdk.java.net/jdk/pull/8285
Re: RFR: 8284932: [Vector API] Incorrect implementation of LSHR operator for negative byte/short elements
On Mon, 18 Apr 2022 05:14:25 GMT, Jie Fu wrote: >>> > However, just image that someone would like to optimize some code >>> > segments of bytes/shorts `>>>` >>> >>> Then that person can just use signed shift (`VectorOperators.ASHR`), right? >>> Shifting on masked shift counts means that the shift count cannot be >>> greater than 8 for bytes and 16 for shorts, which means that `(byte)(src[i] >>> >>> 3)` is exactly the same as `(byte)(src[i] >> 3)`. Please correct me if >>> I misunderstood something here. >> >> Yes, you're right that's why I said `LSHR` can be replaced with `ASHR`. >> >> However, not all the people are clever enough to do this source code level >> replacement. >> To be honest, I would never think of that `>>>` can be auto-vectorized by >> this idea proposed by https://github.com/openjdk/jdk/pull/7979 . >> >>> >>> Your proposed changes make unsigned shifts for subwords behave exactly the >>> same as signed shifts, which is both redundant (we have 2 operations doing >>> exactly the same thing) and inadequate (we lack the operation to do the >>> proper unsigned shifts) >> >> `LSHR` following the behavior of scalar `>>>` is very important for Java >> developers to rewrite the code with vector api. >> Maybe, we should add one more operator to support what you called `the >> proper unsigned shifts`, right? >> But that's another topic which can be done in a separate issue. > >> @DamonFool >> >> I think the issue is that these two cases of yours are not equal >> semantically. > > Why? > According to the vector api doc, they should compute the same value when the > shift_cnt is 3, right? > >> >> ``` >> 13 public static void urshift(byte[] src, byte[] dst) { >> 14 for (int i = 0; i < src.length; i++) { >> 15 dst[i] = (byte)(src[i] >>> 3); >> 16 } >> 17 } >> 18 >> 19 public static void urshiftVector(byte[] src, byte[] dst) { >> 20 int i = 0; >> 21 for (; i < spec.loopBound(src.length); i +=spec.length()) { >> 22 var va = ByteVector.fromArray(spec, src, i); >> 23 var vb = va.lanewise(VectorOperators.LSHR, 3); >> 24 vb.intoArray(dst, i); >> 25 } >> 26 >> 27 for (; i < src.length; i++) { >> 28 dst[i] = (byte)(src[i] >>> 3); >> 29 } >> 30 } >> ``` >> >> Besides the unsigned shift, line15 also has a type conversion which is >> missing in the vector api case. To get the equivalent result, one need to >> cast the result explicitly at line24, e.g, >> `((IntVector)vb.castShape(SPECISE_XXX, 0)).intoArray(idst, i);` > > Since all the vector operations are already based on byte lane type, I don't > think we need a `cast` operation here. > Can we solve this problem by inserting a cast operation? > @DamonFool > > `>>>` can not apply to sub-word type in Java. `(byte)(src[i] >>> 3)` is > unsigned right shift in type of INT and transformed the result to BYTE. In > vector api, it extends the `>>>` to sub-word type with the same semantic > meaning like `iushr`[1], that is zero extending. > > > The vector api docs says it would compute a>>>(n&(ESIZE*8-1)). > > I think `>>>` has some extending meanings here. As I said above, no sub-word > type for `>>>` in Java. > > [1] > https://docs.oracle.com/javase/specs/jvms/se18/html/jvms-6.html#jvms-6.5.iushr As discussed above https://github.com/openjdk/jdk/pull/8276#issuecomment-1101016904 , there isn't any problem to apply `>>>` upon shorts/bytes. What do you think of https://github.com/openjdk/jdk/pull/7979 , which tries to vectorize unsigned shift right on signed subword types ? And what do you think of the benchmarks mentioned in that PR? The vector api doc clearly states `LSHR` operator would compute `a>>>(n&(ESIZE*8-1))`. But it fails to do so when `a` is negative byte/short element. So if the doc description is correct, the current implementation would be wrong, right? However, if you think the current implementation is correct, the vector api doc would be wrong. Then, we would lack an operator working like the scalar `>>>` since current implementation fails to do so for negative bytes/shorts. - PR: https://git.openjdk.java.net/jdk/pull/8276
Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes
On 17/04/2022 17:24, liach wrote: Convert dynamic proxies to hidden classes. Modifies the serialization of proxies (requires change in "Java Object Serialization Specification"). Makes the proxies hidden in stack traces. Removes duplicate logic in proxy building. The main compatibility changes and their rationales are: 1. Modification to the serialization specification: In the "An instance of the class is allocated... The contents restored appropriately" section, I propose explicitly state that handling of proxies are unspecified as to allow implementation freedom, though I've seen deliberate attempts for proxies to implement interfaces with `readResolve` in order to control their serialization behavior. - This is for the existing generated constructor accessor is bytecode-based, which cannot refer to hidden classes. - An alternative is to preserve the behavior, where the serialization constructor calls `invokespecial` on the closest serializable superclass' no-arg constructor, like in #1830 by @DasBrain. - My rationale against preservation is such super calls are unsafe and should be discouraged in the long term. Calling the existing constructor with a dummy argument, as in my implementation, would be more safe. 2. The disappearance of proxies in stack traces. - Same behavior exists in lambda expressions: For instance, in `((Runnable) () -> { throw new Error(); }).run();`, the `run` method, implemented by the lambda, will not appear in the stack trace, and isn't too problematic. It's great that you have time to experiment in this area but just to set expectations that it will likely require significant discussion and maybe prototyping of alternatives. It suspect the Reviewer effort will end up being many times the effort required to do the actual work because of the compatibility and security issues that will need to be worked through. I think you need to add non-discoverability to the list of compatibility issues. Do we know for sure that there aren't frameworks and libraries that use Class.forName on proxy classes? We've had issues in the past with changes in this area. It's too early to say but it might be that the compatibility risks may nudge this one into creating a new API. -Alan.
Re: RFR: 8284932: [Vector API] Incorrect implementation of LSHR operator for negative byte/short elements
On Mon, 18 Apr 2022 05:14:25 GMT, Jie Fu wrote: >>> > However, just image that someone would like to optimize some code >>> > segments of bytes/shorts `>>>` >>> >>> Then that person can just use signed shift (`VectorOperators.ASHR`), right? >>> Shifting on masked shift counts means that the shift count cannot be >>> greater than 8 for bytes and 16 for shorts, which means that `(byte)(src[i] >>> >>> 3)` is exactly the same as `(byte)(src[i] >> 3)`. Please correct me if >>> I misunderstood something here. >> >> Yes, you're right that's why I said `LSHR` can be replaced with `ASHR`. >> >> However, not all the people are clever enough to do this source code level >> replacement. >> To be honest, I would never think of that `>>>` can be auto-vectorized by >> this idea proposed by https://github.com/openjdk/jdk/pull/7979 . >> >>> >>> Your proposed changes make unsigned shifts for subwords behave exactly the >>> same as signed shifts, which is both redundant (we have 2 operations doing >>> exactly the same thing) and inadequate (we lack the operation to do the >>> proper unsigned shifts) >> >> `LSHR` following the behavior of scalar `>>>` is very important for Java >> developers to rewrite the code with vector api. >> Maybe, we should add one more operator to support what you called `the >> proper unsigned shifts`, right? >> But that's another topic which can be done in a separate issue. > >> @DamonFool >> >> I think the issue is that these two cases of yours are not equal >> semantically. > > Why? > According to the vector api doc, they should compute the same value when the > shift_cnt is 3, right? > >> >> ``` >> 13 public static void urshift(byte[] src, byte[] dst) { >> 14 for (int i = 0; i < src.length; i++) { >> 15 dst[i] = (byte)(src[i] >>> 3); >> 16 } >> 17 } >> 18 >> 19 public static void urshiftVector(byte[] src, byte[] dst) { >> 20 int i = 0; >> 21 for (; i < spec.loopBound(src.length); i +=spec.length()) { >> 22 var va = ByteVector.fromArray(spec, src, i); >> 23 var vb = va.lanewise(VectorOperators.LSHR, 3); >> 24 vb.intoArray(dst, i); >> 25 } >> 26 >> 27 for (; i < src.length; i++) { >> 28 dst[i] = (byte)(src[i] >>> 3); >> 29 } >> 30 } >> ``` >> >> Besides the unsigned shift, line15 also has a type conversion which is >> missing in the vector api case. To get the equivalent result, one need to >> cast the result explicitly at line24, e.g, >> `((IntVector)vb.castShape(SPECISE_XXX, 0)).intoArray(idst, i);` > > Since all the vector operations are already based on byte lane type, I don't > think we need a `cast` operation here. > Can we solve this problem by inserting a cast operation? @DamonFool `>>>` can not apply to sub-word type in Java. `(byte)(src[i] >>> 3)` is unsigned right shift in type of INT and transformed the result to BYTE. In vector api, it extends the `>>>` to sub-word type with the same semantic meaning like `iushr`[1], that is zero extending. > The vector api docs says it would compute a>>>(n&(ESIZE*8-1)). I think `>>>` has some extending meanings here. As I said above, no sub-word type for `>>>` in Java. [1] https://docs.oracle.com/javase/specs/jvms/se18/html/jvms-6.html#jvms-6.5.iushr - PR: https://git.openjdk.java.net/jdk/pull/8276
RFR: 8284942: Proxy building can just iterate superinterfaces once
Currently, in ProxyBuilder::mapToModule and ProxyBuilder::defineProxyClass, the interfaces are iterated twice. The two passes can be merged into one, yielding the whole proxy definition context (module, package, whether there's package-private interface) when determining the module. Split from #8278. Helpful for moving proxies to hidden classes, but is a good cleanup on its own. - Commit messages: - 8284942: Proxy building can just iterate superinterfaces once Changes: https://git.openjdk.java.net/jdk/pull/8281/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8281=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284942 Stats: 64 lines in 1 file changed: 16 ins; 25 del; 23 mod Patch: https://git.openjdk.java.net/jdk/pull/8281.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8281/head:pull/8281 PR: https://git.openjdk.java.net/jdk/pull/8281