Re: RFR: 8287917: System.loadLibrary does not work on Big Sur if JDK is built with macOS SDK 10.15 and earlier [v2]

2022-06-13 Thread Yoshiki Sato
> Please review this PR.
> SDK 10.15 and earlier reports os.version as 10.16 on Big Sur.  This fix will 
> check if dynamic linker support, which is supported from Big Sur, is 
> available or not on the OS even if os.version is reported as 10.16 instead of 
> 11.  The os.version 10.16 doesn't include the update version like y of 
> 10.x.y.  Hence the change only see if it is 10.16 or not.

Yoshiki Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Update copyright end year

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9077/files
  - new: https://git.openjdk.org/jdk/pull/9077/files/a9b95965..bdd97130

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=9077=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9077=00-01

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

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


Re: RFR: 8287917: System.loadLibrary does not work on Big Sur if JDK is built with macOS SDK 10.15 and earlier

2022-06-13 Thread Yoshiki Sato
On Fri, 10 Jun 2022 17:51:37 GMT, Mandy Chung  wrote:

>> Please review this PR.
>> SDK 10.15 and earlier reports os.version as 10.16 on Big Sur.  This fix will 
>> check if dynamic linker support, which is supported from Big Sur, is 
>> available or not on the OS even if os.version is reported as 10.16 instead 
>> of 11.  The os.version 10.16 doesn't include the update version like y of 
>> 10.x.y.  Hence the change only see if it is 10.16 or not.
>
> test/jdk/java/lang/RuntimeTests/loadLibrary/exeLibraryCache/LibraryFromCache.java
>  line 45:
> 
>> 43: public class LibraryFromCache {
>> 44: public static void main(String[] args) throws IOException {
>> 45: System.out.println("os.version = " + 
>> System.getProperty("os.version"));
> 
> The copyright end year needs to be updated.

Thanks for catching.

-

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v25]

2022-06-13 Thread Joe Darcy
> This is an early review of changes to better model JVM access flags, that is 
> "modifiers" like public, protected, etc. but explicitly at a VM level.
> 
> Language level modifiers and JVM level access flags are closely related, but 
> distinct. There are concepts that overlap in the two domains (public, 
> private, etc.), others that only have a language-level modifier (sealed), and 
> still others that only have an access flag (synthetic).
> 
> The existing java.lang.reflect.Modifier class is inadequate to model these 
> subtleties. For example, the bit positions used by access flags on different 
> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
> methods. Just having a raw integer does not provide sufficient context to 
> decode the corresponding language-level string. Methods like 
> Modifier.methodModifiers() were introduced to cope with this situation.
> 
> With additional modifiers and flags on the horizon with projects like 
> Valhalla, addressing the existent modeling deficiency now ahead of time is 
> reasonable before further strain is introduced.
> 
> This PR in its current form is meant to give the overall shape of the API. It 
> is missing implementations to map from, say, method modifiers to access 
> flags, taking into account overlaps in bit positions.
> 
> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
> once the API is further along.

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Respond to review feedback.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/7445/files
  - new: https://git.openjdk.org/jdk/pull/7445/files/75ac9c18..adcbcb71

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7445=24
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7445=23-24

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

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]

2022-06-13 Thread Joe Darcy
On Wed, 1 Jun 2022 05:09:42 GMT, ExE Boss  wrote:

>> Or `AbstractMethodError`, which is what `Executable::getParameterCount()` 
>> does:
>> https://github.com/openjdk/jdk/blob/e751b7b1b6f7269a1fe20c07748c726536388f6d/src/java.base/share/classes/java/lang/reflect/Executable.java#L248-L258
>
> Actually, it should probably be `UnsupportedOperationException` instead.

Hmm. If we had sealed classes and interfaces back in JDK 1.1 when Member was 
added, it most likely would have been added as sealed interface. But, we didn't 
have sealed interfaces back then so Member can (potentially) be extended by 
anyone. IIRC, there are a few classes implementing Member outside of the JDK.

So, when adding a new method would JDK 20, the method should certainly have be 
default method. I think throwing UnsupportedOperationException in the default 
is marginally better than the alternatives. I'll update the PR accordingly in a 
subsequent push. Thanks.

-

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v24]

2022-06-13 Thread Joe Darcy
On Tue, 14 Jun 2022 01:40:53 GMT, liach  wrote:

