Re: RFR: 8284932: [Vector API] Incorrect implementation of LSHR operator for negative byte/short elements

2022-04-18 Thread Jie Fu
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]

2022-04-18 Thread Koichi Sakata
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]

2022-04-18 Thread Naoto Sato
> 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

2022-04-18 Thread xpbob
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

2022-04-18 Thread Quan Anh Mai
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]

2022-04-18 Thread Mandy Chung
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

2022-04-18 Thread David Holmes
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

2022-04-18 Thread liach
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]

2022-04-18 Thread Mandy Chung
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]

2022-04-18 Thread openjdk-notifier[bot]
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

2022-04-18 Thread XenoAmess
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]

2022-04-18 Thread Stuart Marks
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

2022-04-18 Thread Hiran Chaudhuri
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]

2022-04-18 Thread Mandy Chung
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

2022-04-18 Thread Alexey Semenyuk




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]

2022-04-18 Thread Stuart Marks
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

2022-04-18 Thread Remi Forax
- 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]

2022-04-18 Thread jmehrens
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

2022-04-18 Thread liach
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

2022-04-18 Thread Roger Riggs
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]

2022-04-18 Thread Roger Riggs
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

2022-04-18 Thread Pavel Rappo
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

2022-04-18 Thread Mandy Chung
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]

2022-04-18 Thread Jan Lahoda
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]

2022-04-18 Thread Jan Lahoda
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]

2022-04-18 Thread Jan Lahoda
> 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]

2022-04-18 Thread Sean Mullan
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

2022-04-18 Thread Ioi Lam
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

2022-04-18 Thread Ioi Lam
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]

2022-04-18 Thread Stuart Marks
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

2022-04-18 Thread Koichi Sakata
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]

2022-04-18 Thread ExE Boss
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

2022-04-18 Thread Jie Fu
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

2022-04-18 Thread Remi Forax
- 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

2022-04-18 Thread Quan Anh Mai
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

2022-04-18 Thread xpbob
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

2022-04-18 Thread Jie Fu
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

2022-04-18 Thread Alan Bateman

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

2022-04-18 Thread Eric Liu
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

2022-04-18 Thread liach
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