>> Joe Darcy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Make mask fields final in ModuleDescriptor.
>
> src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 300:
> 
>> 298: /**
>> 299:  * {@return a set of access flags for the given mask value
>> 300:  * appropriate for the location in question}
> 
> Should we specify that the returned set is unmodifiable/immutable?

Sure; that would be consistent with other parts of the PR.

-

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v4]

2022-06-13 Thread Joe Darcy
On Tue, 31 May 2022 17:23:18 GMT, Rémi Forax  wrote:

>> For completeness, I think including SUPER is reasonable, even though has 
>> been a no-op for some time. (Some time in the future, there could be a class 
>> file version aware additions to this enum class.) Well spotted omission.
>
> `ACC_SUPER`may be reconed as `ACC_IDENTITY`by Valhalla, so i'm not sure it's 
> a good idea to add ACC_SUPER.

As discussed elsewhere in the PR, given the historical existence of ACC_SUPER, 
I think it is reasonable to leave it all. Also, even if removed later in the 
release, it would be helpful to validate how overlapping flags can be handled 
in practice in Valhalla.

-

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v24]

2022-06-13 Thread liach
On Tue, 14 Jun 2022 01:25:02 GMT, Joe Darcy  wrote:

>> This is an early review of changes to better model JVM access flags, that is 
>> "modifiers" like public, protected, etc. but explicitly at a VM level.
>> 
>> Language level modifiers and JVM level access flags are closely related, but 
>> distinct. There are concepts that overlap in the two domains (public, 
>> private, etc.), others that only have a language-level modifier (sealed), 
>> and still others that only have an access flag (synthetic).
>> 
>> The existing java.lang.reflect.Modifier class is inadequate to model these 
>> subtleties. For example, the bit positions used by access flags on different 
>> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
>> methods. Just having a raw integer does not provide sufficient context to 
>> decode the corresponding language-level string. Methods like 
>> Modifier.methodModifiers() were introduced to cope with this situation.
>> 
>> With additional modifiers and flags on the horizon with projects like 
>> Valhalla, addressing the existent modeling deficiency now ahead of time is 
>> reasonable before further strain is introduced.
>> 
>> This PR in its current form is meant to give the overall shape of the API. 
>> It is missing implementations to map from, say, method modifiers to access 
>> flags, taking into account overlaps in bit positions.
>> 
>> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
>> once the API is further along.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Make mask fields final in ModuleDescriptor.

src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 127:

> 125:  * 0x0020}.
> 126:  */
> 127: SUPER(0x_0020, false, Set.of(Location.CLASS)),

Should we document that this flag won't appear in `Class#accessFlags` no matter 
if it's declared in the class file?

src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 300:

> 298: /**
> 299:  * {@return a set of access flags for the given mask value
> 300:  * appropriate for the location in question}

Should we specify that the returned set is unmodifiable/immutable?

-

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v24]

2022-06-13 Thread Joe Darcy
> This is an early review of changes to better model JVM access flags, that is 
> "modifiers" like public, protected, etc. but explicitly at a VM level.
> 
> Language level modifiers and JVM level access flags are closely related, but 
> distinct. There are concepts that overlap in the two domains (public, 
> private, etc.), others that only have a language-level modifier (sealed), and 
> still others that only have an access flag (synthetic).
> 
> The existing java.lang.reflect.Modifier class is inadequate to model these 
> subtleties. For example, the bit positions used by access flags on different 
> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
> methods. Just having a raw integer does not provide sufficient context to 
> decode the corresponding language-level string. Methods like 
> Modifier.methodModifiers() were introduced to cope with this situation.
> 
> With additional modifiers and flags on the horizon with projects like 
> Valhalla, addressing the existent modeling deficiency now ahead of time is 
> reasonable before further strain is introduced.
> 
> This PR in its current form is meant to give the overall shape of the API. It 
> is missing implementations to map from, say, method modifiers to access 
> flags, taking into account overlaps in bit positions.
> 
> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
> once the API is further along.

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Make mask fields final in ModuleDescriptor.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/7445/files
  - new: https://git.openjdk.org/jdk/pull/7445/files/fd682ac8..75ac9c18

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7445=23
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7445=22-23

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

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]

2022-06-13 Thread Joe Darcy
On Tue, 31 May 2022 17:20:08 GMT, Rémi Forax  wrote:

>> Joe Darcy 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 32 additional commits 
>> since the last revision:
>> 
>>  - Target JDK 20 rather than 19.
>>  - Merge branch 'master' into JDK-8266670
>>  - Add mask values to constants' javadoc.
>>  - Implement review feedback from mlchung.
>>  - Fix type in @throws tag.
>>  - Merge branch 'master' into JDK-8266670
>>  - Respond to review feedback.
>>  - Merge branch 'master' into JDK-8266670
>>  - Make workding changes suggested in review feedback.
>>  - Merge branch 'master' into JDK-8266670
>>  - ... and 22 more: https://git.openjdk.org/jdk/compare/3d668090...05cf2d8b
>
> src/java.base/share/classes/java/lang/module/ModuleDescriptor.java line 130:
> 
>> 128: MANDATED(AccessFlag.MANDATED.mask());
>> 129: 
>> 130: private int mask;
> 
> it should be declared final ?

Sure; will change.

> src/java.base/share/classes/java/lang/module/ModuleDescriptor.java line 134:
> 
>> 132: this.mask = mask;
>> 133: }
>> 134: private int mask() {return mask;}
> 
> seem useless, `mask` can be accessed directly

Hmm. Stylistically, I prefer methods.

-

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


Integrated: 8288378: [BACKOUT] DST not applying properly with zone id offset set with TZ env variable

2022-06-13 Thread Naoto Sato
On Tue, 14 Jun 2022 00:56:47 GMT, Naoto Sato  wrote:

> Backing out a fix that is causing a T1 failure.
> 
> This reverts commit 9b6d0a7e94fd18d302c559bec6f785d71a919a88.

This pull request has now been integrated.

Changeset: fbe92666
Author:Naoto Sato 
URL:   
https://git.openjdk.org/jdk/commit/fbe926662287283c579fdb4ca8290670500cf5a5
Stats: 95 lines in 2 files changed: 1 ins; 91 del; 3 mod

8288378: [BACKOUT] DST not applying properly with zone id offset set with TZ 
env variable

Reviewed-by: dholmes

-

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


RFR: 8288378: [BACKOUT] DST not applying properly with zone id offset set with TZ env variable

2022-06-13 Thread Naoto Sato
Backing out a fix that is causing a T1 failure.

This reverts commit 9b6d0a7e94fd18d302c559bec6f785d71a919a88.

-

Commit messages:
 - 8288378: [BACKOUT] DST not applying properly with zone id offset set with TZ 
env variable

Changes: https://git.openjdk.org/jdk/pull/9151/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9151=00
  Issue: https://bugs.openjdk.org/browse/JDK-8288378
  Stats: 95 lines in 2 files changed: 1 ins; 91 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/9151.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9151/head:pull/9151

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


Re: RFR: 8288378: [BACKOUT] DST not applying properly with zone id offset set with TZ env variable

2022-06-13 Thread David Holmes
On Tue, 14 Jun 2022 00:56:47 GMT, Naoto Sato  wrote:

> Backing out a fix that is causing a T1 failure.
> 
> This reverts commit 9b6d0a7e94fd18d302c559bec6f785d71a919a88.

Backout looks accurate - thanks.

-

Marked as reviewed by dholmes (Reviewer).

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v23]

2022-06-13 Thread Joe Darcy
> This is an early review of changes to better model JVM access flags, that is 
> "modifiers" like public, protected, etc. but explicitly at a VM level.
> 
> Language level modifiers and JVM level access flags are closely related, but 
> distinct. There are concepts that overlap in the two domains (public, 
> private, etc.), others that only have a language-level modifier (sealed), and 
> still others that only have an access flag (synthetic).
> 
> The existing java.lang.reflect.Modifier class is inadequate to model these 
> subtleties. For example, the bit positions used by access flags on different 
> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
> methods. Just having a raw integer does not provide sufficient context to 
> decode the corresponding language-level string. Methods like 
> Modifier.methodModifiers() were introduced to cope with this situation.
> 
> With additional modifiers and flags on the horizon with projects like 
> Valhalla, addressing the existent modeling deficiency now ahead of time is 
> reasonable before further strain is introduced.
> 
> This PR in its current form is meant to give the overall shape of the API. It 
> is missing implementations to map from, say, method modifiers to access 
> flags, taking into account overlaps in bit positions.
> 
> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
> once the API is further along.

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Correct STATIC vs STATIC_PHASE issue found in code review.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/7445/files
  - new: https://git.openjdk.org/jdk/pull/7445/files/840edf2a..fd682ac8

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7445=22
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7445=21-22

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

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v17]

2022-06-13 Thread Joe Darcy
On Wed, 13 Apr 2022 21:21:25 GMT, liach  wrote:

>> src/java.base/share/classes/java/lang/module/ModuleDescriptor.java line 167:
>> 
>>> 165:  * but is optional in the dynamic phase, during execution.
>>> 166:  */
>>> 167: STATIC(AccessFlag.STATIC.mask()),
>> 
>> This is actually `AccessFlag.STATIC_PHASE` (`0x0040`), and not 
>> `AccessFlag.STATIC` (`0x0008`):
>> Suggestion:
>> 
>> STATIC(AccessFlag.STATIC_PHASE.mask()),
>
>> In the current hodgepodge AccessFlag, we have STATIC and STATIC_PHASE, and 
>> the incorrect ModuleDescriptor.accessFlags().contains(AccessFlag.STATIC) 
>> call is much more subtle, especially to new users of this class. Arguably, 
>> this misuse would be way worse than that in the distinct enum case.
> 
> Oops, didn't know this already happened. Good spot right there.

Corrected to STATIC_PHASE in subsequent push; thanks.

-

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v22]

2022-06-13 Thread Joe Darcy
> This is an early review of changes to better model JVM access flags, that is 
> "modifiers" like public, protected, etc. but explicitly at a VM level.
> 
> Language level modifiers and JVM level access flags are closely related, but 
> distinct. There are concepts that overlap in the two domains (public, 
> private, etc.), others that only have a language-level modifier (sealed), and 
> still others that only have an access flag (synthetic).
> 
> The existing java.lang.reflect.Modifier class is inadequate to model these 
> subtleties. For example, the bit positions used by access flags on different 
> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
> methods. Just having a raw integer does not provide sufficient context to 
> decode the corresponding language-level string. Methods like 
> Modifier.methodModifiers() were introduced to cope with this situation.
> 
> With additional modifiers and flags on the horizon with projects like 
> Valhalla, addressing the existent modeling deficiency now ahead of time is 
> reasonable before further strain is introduced.
> 
> This PR in its current form is meant to give the overall shape of the API. It 
> is missing implementations to map from, say, method modifiers to access 
> flags, taking into account overlaps in bit positions.
> 
> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
> once the API is further along.

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  From review feedback, use package-private contstants in Modifier.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/7445/files
  - new: https://git.openjdk.org/jdk/pull/7445/files/111c6014..840edf2a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7445=21
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7445=20-21

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

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


Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v4]

2022-06-13 Thread Joe Darcy
On Tue, 15 Feb 2022 09:06:02 GMT, ExE Boss  wrote:

>> Joe Darcy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Respond to more review feedback.
>
> src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 163:
> 
>> 161: * @see Class#isEnum()
>> 162: */
>> 163: ENUM(0x4000, false, Set.of(TYPE, FIELD)),
> 
> These can use the **package‑private** constant fields from 
> `java.lang.reflect.Modifier`: 
> 

Good suggestion; will be reflected in the next push.

-

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


Re: RFR: 8287186: JDK modules participating in preview [v3]

2022-06-13 Thread Paul Sandoz
> Allow JDK modules that use preview features (preview language features or 
> preview API features from dependent modules) to participate without the need 
> to compile with `--enable-preview`.
> 
> It's difficult to enable participation using an annotation due to the nature 
> in which symbols are encountered when processing source as there is no 
> guaranteed order to the processing of certain symbols.
> 
> Instead a JDK module participates if the `java.base` package 
> `jdk.internal.javac` is exported to that module (@lahodaj clever idea!). An 
> internal annotation `jdk.internal.javac.ParticipatesInPreview` can be 
> declared on the module. Such a declaration cannot be enforced but does by its 
> use require the `jdk.internal.javac`'s export list to be updated.
> 
> The modules `jdk.incubator.vector` and `jdk.incubator.concurrent` have been 
> updated accordingly, both of which participate in preview APIs (APIs in 
> `java.lang.foreign` and `Thread.ofVirtual`, respectively).

Paul Sandoz 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 four additional commits since 
the last revision:

 - Merge remote-tracking branch 'upstream/master' into 
JDK-8287186-preview-participating
 - Let java.management participate in preview features.
 - Unused import.
 - Generalize the pariticipating in preview APIs.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9087/files
  - new: https://git.openjdk.org/jdk/pull/9087/files/9defdf23..abd1fbf6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=9087=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9087=01-02

  Stats: 17219 lines in 554 files changed: 13408 ins; 1651 del; 2160 mod
  Patch: https://git.openjdk.org/jdk/pull/9087.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9087/head:pull/9087

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


Re: RFR: JDK-8288207: Enhance MalformedURLException in Uri.parseCompat

2022-06-13 Thread Matthias Baesken
On Mon, 13 Jun 2022 14:29:44 GMT, Jaikiran Pai  wrote:

> > Hi Daniel, should we maybe better print something like "check for not 
> > allowed characters" in the exception ? Do you have an easy and cheap way in 
> > mind to the get the unsupported character (in this case "_") to add it to 
> > the output ? Would maybe be more helpful than the proposed host:port and 
> > better regarding security concerns.
> 
> Hello Matthias, the current error message is:
> 
> > java.net.MalformedURLException: unsupported authority: ad_jbs.ttt.net:389
> 
> Are you suggesting that the error message should include some additional 
> wording which states why the authority isn't supported (in this case because 
> of the presence of that `_` character)?

Yes , this is what I meant.  Ideally (and if it is not much overhead/easy to 
get) we show the 'bad'  character in the message.  Otherwise just some 
additional wording.

-

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


Re: RFR: JDK-8288207: Enhance MalformedURLException in Uri.parseCompat

2022-06-13 Thread Jaikiran Pai
On Mon, 13 Jun 2022 07:26:32 GMT, Matthias Baesken  wrote:

> Hi Daniel, should we maybe better print something like "check for not allowed 
> characters" in the exception ? Do you have an easy and cheap way in mind to 
> the get the unsupported character (in this case "_") to add it to the output 
> ? Would maybe be more helpful than the proposed host:port and better 
> regarding security concerns.

Hello Matthias, the current error message is:

> java.net.MalformedURLException: unsupported authority: ad_jbs.ttt.net:389

Are you suggesting that the error message should include some additional 
wording which states why the authority isn't supported (in this case because of 
the presence of that `_` character)?

-

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


RFR: 8286176: Add JNI_VERSION_19 to jni.h and JNI spec

2022-06-13 Thread Alan Bateman
JNI is updated in Java 19 so we need to define JNI_VERSION_19 and change 
GetVersion to return this version.

test/hotspot/jtreg/native_sanity/JniVersion.java is updated to check that 
JNI_VERSION_19 is returned. The native library in the JMX agent, and several 
tests, define JNI_OnLoad that return JNI_VERSION_10 as the JNI version needed. 
These are updated to return JNI_VERSION_19 although these updates aren't 
strictly needed.

-

Commit messages:
 - Initial commit

Changes: https://git.openjdk.org/jdk19/pull/7/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk19=7=00
  Issue: https://bugs.openjdk.org/browse/JDK-8286176
  Stats: 16 lines in 10 files changed: 2 ins; 0 del; 14 mod
  Patch: https://git.openjdk.org/jdk19/pull/7.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/7/head:pull/7

PR: https://git.openjdk.org/jdk19/pull/7


Re: [Very fast List/Deque to java.util?]

2022-06-13 Thread John Hendrikx

I took a look.

I found a few results odd:

|com.github.coderodde.util.IndexedLinkedList.addLast in (ms): 8 
java.util.LinkedList.addLast in (ms): 2 java.util.ArrayList.addLast in 
(ms): 157 org.apache.commons.collections4.list.TreeList.addLast in (ms): 38|


Basically, ArrayList's performance (which should be O(1) in this case) 
is surprising. Looking at the benchmark, it is calling 
ArrayList#add(int, E) -- this method is not special cased for adding at 
the end of the list (it will do a range check and call System#arrayCopy 
in all cases).


|com.github.coderodde.util.IndexedLinkedList.stream() in (ms): 1 
java.util.LinkedList.stream() in (ms): 20 java.util.ArrayList.stream() 
in (ms): 16 org.apache.commons.collections4.list.TreeList.stream() in 
(ms): 15|


This test isn't only measuring streaming (iteration?) but also 
re-inserting all elements back into a List 
(collect(Collectors.toList()). What I find odd is that the 
IndexedLinkedList would perform much better here than ArrayList, given 
that the time is probably dominated by the final collect, and not the 
iteration itself.  Is ArrayList#stream poorly optimized or is something 
else going on?


A pure iteration test would be interesting to see.

I also ran the benchmark on my machine. The benchmark on Github doesn't 
mention with what parameters it is run, and so when I run it I get quite 
different results. The committed version of the benchmark seems to use 
collections with a max size of 20 elements. The total time when I run 
the benchmark favors TreeList more than any other:


--- Total time elapsed ---
com.github.coderodde.util.IndexedLinkedList in (ms): 207
java.util.LinkedList in (ms): 4248
java.util.ArrayList in (ms): 1799
org.apache.commons.collections4.list.TreeList in (ms): 159

That said, I think there might be a place for a new list implementation 
in the JDK.  One use case I've had in the past was for a list that is 
always sorted but also allows index based access when displaying a 
window into the list. A TreeMap satisfies the sorting criteria but 
doesn't offer index access, while a plain ArrayList doesn't lend itself 
well for doing sorted inserts/removals (locating the correct location is 
trivial enough, but the actual insert or removal results in a 
potentially large copy).  Apache's TreeList is fairly good at this use case.


--John


On 13/06/2022 09:56, Rodion Efremov wrote:

Hello,

I have this List/Deque implementation [1] that (in a versatile benchmark)
runs much faster than ArrayList/LinkedList. Under mild assumptions, it
accesses an element in O(sqrt(N)) time.

Now, if all we want to do is to add at the tail and read via get(int
index), you are better of using java.util.ArrayList. If you wish to iterate
a list removing some elements, the way to go is to use java.util.LinkedList.

  However,, if you deal with larger lists via many different operations
(addFirst/addLast/add(int, E)/get(int)/remove(int)/ettc. my
IndexedLinkedLiist will outperform both of them gracefully.

So, what do you think? Does it deserve to become a class candidate for
java.util?

[1]https://github.com/coderodde/IndexedLinkedList


Re: RFR: JDK-8288094: cleanup old _MSC_VER handling [v3]

2022-06-13 Thread Matthias Baesken
> We still handle at a number of places ancient historic _MSC_VER versions of 
> Visual Studio releases e.g. pre VS2013 (VS2013 has _MSC_VER 1800).
> This should be cleaned up, as long as it is not 3rd party code that we don't 
> want to adjust.
> 
> Currently still supported ("valid") VS version are 2017+, see 
> https://github.com/openjdk/jdk/blob/master/make/autoconf/toolchain_microsoft.m4
>  .
> VALID_VS_VERSIONS="2019 2017 2022"
> Even with adjusted toolchain m4 files, something older than VS2013 (also 
> probably older than VS2015) would not be able to build jdk anymore.

Matthias Baesken has updated the pull request incrementally with one additional 
commit since the last revision:

  remove VS2005 comment in adlc

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9105/files
  - new: https://git.openjdk.org/jdk/pull/9105/files/e97c8329..97659965

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=9105=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9105=01-02

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

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


Re: RFR: JDK-8288094: cleanup old _MSC_VER handling [v2]

2022-06-13 Thread Matthias Baesken
On Mon, 13 Jun 2022 10:33:24 GMT, Andrey Turbanov  wrote:

>> Matthias Baesken has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   use round directly
>
> src/hotspot/share/adlc/main.cpp line 491:
> 
>> 489: }
>> 490: 
>> 491: // VS2005 has its own definition, identical to this one.
> 
> Let's adjust comment too. No need to mention VS2005.

Thanks for the hint, I removed the VS2005 related comment.

-

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


Re: RFR: JDK-8288094: cleanup old _MSC_VER handling [v2]

2022-06-13 Thread Andrey Turbanov
On Mon, 13 Jun 2022 07:24:52 GMT, Matthias Baesken  wrote:

>> We still handle at a number of places ancient historic _MSC_VER versions of 
>> Visual Studio releases e.g. pre VS2013 (VS2013 has _MSC_VER 1800).
>> This should be cleaned up, as long as it is not 3rd party code that we don't 
>> want to adjust.
>> 
>> Currently still supported ("valid") VS version are 2017+, see 
>> https://github.com/openjdk/jdk/blob/master/make/autoconf/toolchain_microsoft.m4
>>  .
>> VALID_VS_VERSIONS="2019 2017 2022"
>> Even with adjusted toolchain m4 files, something older than VS2013 (also 
>> probably older than VS2015) would not be able to build jdk anymore.
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   use round directly

src/hotspot/share/adlc/main.cpp line 491:

> 489: }
> 490: 
> 491: // VS2005 has its own definition, identical to this one.

Let's adjust comment too. No need to mention VS2005.

-

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


Re: RFR: JDK-8288094: cleanup old _MSC_VER handling [v2]

2022-06-13 Thread Martin Doerr
On Mon, 13 Jun 2022 07:24:52 GMT, Matthias Baesken  wrote:

>> We still handle at a number of places ancient historic _MSC_VER versions of 
>> Visual Studio releases e.g. pre VS2013 (VS2013 has _MSC_VER 1800).
>> This should be cleaned up, as long as it is not 3rd party code that we don't 
>> want to adjust.
>> 
>> Currently still supported ("valid") VS version are 2017+, see 
>> https://github.com/openjdk/jdk/blob/master/make/autoconf/toolchain_microsoft.m4
>>  .
>> VALID_VS_VERSIONS="2019 2017 2022"
>> Even with adjusted toolchain m4 files, something older than VS2013 (also 
>> probably older than VS2015) would not be able to build jdk anymore.
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   use round directly

Nice!

-

Marked as reviewed by mdoerr (Reviewer).

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


Re: RFR: 8287007: [cgroups] Consistently use stringStream throughout parsing code [v3]

2022-06-13 Thread Severin Gehwolf
On Wed, 8 Jun 2022 12:09:27 GMT, Severin Gehwolf  wrote:

>> Please review this cleanup change in the cgroup subsystem which used to use 
>> hard-coded stack allocated
>> buffers for concatenating strings in memory. We can use `stringStream` 
>> instead which doesn't have the issue
>> of hard-coding maximum lengths (and related checks) and makes the code, 
>> thus, easier to read.
>> 
>> While at it, I've written basic `gtests` verifying current behaviour (and 
>> the same for the JDK code). From
>> a functionality point of view this is a no-op (modulo the one bug in the 
>> substring case which seems to be
>> there since day one). I'd prefer if we could defer any change in 
>> functionality to a separate bug as this is
>> meant to only use stringStream throughout the cgroups code.
>> 
>> Testing:
>> - [x] Container tests on Linux x86_64 cgroups v1 and cgroups v2
>> - [x] Added tests, which I've verified also pass before the stringStream 
>> change
>> 
>> Thoughts?
>
> Severin Gehwolf has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Remove fix for substring match

@erikj79 Do you know why the SKARA bots didn't move this to `ready` by now? Is 
there an issue with the bots? Thanks!

-

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


Re: RFR: 8285519: Change usages of TimeUnit.convert to TimeUnit.toXXX [v2]

2022-06-13 Thread Andrey Turbanov
On Sun, 15 May 2022 10:31:20 GMT, Andrey Turbanov  wrote:

>> TimeUnit.toMillis/toNanos/toMicros/toSeconds is shorter and a bit faster.
>> Compared via JMH benchmark: 150ns -> 125ns/op
>> 
>> Benchamark adapted from 
>> http://cr.openjdk.java.net/~shade/8152083/TimeUnitBench.java
>> 
>> 
>> @Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
>> @Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
>> @Fork(1)
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class TimeUnitBench {
>> 
>> static final int SIZE = TimeUnit.values().length * 10;
>> 
>> @Param({"0", "1", "2", "3", "4", "5", "6", "-1"})
>> int bias;
>> 
>> TimeUnit[] mod;
>> 
>> @Setup
>> public void setup() {
>> mod = new TimeUnit[SIZE];
>> 
>> TimeUnit[] vals = TimeUnit.values();
>> for (int c = 0; c < SIZE; c++) {
>> if (bias == -1) {
>> // megamorphic
>> mod[c] = vals[c % vals.length];
>> } else {
>> mod[c] = vals[bias];
>> }
>> }
>> 
>> Random r = new Random();
>> for (int c = 0; c < 1; c++) {
>> int i = r.nextInt();
>> for (int o = 0; o < vals.length; o++) {
>> if (vals[o].toNanos(i) != TimeUnit.NANOSECONDS.convert(i, 
>> vals[o]))
>> throw new IllegalStateException("switch " + o);
>> }
>> }
>> }
>> 
>> @Benchmark
>> public void const_convert(Blackhole bh) {
>> for (TimeUnit tu : mod) {
>> bh.consume(do_convert(tu));
>> }
>> }
>> 
>> @Benchmark
>> public void value_convert(Blackhole bh) {
>> for (TimeUnit tu : mod) {
>> bh.consume(do_convert(tu, 1L));
>> }
>> }
>> 
>> @Benchmark
>> public void const_toNanos(Blackhole bh) {
>> for (TimeUnit tu : mod) {
>> bh.consume(do_toNanos(tu));
>> }
>> }
>> 
>> @Benchmark
>> public void value_toNanos(Blackhole bh) {
>> for (TimeUnit tu : mod) {
>> bh.consume(do_toNanos(tu, 1L));
>> }
>> }
>> 
>> @CompilerControl(CompilerControl.Mode.DONT_INLINE)
>> private long do_toNanos(TimeUnit tu) {
>> return tu.toNanos(1L);
>> }
>> 
>> @CompilerControl(CompilerControl.Mode.DONT_INLINE)
>> private long do_toNanos(TimeUnit tu, long value) {
>> return tu.toNanos(value);
>> }
>> 
>> @CompilerControl(CompilerControl.Mode.DONT_INLINE)
>> private long do_convert(TimeUnit tu) {
>> return TimeUnit.NANOSECONDS.convert(1L, tu);
>> }
>> 
>> @CompilerControl(CompilerControl.Mode.DONT_INLINE)
>> private long do_convert(TimeUnit tu, long value) {
>> return TimeUnit.NANOSECONDS.convert(value, tu);
>> }
>> }
>> 
>> Results:
>> 
>> Benchmark(bias)  Mode  CntScoreError  Units
>> TimeUnitBench.const_convert   0  avgt5  151,856 ± 28,595  ns/op
>> TimeUnitBench.const_convert   1  avgt5  150,720 ± 23,863  ns/op
>> TimeUnitBench.const_convert   2  avgt5  151,432 ± 23,184  ns/op
>> TimeUnitBench.const_convert   3  avgt5  150,959 ± 24,726  ns/op
>> TimeUnitBench.const_convert   4  avgt5  150,966 ± 25,280  ns/op
>> TimeUnitBench.const_convert   5  avgt5  150,976 ± 25,542  ns/op
>> TimeUnitBench.const_convert   6  avgt5  151,118 ± 25,254  ns/op
>> TimeUnitBench.const_convert  -1  avgt5  152,673 ± 29,861  ns/op
>> TimeUnitBench.const_toNanos   0  avgt5  121,296 ± 25,236  ns/op
>> TimeUnitBench.const_toNanos   1  avgt5  121,707 ± 25,364  ns/op
>> TimeUnitBench.const_toNanos   2  avgt5  121,736 ± 25,726  ns/op
>> TimeUnitBench.const_toNanos   3  avgt5  121,822 ± 25,491  ns/op
>> TimeUnitBench.const_toNanos   4  avgt5  121,867 ± 26,090  ns/op
>> TimeUnitBench.const_toNanos   5  avgt5  121,927 ± 25,611  ns/op
>> TimeUnitBench.const_toNanos   6  avgt5  121,821 ± 25,843  ns/op
>> TimeUnitBench.const_toNanos  -1  avgt5  121,923 ± 25,206  ns/op
>> TimeUnitBench.value_convert   0  avgt5  150,525 ± 25,439  ns/op
>> TimeUnitBench.value_convert   1  avgt5  151,098 ± 24,024  ns/op
>> TimeUnitBench.value_convert   2  avgt5  151,259 ± 25,381  ns/op
>> TimeUnitBench.value_convert   3  avgt5  151,030 ± 25,548  ns/op
>> TimeUnitBench.value_convert   4  avgt5  151,290 ± 25,998  ns/op
>> TimeUnitBench.value_convert   5  avgt5  151,006 ± 25,448  ns/op
>> TimeUnitBench.value_convert   6  avgt5  150,945 ± 25,314  ns/op
>> TimeUnitBench.value_convert  -1  avgt5  151,186 ± 25,814  ns/op
>> TimeUnitBench.value_toNanos   0  avgt5  121,556 ± 25,745  ns/op
>> TimeUnitBench.value_toNanos   1  avgt5  123,410 ± 22,323  ns/op
>> TimeUnitBench.value_toNanos   2  avgt5  125,452 

[Very fast List/Deque to java.util?]

2022-06-13 Thread Rodion Efremov
Hello,

I have this List/Deque implementation [1] that (in a versatile benchmark)
runs much faster than ArrayList/LinkedList. Under mild assumptions, it
accesses an element in O(sqrt(N)) time.

Now, if all we want to do is to add at the tail and read via get(int
index), you are better of using java.util.ArrayList. If you wish to iterate
a list removing some elements, the way to go is to use java.util.LinkedList.

 However,, if you deal with larger lists via many different operations
(addFirst/addLast/add(int, E)/get(int)/remove(int)/ettc. my
IndexedLinkedLiist will outperform both of them gracefully.

So, what do you think? Does it deserve to become a class candidate for
java.util?

[1] https://github.com/coderodde/IndexedLinkedList


Re: RFR: JDK-8288207: Enhance MalformedURLException in Uri.parseCompat

2022-06-13 Thread Matthias Baesken
On Fri, 10 Jun 2022 14:19:27 GMT, Daniel Fuchs  wrote:

> I might question whether the added "null:-1" information is really helpful, 
> or just as confusing however.

Hi Daniel, should we maybe better print something like "check for not allowed 
characters" in the exception ? Do you have an easy and cheap way in mind to the 
get the unsupported character (in this case "_") to add it to the output ? 
Would maybe be more helpful than the proposed host:port and better regarding 
security concerns.

-

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


Re: RFR: JDK-8288094: cleanup old _MSC_VER handling [v2]

2022-06-13 Thread Matthias Baesken
On Fri, 10 Jun 2022 21:04:04 GMT, Phil Race  wrote:

>> Matthias Baesken has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   use round directly
>
> src/java.desktop/windows/native/libawt/windows/ThemeReader.cpp line 38:
> 
>> 36: #  define ROUND_TO_INT(num)((int) round(num))
>> 37: #else
>> 38: #  define ROUND_TO_INT(num)((int) floor((num) + 0.5))
> 
> If you look at when and why this was introduced (*), the "else" was not to 
> support some other compiler - it was to support the older MS compiler. So if 
> you don't want that, then the whole thing reduces to
> #define ROUND_TO_INT(num) ((int) round(num))
> 
> .. you could even go further and eliminate the macro altogether if it makes 
> sense - you'd have to look at the usages.
> 
> Same logic applies to the other files.
> 
> 
> (*) https://mail.openjdk.java.net/pipermail/awt-dev/2016-March/010889.html

Hi Phil, I simplified the code more and now use round directly.

-

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


Re: RFR: JDK-8288094: cleanup old _MSC_VER handling [v2]

2022-06-13 Thread Matthias Baesken
> We still handle at a number of places ancient historic _MSC_VER versions of 
> Visual Studio releases e.g. pre VS2013 (VS2013 has _MSC_VER 1800).
> This should be cleaned up, as long as it is not 3rd party code that we don't 
> want to adjust.
> 
> Currently still supported ("valid") VS version are 2017+, see 
> https://github.com/openjdk/jdk/blob/master/make/autoconf/toolchain_microsoft.m4
>  .
> VALID_VS_VERSIONS="2019 2017 2022"
> Even with adjusted toolchain m4 files, something older than VS2013 (also 
> probably older than VS2015) would not be able to build jdk anymore.

Matthias Baesken has updated the pull request incrementally with one additional 
commit since the last revision:

  use round directly

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9105/files
  - new: https://git.openjdk.org/jdk/pull/9105/files/ee8dc996..e97c8329

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=9105=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9105=00-01

  Stats: 21 lines in 3 files changed: 0 ins; 17 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/9105.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9105/head:pull/9105

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