Re: RFR: 8324718: Add a static function to java.util.Objects to simplify object equality checks [v6]
> Adding a function to Objects in order to facilitate equality checking and > enhance readability. You simply specify the 2 objects that you want to check > for equality, and then provide the functions which will be used to provide > the values that we will check for equality. David Alayachew has updated the pull request incrementally with one additional commit since the last revision: Rather than reiterating the precondition, let's explain why the method failed - Changes: - all: https://git.openjdk.org/jdk/pull/17603/files - new: https://git.openjdk.org/jdk/pull/17603/files/6d424c7a..5a578015 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17603=05 - incr: https://webrevs.openjdk.org/?repo=jdk=17603=04-05 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17603.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17603/head:pull/17603 PR: https://git.openjdk.org/jdk/pull/17603
Re: RFR: 8324718: Add a static function to java.util.Objects to simplify object equality checks [v5]
> Adding a function to Objects in order to facilitate equality checking and > enhance readability. You simply specify the 2 objects that you want to check > for equality, and then provide the functions which will be used to provide > the values that we will check for equality. David Alayachew has updated the pull request incrementally with one additional commit since the last revision: I'm sure developers would appreciate a more explicit error message - Changes: - all: https://git.openjdk.org/jdk/pull/17603/files - new: https://git.openjdk.org/jdk/pull/17603/files/57cb7093..6d424c7a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17603=04 - incr: https://webrevs.openjdk.org/?repo=jdk=17603=03-04 Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/17603.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17603/head:pull/17603 PR: https://git.openjdk.org/jdk/pull/17603
Re: RFR: 8324718: Add a static function to java.util.Objects to simplify object equality checks [v4]
> Adding a function to Objects in order to facilitate equality checking and > enhance readability. You simply specify the 2 objects that you want to check > for equality, and then provide the functions which will be used to provide > the values that we will check for equality. David Alayachew has updated the pull request incrementally with one additional commit since the last revision: Correcting JavaDoc - Changes: - all: https://git.openjdk.org/jdk/pull/17603/files - new: https://git.openjdk.org/jdk/pull/17603/files/dc375429..57cb7093 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17603=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17603=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17603.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17603/head:pull/17603 PR: https://git.openjdk.org/jdk/pull/17603
Re: RFR: 8324718: Add a static function to java.util.Objects to simplify object equality checks [v3]
> Adding a function to Objects in order to facilitate equality checking and > enhance readability. You simply specify the 2 objects that you want to check > for equality, and then provide the functions which will be used to provide > the values that we will check for equality. David Alayachew has updated the pull request incrementally with one additional commit since the last revision: Implied is ok, but explicit is better - Changes: - all: https://git.openjdk.org/jdk/pull/17603/files - new: https://git.openjdk.org/jdk/pull/17603/files/e328c4eb..dc375429 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17603=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17603=01-02 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17603.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17603/head:pull/17603 PR: https://git.openjdk.org/jdk/pull/17603
Re: RFR: 8324718: Add a static function to java.util.Objects to simplify object equality checks [v2]
On Sat, 27 Jan 2024 04:21:02 GMT, David Alayachew wrote: >> Adding a function to Objects in order to facilitate equality checking and >> enhance readability. You simply specify the 2 objects that you want to check >> for equality, and then provide the functions which will be used to provide >> the values that we will check for equality. > > David Alayachew has updated the pull request incrementally with one > additional commit since the last revision: > > Aligning the documentation across examples > > I didn't really want to use a record since that might distract from the > example for those unfamiliar with a record. But, it makes things simpler, and > I want to keep my examples consistent, especially if the examples use the > same name. Adding a screenshot to demonstrate what I see.  Clicking NEW on the top left says that I don't have permissions. - PR Comment: https://git.openjdk.org/jdk/pull/17603#issuecomment-1913033434
Re: RFR: 8324718: Add a static function to java.util.Objects to simplify object equality checks [v2]
On Sat, 27 Jan 2024 04:21:02 GMT, David Alayachew wrote: >> Adding a function to Objects in order to facilitate equality checking and >> enhance readability. You simply specify the 2 objects that you want to check >> for equality, and then provide the functions which will be used to provide >> the values that we will check for equality. > > David Alayachew has updated the pull request incrementally with one > additional commit since the last revision: > > Aligning the documentation across examples > > I didn't really want to use a record since that might distract from the > example for those unfamiliar with a record. But, it makes things simpler, and > I want to keep my examples consistent, especially if the examples use the > same name. @AlanBateman ty vm for helping! I took a look at the CSR FAQ, and found this. > Q: How do I create a CSR ? > A: Do not directly create a CSR from the Create Menu. JIRA will let you do > this right up until the moment you try to save it and find your typing was in > vain. Instead you should go to the target bug, select "More", and from the drop down menu select "Create CSR". This is required to properly associate the CSR with the main bug, just as is done for backports. I don't see this on my bug page. Am I looking in the wrong place? Or do I lack the permissions? -- https://bugs.openjdk.org/browse/JDK-8324718 - PR Comment: https://git.openjdk.org/jdk/pull/17603#issuecomment-1913031496
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 26 Jan 2024 12:27:02 GMT, Andrew Dinn wrote: > Luckily, you and your customers are not obliged to use the JPMS They are obliged to deal with it, and so am I as a tool maintainer. Just look the the approaches mentioned here. They all are in the category which in German we would call "von hinten durch die Brust ins Auge". That literally translates as "(a shot) from behind through the chest into the eye". Think Kennedy and "magic bullet". It means that something is unnecessarily complicated. But it is not of any developer's own choosing, but because the Java API and runtime environment requires them to jump through hoops. That is exactly what I meant before. Bytecode transformation should not be rocket science, but it progressively is developing in that direction. I have seen what Byte Buddy does to get access to an Unsafe instance, thought about what @JornVernee just suggested and have yet to look into the ByteMan source code. This all sounds pretty much like black magic and a maintenance nightmare to me. A language ought to provide means to be productive and maybe offer some (opt-in) guard railing, but not be a corset so tight that I cannot breathe anymore. I need something that supports me without strangling me. - PR Comment: https://git.openjdk.org/jdk/pull/3546#issuecomment-1912999685
Re: RFR: 8324718: Add a static function to java.util.Objects to simplify object equality checks [v2]
> Adding a function to Objects in order to facilitate equality checking and > enhance readability. You simply specify the 2 objects that you want to check > for equality, and then provide the functions which will be used to provide > the values that we will check for equality. David Alayachew has updated the pull request incrementally with one additional commit since the last revision: Aligning the documentation across examples I didn't really want to use a record since that might distract from the example for those unfamiliar with a record. But, it makes things simpler, and I want to keep my examples consistent, especially if the examples use the same name. - Changes: - all: https://git.openjdk.org/jdk/pull/17603/files - new: https://git.openjdk.org/jdk/pull/17603/files/db5c7b43..e328c4eb Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17603=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17603=00-01 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/17603.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17603/head:pull/17603 PR: https://git.openjdk.org/jdk/pull/17603
RFR: 8324718: Add a static function to java.util.Objects to simplify object equality checks
Adding a function to Objects in order to facilitate equality checking and enhance readability. You simply specify the 2 objects that you want to check for equality, and then provide the functions which will be used to provide the values that we will check for equality. - Commit messages: - Fixing whitespace - Correcting JavaDoc - Fixing a bug in the provided code example. - Adding better documentation for more corner cases - Adding helpful documentation to cover corner cases - paren - Using the @snippet feature - Merge branch 'openjdk:master' into patch-1 - Add new method equalsBy to java.util.Objects Changes: https://git.openjdk.org/jdk/pull/17603/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17603=00 Issue: https://bugs.openjdk.org/browse/JDK-8324718 Stats: 102 lines in 1 file changed: 101 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17603.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17603/head:pull/17603 PR: https://git.openjdk.org/jdk/pull/17603
Re: RFR: 8296244: Alternate implementation of user-based authorization Subject APIs that doesn’t depend on Security Manager APIs
On Wed, 17 Jan 2024 23:41:53 GMT, Weijun Wang wrote:
> This code change adds an alternative implementation of user-based
> authorization `Subject` APIs that doesn't depend on Security Manager APIs.
> Depending on if the Security Manager is allowed, the methods store the
> current subject differently. See the spec change in the `Subject.java` file
> for details. When the Security Manager APIs are finally removed in a future
> release, this new implementation will be only implementation for these
> methods.
>
> One major change in the new implementation is that `Subject.getSubject`
> always throws an `UnsupportedOperationException` since it has an
> `AccessControlContext` argument but the current subject is no longer
> associated with an `AccessControlContext` object.
>
> Now it's the time to migrate from the `getSubject` and `doAs` methods to
> `current` and `callAs`. If the user application is simply calling
> `getSubject(AccessController.getContext())`, then switching to `current()`
> would work. If the `AccessControlContext` argument is retrieved from an
> earlier `getContext()` call and the associated subject might be different
> from that of the current `AccessControlContext`, then instead of storing the
> previous `AccessControlContext` object and passing it into `getSubject` to
> get the "previous" subject, the application should store the `current()`
> return value directly.
src/java.base/share/classes/javax/security/auth/Subject.java line 112:
> 110: *
> 111: * These methods behave differently
> depending on
> 112: * whether a security manager is allowed or disallowed:
"is allowed or disallowed" but the detail presents them in the reverse order. I
think it would be easier to read if the allowed case went first, this goes for
all the methods. I understand that disallow is the default but I think a bit
easier to present in that order.
src/java.base/share/classes/javax/security/auth/Subject.java line 298:
> 296: * {@code AccessControlContext}. When a security manager is
> 297: * not allowed, this method is not
> supported
> 298: * and throws an {@code UnsupportedOperationException}.
I think it might be better to say "This method is intended to be used with a
security manager. It throws UOE if a security manager is not allowed".
src/java.base/share/classes/javax/security/auth/Subject.java line 379:
> 377: * current subject binds to the period of the execution of the
> current
> 378: * thread. Otherwise, it is associated with the current
> 379: * {@code AccessControlContext}.
What would you think of "If a security manager is allowed, this method is
equivalent to calling getSubject with the current ACC. If a security manager is
not allowed, this returns the Subject bound to the current Thread."
src/java.base/share/classes/javax/security/auth/Subject.java line 411:
> 409: * Finally, this method invokes {@code
> AccessController.doPrivileged},
> 410: * passing it the provided {@code PrivilegedAction},
> 411: * as well as the newly constructed {@code AccessControlContext}.
I think this method would be easier to present if the allowed and not-allowed
cases were in separate paragraphs. The reason is that the SM allowed case has a
lot more test. For the not allowed case then it would be clearer to say that
the calls the value returning function with the current Thread bound to the
given Subject.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1467824941
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1467826752
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1467829318
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1467831325
RFR: 8296244: Alternate implementation of user-based authorization Subject APIs that doesn’t depend on Security Manager APIs
This code change adds an alternative implementation of user-based authorization `Subject` APIs that doesn't depend on Security Manager APIs. Depending on if the Security Manager is allowed, the methods store the current subject differently. See the spec change in the `Subject.java` file for details. When the Security Manager APIs are finally removed in a future release, this new implementation will be only implementation for these methods. One major change in the new implementation is that `Subject.getSubject` always throws an `UnsupportedOperationException` since it has an `AccessControlContext` argument but the current subject is no longer associated with an `AccessControlContext` object. Now it's the time to migrate from the `getSubject` and `doAs` methods to `current` and `callAs`. If the user application is simply calling `getSubject(AccessController.getContext())`, then switching to `current()` would work. If the `AccessControlContext` argument is retrieved from an earlier `getContext()` call and the associated subject might be different from that of the current `AccessControlContext`, then instead of storing the previous `AccessControlContext` object and passing it into `getSubject` to get the "previous" subject, the application should store the `current()` return value directly. - Commit messages: - remove exe bits - remove x bit - the patch Changes: https://git.openjdk.org/jdk/pull/17472/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17472=00 Issue: https://bugs.openjdk.org/browse/JDK-8296244 Stats: 620 lines in 14 files changed: 464 ins; 52 del; 104 mod Patch: https://git.openjdk.org/jdk/pull/17472.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17472/head:pull/17472 PR: https://git.openjdk.org/jdk/pull/17472
Re: Integrated: 8324786: validate-source fails after JDK-8042981
On Fri, 26 Jan 2024 21:52:37 GMT, Joe Darcy wrote: >> A trivial fix for validate-source. > > Marked as reviewed by darcy (Reviewer). > @jddarcy - Thanks for the lightning fast review! > > /integrate auto Sorry for missing that before pushing. - PR Comment: https://git.openjdk.org/jdk/pull/17599#issuecomment-1912746596
Re: Integrated: 8324786: validate-source fails after JDK-8042981
On Fri, 26 Jan 2024 21:52:37 GMT, Joe Darcy wrote: >> A trivial fix for validate-source. > > Marked as reviewed by darcy (Reviewer). @jddarcy - Thanks for the lightning fast review! - PR Comment: https://git.openjdk.org/jdk/pull/17599#issuecomment-1912744650
Re: Integrated: 8324786: validate-source fails after JDK-8042981
On Fri, 26 Jan 2024 21:51:04 GMT, Daniel D. Daugherty wrote: > A trivial fix for validate-source. Marked as reviewed by darcy (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17599#pullrequestreview-1846604299
Integrated: 8324786: validate-source fails after JDK-8042981
On Fri, 26 Jan 2024 21:51:04 GMT, Daniel D. Daugherty wrote: > A trivial fix for validate-source. This pull request has now been integrated. Changeset: 70f4a4e1 Author:Daniel D. Daugherty URL: https://git.openjdk.org/jdk/commit/70f4a4e18e257110f45565ba0d708f1fa48aed76 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8324786: validate-source fails after JDK-8042981 Reviewed-by: darcy - PR: https://git.openjdk.org/jdk/pull/17599
Integrated: 8324786: validate-source fails after JDK-8042981
A trivial fix for validate-source. - Commit messages: - 8324786: validate-source fails after JDK-8042981 Changes: https://git.openjdk.org/jdk/pull/17599/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17599=00 Issue: https://bugs.openjdk.org/browse/JDK-8324786 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17599.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17599/head:pull/17599 PR: https://git.openjdk.org/jdk/pull/17599
Re: RFR: 8323699: MessageFormat.toPattern() generates non-equivalent MessageFormat pattern [v5]
On Fri, 26 Jan 2024 21:28:47 GMT, Justin Lu wrote:
>>> The "can be used to create" seems conditional.
>>
>> It is conditional - in the sense that you don't _have_ to use it to create a
>> new instance of `MessageFormat`. You can also use it for something else, in
>> other words.
>>
>> But I also understand how it comes across as a bit wishy-washy...
>>
>> Hmm, what do you think about this wording?
>>
>>
>> @implSpec The implementation in {@link MessageFormat} returns a string that,
>> when passed to the {@link MessageFormat(String)} constructor, produces an
>> instance that is semantically equivalent to this instance.
>
> Not sure which wording will ultimately be used, but if the wording ends up
> including the constructor, it's probably worth mentioning the `applyPattern`
> method as well.
Good point... maybe this?
@implSpec The implementation in {@link MessageFormat} returns a string that,
when passed
to a {@code MessageFormat()} constructor or {@link #applyPattern
applyPattern()}, produces
an instance that is semantically equivalent to this instance.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1468170808
Re: RFR: 8323699: MessageFormat.toPattern() generates non-equivalent MessageFormat pattern [v5]
On Fri, 26 Jan 2024 21:16:47 GMT, Archie Cobbs wrote:
>> src/java.base/share/classes/java/text/MessageFormat.java line 558:
>>
>>> 556: * @implSpec The implementation in {@link MessageFormat} returns a
>>> string
>>> 557: * that can be used to create a new instance that is semantically
>>> equivalent
>>> 558: * to this instance.
>>
>> The "can be used to create" seems conditional. Perhaps it can be worded as
>> an assertion that can be tested.
>> Suggestion:
>>
>> * @implSpec A new MessageFormat created from the string returned from
>> the implementation of
>> * `MessageFormat.toPattern()` is semantically equivalent to this
>> instance.
>
>> The "can be used to create" seems conditional.
>
> It is conditional - in the sense that you don't _have_ to use it to create a
> new instance of `MessageFormat`. You can also use it for something else, in
> other words.
>
> But I also understand how it comes across as a bit wishy-washy...
>
> Hmm, what do you think about this wording?
>
>
> @implSpec The implementation in {@link MessageFormat} returns a string that,
> when passed to the {@link MessageFormat(String)} constructor, produces an
> instance that is semantically equivalent to this instance.
Not sure which wording will ultimately be used, but if the wording ends up
including the constructor, it's probably worth mentioning the `applyPattern`
method as well.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1468163934
Re: RFR: 8323699: MessageFormat.toPattern() generates non-equivalent MessageFormat pattern [v5]
On Fri, 26 Jan 2024 20:30:42 GMT, Roger Riggs wrote:
> The "can be used to create" seems conditional.
It is conditional - in the sense that you don't _have_ to use it to create a
new instance of `MessageFormat`. You can also use it for something else, in
other words.
But I also understand how it comes across as a bit wishy-washy...
Hmm, what do you think about this wording?
@implSpec The implementation in {@link MessageFormat} returns a string that,
when passed to the {@link MessageFormat(String)} constructor, produces an
instance that is semantically equivalent to this instance.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1468155069
Re: RFR: 8324681: Replace NULL with nullptr in HotSpot jtreg test native code files [v3]
> This mechanically replaces NULL with nullptr in hpp/cpp native files in test > native code. This didn't attempt to change NULL in comments or strings to > just null. If you run into this and it bothers you after this push, you can > change it in a smaller patch. I didn't see any when it was scrolling by to > make my script more complicated. > > Ran tier1-4 testing. Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision: Fix nullptr only contained in strings. - Changes: - all: https://git.openjdk.org/jdk/pull/17593/files - new: https://git.openjdk.org/jdk/pull/17593/files/e15a3a0b..33786c7d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17593=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17593=01-02 Stats: 19 lines in 3 files changed: 0 ins; 0 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/17593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17593/head:pull/17593 PR: https://git.openjdk.org/jdk/pull/17593
Re: RFR: 8323699: MessageFormat.toPattern() generates non-equivalent MessageFormat pattern [v5]
On Thu, 25 Jan 2024 21:38:54 GMT, Archie Cobbs wrote:
>> Please consider this fix to ensure that going from `MessageFormat` to
>> pattern string via `toPattern()` and then back via `new MessageFormat()`
>> results in a format that is equivalent to the original.
>>
>> The quoting and escaping rules for `MessageFormat` pattern strings are
>> really tricky. I admit not completely understanding them. At a high level,
>> they work like this: The normal way one would "nest" strings containing
>> special characters is with straightforward recursive escaping like with the
>> `bash` command line. For example, if you want to echo `a "quoted string"
>> example` then you enter `echo "a "quoted string" example"`. With this scheme
>> it's always the "outer" layer's job to (un)escape special characters as
>> needed. That is, the echo command never sees the backslash characters.
>>
>> In contrast, with `MessageFormat` and friends, nested subformat pattern
>> strings are always provided "pre-escaped". So to build an "outer" string
>> (e.g., for `ChoiceFormat`) the "inner" subformat pattern strings are more or
>> less just concatenated, and then only the `ChoiceFormat` option separator
>> characters (e.g., `<`, `#`, `|`, etc.) are escaped.
>>
>> The "pre-escape" escaping algorithm escapes `{` characters, because `{`
>> indicates the beginning of a format argument. However, it doesn't escape `}`
>> characters. This is OK because the format string parser treats any "extra"
>> closing braces (where "extra" means not matching an opening brace) as plain
>> characters.
>>
>> So far, so good... at least, until a format string containing an extra
>> closing brace is nested inside a larger format string, where the extra
>> closing brace, which was previously "extra", can now suddenly match an
>> opening brace in the outer pattern containing it, thus truncating it by
>> "stealing" the match from some subsequent closing brace.
>>
>> An example is the `MessageFormat` string `"{0,choice,0.0#option A:
>> {1}|1.0#option B: {1}'}'}"`. Note the second option format string has a
>> trailing closing brace in plain text. If you create a `MessageFormat` with
>> this string, you see a trailing `}` only with the second option.
>>
>> However, if you then invoke `toPattern()`, the result is
>> `"{0,choice,0.0#option A: {1}|1.0#option B: {1}}}"`. Oops, now because the
>> "extra" closing brace is no longer quoted, it matches the opening brace at
>> the beginning of the string, and the following closing brace, which was the
>> previous match, is now just plain text in the outer `MessageFormat` string.
>>
>> As a result, invoking `f.format(new ...
>
> Archie Cobbs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Change MessageFormat.toPattern() @implNote to @implSpec.
src/java.base/share/classes/java/text/MessageFormat.java line 558:
> 556: * @implSpec The implementation in {@link MessageFormat} returns a
> string
> 557: * that can be used to create a new instance that is semantically
> equivalent
> 558: * to this instance.
The "can be used to create" seems conditional. Perhaps it can be worded as an
assertion that can be tested.
Suggestion:
* @implSpec A new MessageFormat created from the string returned from the
implementation of
* `MessageFormat.toPattern()` is semantically equivalent to this instance.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1468118990
Re: RFR: 8303866: Allow ZipInputStream.readEnd to parse small Zip64 ZIP files [v15]
On Fri, 26 Jan 2024 14:34:39 GMT, Eirik Bjørsnøs wrote: > To help make progress here, I have relaxed the validation such that we now > check: > > * That the "streaming mode" bit 3 flag is set > * That at least one of the LOC's size fields are marked 0x. > * That the LOC extra field has a field with header ID 0x1 (Zip64) > > Any reading/validation of the contents of the Zip64 extra field has been > removed. > > @jaikiran Is this closer to what you'd like to see? Have not forgotten this, hope to get to it next week - PR Comment: https://git.openjdk.org/jdk/pull/12524#issuecomment-1912678510
Re: RFR: 8324681: Replace NULL with nullptr in HotSpot jtreg test native code files [v2]
> This mechanically replaces NULL with nullptr in hpp/cpp native files in test > native code. This didn't attempt to change NULL in comments or strings to > just null. If you run into this and it bothers you after this push, you can > change it in a smaller patch. I didn't see any when it was scrolling by to > make my script more complicated. > > Ran tier1-4 testing. Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision: Fix the comments to "null" - Changes: - all: https://git.openjdk.org/jdk/pull/17593/files - new: https://git.openjdk.org/jdk/pull/17593/files/079b8931..e15a3a0b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17593=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17593=00-01 Stats: 229 lines in 103 files changed: 0 ins; 0 del; 229 mod Patch: https://git.openjdk.org/jdk/pull/17593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17593/head:pull/17593 PR: https://git.openjdk.org/jdk/pull/17593
RFR: 8324681: Replace NULL with nullptr in HotSpot jtreg test native code files
This mechanically replaces NULL with nullptr in hpp/cpp native files in test native code. This didn't attempt to change NULL in comments or strings to just null. If you run into this and it bothers you after this push, you can change it in a smaller patch. I didn't see any when it was scrolling by to make my script more complicated. Ran tier1-4 testing. - Commit messages: - 8324681: Replace NULL with nullptr in HotSpot jtreg test native code files Changes: https://git.openjdk.org/jdk/pull/17593/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17593=00 Issue: https://bugs.openjdk.org/browse/JDK-8324681 Stats: 8196 lines in 750 files changed: 0 ins; 7 del; 8189 mod Patch: https://git.openjdk.org/jdk/pull/17593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17593/head:pull/17593 PR: https://git.openjdk.org/jdk/pull/17593
Re: RFR: 8323699: MessageFormat.toPattern() generates non-equivalent MessageFormat pattern [v5]
On Fri, 26 Jan 2024 18:58:49 GMT, Justin Lu wrote: > I wasn't sure if you were waiting to make additional changes or something > else, but you can go ahead and re-finalize the CSR (when you are ready), now > that it has been reviewed. Thanks - I wasn't sure about that. I've updated it now. - PR Comment: https://git.openjdk.org/jdk/pull/17416#issuecomment-1912570659
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v11]
On Fri, 26 Jan 2024 17:19:25 GMT, Srinivas Vamsi Parasa wrote: >> Hello Vamsi (@vamsi-parasa), >> >> Could you please run the benchmarking of new DQPS in your environment with >> AVX? >> >> Take all classes below and put them in the package >> org.openjdk.bench.java.util. >> ArraysSort class contains all tests for the new versions and ready to use. >> (it will run all tests in one execution). >> >> https://github.com/iaroslavski/sorting/blob/master/radixsort/ArraysSort.java >> https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_a15.java >> https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r20s.java >> https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r20p.java >> >> Many thanks, >> Vladimir > > Hi Vladimir (@iaroslavski), > > Please see the JMH data below. > > Thanks, > Vamsi > > Benchmark (builder) (size) Mode Cnt Score Error > Units > ArraysSort.Int.a15RANDOM 600 avgt4 7.096 ±0.081 > us/op > ArraysSort.Int.a15RANDOM 2000 avgt4 44.014 ±1.717 > us/op > ArraysSort.Int.a15RANDOM9 avgt44451.444 ± 71.168 > us/op > ArraysSort.Int.a15RANDOM 40 avgt4 22751.966 ± 683.597 > us/op > ArraysSort.Int.a15RANDOM 300 avgt4 190326.306 ± 8008.512 > us/op > ArraysSort.Int.a15 REPEATED 600 avgt4 1.044 ±0.016 > us/op > ArraysSort.Int.a15 REPEATED 2000 avgt4 2.272 ±0.287 > us/op > ArraysSort.Int.a15 REPEATED9 avgt4 412.331 ± 11.656 > us/op > ArraysSort.Int.a15 REPEATED 40 avgt41908.978 ± 30.241 > us/op > ArraysSort.Int.a15 REPEATED 300 avgt4 15163.443 ± 100.425 > us/op > ArraysSort.Int.a15 STAGGER 600 avgt4 1.055 ±0.057 > us/op > ArraysSort.Int.a15 STAGGER 2000 avgt4 3.408 ±0.096 > us/op > ArraysSort.Int.a15 STAGGER9 avgt4 149.220 ±4.022 > us/op > ArraysSort.Int.a15 STAGGER 40 avgt4 663.096 ± 30.252 > us/op > ArraysSort.Int.a15 STAGGER 300 avgt45206.890 ± 234.857 > us/op > ArraysSort.Int.a15 SHUFFLE 600 avgt4 4.611 ±0.118 > us/op > ArraysSort.Int.a15 SHUFFLE 2000 avgt4 17.955 ±0.356 > us/op > ArraysSort.Int.a15 SHUFFLE9 avgt41410.357 ± 41.128 > us/op > ArraysSort.Int.a15 SHUFFLE 40 avgt45739.311 ± 128.270 > us/op > ArraysSort.Int.a15 SHUFFLE 300 avgt4 41501.980 ± 829.443 > us/op > ArraysSort.Int.jdkRANDOM 600 avgt4 1.612 ±0.088 > us/op > ArraysSort.Int.jdkRANDOM 2000 avgt4 6.893 ±0.375 > us/op > ArraysSort.Int.jdkRANDOM9 avgt4 522.749 ± 19.386 > us/op > ArraysSort.Int.jdkRANDOM 40 avgt42424.204 ± 63.844 > us/op > ArraysSort.Int.jdkRANDOM 300 avgt4 21000.434 ± 801.315 > us/op > ArraysSort.Int.jdk REPEATED 600 avgt4 0.496 ±0.030 > us/op > ArraysSort.Int.jdk REPEATED 2000 avgt4 1.037 ±0.083 > us/op > ArraysSort.Int.jdk REPEATED9 avgt4 57.763 ±1.955 > us/op > ArraysSort.Int.jd... Thank you, Vamsi (@vamsi-parasa)! I will convert the data to more readable format. - PR Comment: https://git.openjdk.org/jdk/pull/13568#issuecomment-1912541236
Re: RFR: 8323699: MessageFormat.toPattern() generates non-equivalent MessageFormat pattern [v5]
On Thu, 25 Jan 2024 21:38:54 GMT, Archie Cobbs wrote:
>> Please consider this fix to ensure that going from `MessageFormat` to
>> pattern string via `toPattern()` and then back via `new MessageFormat()`
>> results in a format that is equivalent to the original.
>>
>> The quoting and escaping rules for `MessageFormat` pattern strings are
>> really tricky. I admit not completely understanding them. At a high level,
>> they work like this: The normal way one would "nest" strings containing
>> special characters is with straightforward recursive escaping like with the
>> `bash` command line. For example, if you want to echo `a "quoted string"
>> example` then you enter `echo "a "quoted string" example"`. With this scheme
>> it's always the "outer" layer's job to (un)escape special characters as
>> needed. That is, the echo command never sees the backslash characters.
>>
>> In contrast, with `MessageFormat` and friends, nested subformat pattern
>> strings are always provided "pre-escaped". So to build an "outer" string
>> (e.g., for `ChoiceFormat`) the "inner" subformat pattern strings are more or
>> less just concatenated, and then only the `ChoiceFormat` option separator
>> characters (e.g., `<`, `#`, `|`, etc.) are escaped.
>>
>> The "pre-escape" escaping algorithm escapes `{` characters, because `{`
>> indicates the beginning of a format argument. However, it doesn't escape `}`
>> characters. This is OK because the format string parser treats any "extra"
>> closing braces (where "extra" means not matching an opening brace) as plain
>> characters.
>>
>> So far, so good... at least, until a format string containing an extra
>> closing brace is nested inside a larger format string, where the extra
>> closing brace, which was previously "extra", can now suddenly match an
>> opening brace in the outer pattern containing it, thus truncating it by
>> "stealing" the match from some subsequent closing brace.
>>
>> An example is the `MessageFormat` string `"{0,choice,0.0#option A:
>> {1}|1.0#option B: {1}'}'}"`. Note the second option format string has a
>> trailing closing brace in plain text. If you create a `MessageFormat` with
>> this string, you see a trailing `}` only with the second option.
>>
>> However, if you then invoke `toPattern()`, the result is
>> `"{0,choice,0.0#option A: {1}|1.0#option B: {1}}}"`. Oops, now because the
>> "extra" closing brace is no longer quoted, it matches the opening brace at
>> the beginning of the string, and the following closing brace, which was the
>> previous match, is now just plain text in the outer `MessageFormat` string.
>>
>> As a result, invoking `f.format(new ...
>
> Archie Cobbs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Change MessageFormat.toPattern() @implNote to @implSpec.
Hi Archie,
I wasn't sure if you were waiting to make additional changes or something else,
but you can go ahead and re-finalize the CSR (when you are ready), now that it
has been reviewed.
-
PR Comment: https://git.openjdk.org/jdk/pull/17416#issuecomment-1912543193
Integrated: 8323835: Updating ASM to 9.6 for JDK 23
On Tue, 16 Jan 2024 21:18:55 GMT, Vicente Romero wrote: > Updating ASM to version 9.6, > > Thanks in advance for the reviews, > Vicente This pull request has now been integrated. Changeset: 91d8ea79 Author:Vicente Romero URL: https://git.openjdk.org/jdk/commit/91d8ea79d947aa7dad91d8ed550ed34a7d49d885 Stats: 1536 lines in 127 files changed: 1033 ins; 181 del; 322 mod 8323835: Updating ASM to 9.6 for JDK 23 Reviewed-by: mchung - PR: https://git.openjdk.org/jdk/pull/17453
Re: RFR: 8323835: Updating ASM to 9.6 for JDK 23
On Fri, 26 Jan 2024 01:36:47 GMT, Mandy Chung wrote: > Looks okay to me. I would rely on your testing for verification. I have run tier1-6 tests, they look OK - PR Comment: https://git.openjdk.org/jdk/pull/17453#issuecomment-1912506728
Re: RFR: 8303374: Implement JEP 455: Primitive Types in Patterns, instanceof, and switch (Preview) [v55]
> This is the proposed patch for Primitive types in patterns, instanceof, and > switch (Preview). > > Draft spec here: https://cr.openjdk.org/~abimpoudis/instanceof/latest/ Aggelos Biboudis has updated the pull request incrementally with one additional commit since the last revision: Remove redundant null check and introduce a const boolean for unconditionally exact pairs - Changes: - all: https://git.openjdk.org/jdk/pull/15638/files - new: https://git.openjdk.org/jdk/pull/15638/files/854c20a1..e466cfba Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15638=54 - incr: https://webrevs.openjdk.org/?repo=jdk=15638=53-54 Stats: 12 lines in 1 file changed: 0 ins; 9 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/15638.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15638/head:pull/15638 PR: https://git.openjdk.org/jdk/pull/15638
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v12]
On Fri, 26 Jan 2024 16:54:14 GMT, Roger Riggs wrote:
>> Jim Laskey 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 12 additional
>> commits since the last revision:
>>
>> - Merge remote-tracking branch 'upstream/master' into 8263261
>> - Update unicode to Unicode
>> - Requested changes
>> - Update String.java
>> - Requested changes
>> - Update Copyright
>> - Update copyright year of test
>> - Add JLS Unicode Escapes reference
>> - Update comment
>> - Update copyright year
>> - ... and 2 more: https://git.openjdk.org/jdk/compare/af9bfd62...040bda82
>
> src/java.base/share/classes/java/lang/String.java line 4229:
>
>> 4227: * {@code \u005Cu...u}
>> 4228: * Unicode escape
>> 4229: * single UTF-16 code unit equivalent
>
> The `...` makes it less clear what is being shown. It might be clearer to
> include the in the resulting value and drop the multiple `u` case.
Changed
> src/java.base/share/classes/java/lang/String.java line 4245:
>
>> 4243: * escape sequences and Unicode escapes are translated as
>> encountered in one pass and
>> 4244: * not done as an Unicode escapes pass followed
>> by an escape sequences
>> 4245: * pass.
>
> I would move the description of the compiler behavior to the end and remove
> "also". For example,
> Suggestion:
>
> * @implNote As a convenience for use with constructed
> * strings, this method translates Unicode escapes. For example, this
> * method could be used when ASCII encoded text files need to maintain
> Unicode
> * content. The translation is done in a single pass and is
> non-recursive. That is,
> * escape sequences and Unicode escapes are translated as encountered in
> one pass and
> * not done as an Unicode escapes pass followed by an
> escape sequences
> * pass. By comparison, the compiler translates all Unicode escapes
> before string
> * literals are translated.
Changed
> test/jdk/java/lang/String/TranslateEscapes.java line 97:
>
>> 95: verifyUnicodeEscape("\\u2022", "\u2022");
>> 96: verifyUnicodeEscape("\\ud83c\\udf09", "\ud83c\udf09");
>> 97: verifyUnicodeEscape("\\u2022", "\u2022");
>
> Include the code from the example as a test case too.
None present. Was a mis-paste.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467926349
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467926483
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467929023
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v13]
> Currently String::translateEscapes does not support unicode escapes, reported
> as a IllegalArgumentException("Invalid escape sequence: ...").
> String::translateEscapes should translate unicode escape sequences to provide
> full coverage,
Jim Laskey has updated the pull request incrementally with one additional
commit since the last revision:
Requested changes
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17491/files
- new: https://git.openjdk.org/jdk/pull/17491/files/040bda82..74707a66
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=17491=12
- incr: https://webrevs.openjdk.org/?repo=jdk=17491=11-12
Stats: 13 lines in 1 file changed: 5 ins; 5 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/17491.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17491/head:pull/17491
PR: https://git.openjdk.org/jdk/pull/17491
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v11]
On Thu, 18 Jan 2024 21:36:22 GMT, Vladimir Yaroslavskiy wrote: >> Hi Vladimir (@iaroslavski) >> >> Please see the data below using the latest version of AVX512 sort that got >> integrated into OpenJDK. >> >> > xmlns:o="urn:schemas-microsoft-com:office:office" >> xmlns:x="urn:schemas-microsoft-com:office:excel" >> xmlns="http://www.w3.org/TR/REC-html40;> >> >> >> >> >> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip.htm"> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml"> >> >> >> >> >> >> >> >> >> Benchmark (us/op) | (builder) | Stock JDK | a10 | r14 | r17 | r18 >> -- | -- | -- | -- | -- | -- | -- >> ArraysSort.Int.testSort | RANDOM | 2.202 | 2.226 | 1.535 | 1.556 | 1.546 >> ArraysSort.Int.testSort | RANDOM | 35.128 | 34.804 | 30.808 | 30.914 | 31.284 >> ArraysSort.Int.testSort | RANDOM | 78.571 | 77.224 | 72.567 | 73.098 | 73.337 >> ArraysSort.Int.testSort | RANDOM | 2466.487 | 2470.66 | 2504.654 | 2494.051 >> | 2499.746 >> ArraysSort.Int.testSort | RANDOM | 20704.14 | 20668.19 | 21377.73 | 21362.63 >> | 21278.94 >> ArraysSort.Int.testSort | REPEATED | 0.877 | 0.892 | 0.74 | 0.724 | 0.718 >> ArraysSort.Int.testSort | REPEATED | 4.789 | 4.788 | 4.92 | 4.721 | 4.891 >> ArraysSort.Int.testSort | REPEATED | 11.172 | 11.778 | 11.53 | 11.467 | >> 11.406 >> ArraysSort.Int.testSort | REPEATED | 207.212 | 207.292 | 255.46 | 258.832 | >> 254.44 >> ArraysSort.Int.testSort | REPEATED | 1862.544 | 1876.759 | 1952.646 | >> 1957.978 | 1981.906 >> ArraysSort.Int.testSort | STAGGER | 2.092 | 2.137 | 1.999 | 2.031 | 2.015 >> ArraysSort.Int.testSort | STAGGER | 29.891 | 30.321 | 25.626 | 26.318 | >> 26.396 >> ArraysSort.Int.testSort | STAGGER | 60.979 | 83.439 | 57.864 | 57.213 | >> 79.762 >> ArraysSort.Int.testSort | STAGGER | 1227.933 | 1224.495 | 1236.133 | >> 1229.773 | 1228.877 >> ArraysSort.Int.testSort | STAGGER | 9514.873 | 9565.599 | 9491.509 | >> 9481.147 | 9481.905 >> ArraysSort.Int.testSort | SHUFFLE | 1.608 | 1.595 | 1.419 | 1.442 | 1.491 >> ArraysSort.Int.testSort | SHUFFLE | 31.566 | 32.789 | 28.718 | 28.768 | >> 28.671 >> ArraysSort.Int.testSort | SHUFFLE | 82.157 | 83.741 | 70.889 | 69.951 | >> 71.196 >> ArraysSort.Int.testSort | SHUFFLE | 2251.219 | 2248.496 | 2184.459 | >> 2163.969 | 2156.239 >> ArraysSort.Int.testSort | SHUFFLE | 18211.05 | 18223.24 | 17987.4 | 18114.26 >> | 17994.98 > ... > > Hello Vamsi (@vamsi-parasa), > > Could you please run the benchmarking of new DQPS in your environment with > AVX? > > Take all classes below and put them in the package > org.openjdk.bench.java.util. > ArraysSort class contains all tests for the new versions and ready to use. > (it will run all tests in one execution). > > https://github.com/iaroslavski/sorting/blob/master/radixsort/ArraysSort.java > https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_a15.java > https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r20s.java > https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r20p.java > > Many thanks, > Vladimir Hi Vladimir (@iaroslavski), Please see the JMH data below. Thanks, Vamsi Benchmark (builder) (size) Mode Cnt Score Error Units ArraysSort.Int.a15RANDOM 600 avgt4 7.096 ±0.081 us/op ArraysSort.Int.a15RANDOM 2000 avgt4 44.014 ±1.717 us/op ArraysSort.Int.a15RANDOM9 avgt44451.444 ± 71.168 us/op ArraysSort.Int.a15RANDOM 40 avgt4 22751.966 ± 683.597 us/op ArraysSort.Int.a15RANDOM 300 avgt4 190326.306 ± 8008.512 us/op ArraysSort.Int.a15 REPEATED 600 avgt4 1.044 ±0.016 us/op ArraysSort.Int.a15 REPEATED 2000 avgt4 2.272 ±0.287 us/op ArraysSort.Int.a15 REPEATED9 avgt4 412.331 ± 11.656 us/op ArraysSort.Int.a15 REPEATED 40 avgt41908.978 ± 30.241 us/op ArraysSort.Int.a15 REPEATED 300 avgt4 15163.443 ± 100.425 us/op ArraysSort.Int.a15 STAGGER 600 avgt4 1.055 ±0.057 us/op ArraysSort.Int.a15 STAGGER 2000 avgt4 3.408 ±0.096 us/op ArraysSort.Int.a15 STAGGER9 avgt4 149.220 ±4.022 us/op ArraysSort.Int.a15 STAGGER 40 avgt4 663.096 ± 30.252 us/op ArraysSort.Int.a15 STAGGER 300 avgt45206.890 ± 234.857 us/op ArraysSort.Int.a15 SHUFFLE 600 avgt4 4.611 ±0.118 us/op ArraysSort.Int.a15 SHUFFLE 2000 avgt4 17.955 ±0.356 us/op ArraysSort.Int.a15 SHUFFLE9 avgt41410.357 ± 41.128 us/op ArraysSort.Int.a15 SHUFFLE 40 avgt45739.311 ± 128.270 us/op ArraysSort.Int.a15 SHUFFLE 300 avgt4 41501.980 ± 829.443 us/op
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v12]
On Fri, 26 Jan 2024 15:06:50 GMT, Jim Laskey wrote:
>> Currently String::translateEscapes does not support unicode escapes,
>> reported as a IllegalArgumentException("Invalid escape sequence: ...").
>> String::translateEscapes should translate unicode escape sequences to
>> provide full coverage,
>
> Jim Laskey 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 12 additional commits since
> the last revision:
>
> - Merge remote-tracking branch 'upstream/master' into 8263261
> - Update unicode to Unicode
> - Requested changes
> - Update String.java
> - Requested changes
> - Update Copyright
> - Update copyright year of test
> - Add JLS Unicode Escapes reference
> - Update comment
> - Update copyright year
> - ... and 2 more: https://git.openjdk.org/jdk/compare/b94b04ff...040bda82
src/java.base/share/classes/java/lang/String.java line 4229:
> 4227: * {@code \u005Cu...u}
> 4228: * Unicode escape
> 4229: * single UTF-16 code unit equivalent
The `...` makes it less clear what is being shown. It might be clearer to
include the in the resulting value and drop the multiple `u` case.
src/java.base/share/classes/java/lang/String.java line 4245:
> 4243: * escape sequences and Unicode escapes are translated as
> encountered in one pass and
> 4244: * not done as an Unicode escapes pass followed by
> an escape sequences
> 4245: * pass.
I would move the description of the compiler behavior to the end and remove
"also". For example,
Suggestion:
* @implNote As a convenience for use with constructed
* strings, this method translates Unicode escapes. For example, this
* method could be used when ASCII encoded text files need to maintain
Unicode
* content. The translation is done in a single pass and is non-recursive.
That is,
* escape sequences and Unicode escapes are translated as encountered in
one pass and
* not done as an Unicode escapes pass followed by an
escape sequences
* pass. By comparison, the compiler translates all Unicode escapes before
string
* literals are translated.
test/jdk/java/lang/String/TranslateEscapes.java line 97:
> 95: verifyUnicodeEscape("\\u2022", "\u2022");
> 96: verifyUnicodeEscape("\\ud83c\\udf09", "\ud83c\udf09");
> 97: verifyUnicodeEscape("\\u2022", "\u2022");
Include the code from the example as a test case too.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467892757
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467895901
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467900516
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v12]
> Currently String::translateEscapes does not support unicode escapes, reported
> as a IllegalArgumentException("Invalid escape sequence: ...").
> String::translateEscapes should translate unicode escape sequences to provide
> full coverage,
Jim Laskey 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 12 additional commits since the
last revision:
- Merge remote-tracking branch 'upstream/master' into 8263261
- Update unicode to Unicode
- Requested changes
- Update String.java
- Requested changes
- Update Copyright
- Update copyright year of test
- Add JLS Unicode Escapes reference
- Update comment
- Update copyright year
- ... and 2 more: https://git.openjdk.org/jdk/compare/eea3c2d9...040bda82
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17491/files
- new: https://git.openjdk.org/jdk/pull/17491/files/b57a551d..040bda82
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=17491=11
- incr: https://webrevs.openjdk.org/?repo=jdk=17491=10-11
Stats: 8261 lines in 340 files changed: 5165 ins; 1908 del; 1188 mod
Patch: https://git.openjdk.org/jdk/pull/17491.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17491/head:pull/17491
PR: https://git.openjdk.org/jdk/pull/17491
Re: RFR: 8303866: Allow ZipInputStream.readEnd to parse small Zip64 ZIP files [v15]
On Fri, 26 Jan 2024 14:32:58 GMT, Eirik Bjørsnøs wrote: >> ZipInputStream.readEnd currently assumes a Zip64 data descriptor if the >> number of compressed or uncompressed bytes read from the inflater is larger >> than the Zip64 magic value. >> >> While the ZIP format mandates that the data descriptor `SHOULD be stored in >> ZIP64 format (as 8 byte values) when a file's size exceeds 0x`, it >> also states that `ZIP64 format MAY be used regardless of the size of a >> file`. For such small entries, the above assumption does not hold. >> >> This PR augments ZipInputStream.readEnd to also assume 8-byte sizes if the >> ZipEntry includes a Zip64 extra information field AND the 'compressed size' >> and 'uncompressed size' have the expected Zip64 "magic" value 0x. >> This brings ZipInputStream into alignment with the APPNOTE format spec: >> >> >> When extracting, if the zip64 extended information extra >> field is present for the file the compressed and >> uncompressed sizes will be 8 byte values. >> >> >> While small Zip64 files with 8-byte data descriptors are not commonly found >> in the wild, it is possible to create one using the Info-ZIP command line >> `-fd` flag: >> >> `echo hello | zip -fd > hello.zip` >> >> The PR also adds a test verifying that such a small Zip64 file can be parsed >> by ZipInputStream. > > Eirik Bjørsnøs has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 228 commits: > > - Merge branch 'master' into data-descriptor > - Update comment of expect64BitDataDescriptor to reflect relaxed validation > - Dial down validation of the Zip64 extra field > - 8321712: C2: "failed: Multiple uses of register" in > C2_MacroAssembler::vminmax_fp > >Co-authored-by: Volodymyr Paprotski >Reviewed-by: kvn, thartmann, epeter, jbhateja > - 8319128: sun/security/pkcs11 tests fail on OL 7.9 aarch64 > >Reviewed-by: mbaesken > - 8322971: KEM.getInstance() should check if a 3rd-party security provider > is signed > >Reviewed-by: mullan, valeriep > - 8320890: [AIX] Find a better way to mimic dl handle equality > >Reviewed-by: stuefe, mdoerr > - 8323276: StressDirListings.java fails on AIX > >Reviewed-by: jpai, dfuchs > - 8319793: C2 compilation fails with "Bad graph detected in build_loop_late" > after JDK-8279888 > >Reviewed-by: chagedorn, epeter > - 8314515: java/util/concurrent/SynchronousQueue/Fairness.java failed with > "Error: fair=false i=8 j=0" > >Reviewed-by: alanb > - ... and 218 more: https://git.openjdk.org/jdk/compare/e10d1400...4af7f500 In help make progress here, I have relaxed the validation here such that we now check: - That the "streaming mode" bit 3 flag is set - That at least one of the LOC's size fields are marked 0x. - That the LOC extra field has a field with header ID 0x1 (Zip64) Any reading/validation of the contents of the Zip64 extra field has been removed. @jaikiran Is this closer to what you'd like to see? - PR Comment: https://git.openjdk.org/jdk/pull/12524#issuecomment-1912164693
Re: RFR: 8303866: Allow ZipInputStream.readEnd to parse small Zip64 ZIP files [v15]
> ZipInputStream.readEnd currently assumes a Zip64 data descriptor if the > number of compressed or uncompressed bytes read from the inflater is larger > than the Zip64 magic value. > > While the ZIP format mandates that the data descriptor `SHOULD be stored in > ZIP64 format (as 8 byte values) when a file's size exceeds 0x`, it > also states that `ZIP64 format MAY be used regardless of the size of a file`. > For such small entries, the above assumption does not hold. > > This PR augments ZipInputStream.readEnd to also assume 8-byte sizes if the > ZipEntry includes a Zip64 extra information field AND the 'compressed size' > and 'uncompressed size' have the expected Zip64 "magic" value 0x. > This brings ZipInputStream into alignment with the APPNOTE format spec: > > > When extracting, if the zip64 extended information extra > field is present for the file the compressed and > uncompressed sizes will be 8 byte values. > > > While small Zip64 files with 8-byte data descriptors are not commonly found > in the wild, it is possible to create one using the Info-ZIP command line > `-fd` flag: > > `echo hello | zip -fd > hello.zip` > > The PR also adds a test verifying that such a small Zip64 file can be parsed > by ZipInputStream. Eirik Bjørsnøs has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 228 commits: - Merge branch 'master' into data-descriptor - Update comment of expect64BitDataDescriptor to reflect relaxed validation - Dial down validation of the Zip64 extra field - 8321712: C2: "failed: Multiple uses of register" in C2_MacroAssembler::vminmax_fp Co-authored-by: Volodymyr Paprotski Reviewed-by: kvn, thartmann, epeter, jbhateja - 8319128: sun/security/pkcs11 tests fail on OL 7.9 aarch64 Reviewed-by: mbaesken - 8322971: KEM.getInstance() should check if a 3rd-party security provider is signed Reviewed-by: mullan, valeriep - 8320890: [AIX] Find a better way to mimic dl handle equality Reviewed-by: stuefe, mdoerr - 8323276: StressDirListings.java fails on AIX Reviewed-by: jpai, dfuchs - 8319793: C2 compilation fails with "Bad graph detected in build_loop_late" after JDK-8279888 Reviewed-by: chagedorn, epeter - 8314515: java/util/concurrent/SynchronousQueue/Fairness.java failed with "Error: fair=false i=8 j=0" Reviewed-by: alanb - ... and 218 more: https://git.openjdk.org/jdk/compare/e10d1400...4af7f500 - Changes: https://git.openjdk.org/jdk/pull/12524/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12524=14 Stats: 335 lines in 2 files changed: 331 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/12524.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/12524/head:pull/12524 PR: https://git.openjdk.org/jdk/pull/12524
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v11]
> Currently String::translateEscapes does not support unicode escapes, reported
> as a IllegalArgumentException("Invalid escape sequence: ...").
> String::translateEscapes should translate unicode escape sequences to provide
> full coverage,
Jim Laskey has updated the pull request incrementally with one additional
commit since the last revision:
Update unicode to Unicode
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17491/files
- new: https://git.openjdk.org/jdk/pull/17491/files/f0ea4bc9..b57a551d
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=17491=10
- incr: https://webrevs.openjdk.org/?repo=jdk=17491=09-10
Stats: 9 lines in 1 file changed: 0 ins; 0 del; 9 mod
Patch: https://git.openjdk.org/jdk/pull/17491.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17491/head:pull/17491
PR: https://git.openjdk.org/jdk/pull/17491
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v10]
> Currently String::translateEscapes does not support unicode escapes, reported
> as a IllegalArgumentException("Invalid escape sequence: ...").
> String::translateEscapes should translate unicode escape sequences to provide
> full coverage,
Jim Laskey has updated the pull request incrementally with one additional
commit since the last revision:
Requested changes
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17491/files
- new: https://git.openjdk.org/jdk/pull/17491/files/8fd58b55..f0ea4bc9
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=17491=09
- incr: https://webrevs.openjdk.org/?repo=jdk=17491=08-09
Stats: 21 lines in 2 files changed: 7 ins; 0 del; 14 mod
Patch: https://git.openjdk.org/jdk/pull/17491.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17491/head:pull/17491
PR: https://git.openjdk.org/jdk/pull/17491
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 26 Jan 2024 09:12:53 GMT, Alan Bateman wrote: > The target class is transformed in such a way to call the auxiliary class, > which necessitates the the aux-class to be in the same classloader as the > target class. But because the aux-class is defined while the target class is > still being transformed during class-loading, we cannot have any look-up > instance pointinmg to it yet. It seems like you could lazily define the aux class when the target class first needs it, instead of eagerly while the target class is being defined. e.g. generate the class bytes for the aux class up front, and embed them in the target class (For instance as a Base64 encoded string, which fits well into the constant pool). Then, have the transformed code in the target class define the the aux class when it is first needed, at which point you do have access to a lookup. - PR Comment: https://git.openjdk.org/jdk/pull/3546#issuecomment-1912057333
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v7]
On Fri, 19 Jan 2024 18:28:22 GMT, Roger Riggs wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update Copyright
>
> test/jdk/java/lang/String/TranslateEscapes.java line 113:
>
>> 111: }
>> 112:
>> 113: static void verifyEscape(String string1, String string2) {
>
> These are unicode escapes too. The method name should reflect that.
Changed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467635284
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v8]
On Tue, 23 Jan 2024 21:41:54 GMT, Naoto Sato wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Requested changes
>
> src/java.base/share/classes/java/lang/String.java line 4229:
>
>> 4227: * {@code \u005Cu...u}
>> 4228: * unicode escape
>> 4229: * single character UTF-16 equivalent
>
> It would be clearer to have "single UTF-16 code unit equivalent" as the
> translation explanation
Changed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467635554
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v9]
On Fri, 26 Jan 2024 08:38:32 GMT, Alan Bateman wrote: >> Jim Laskey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update String.java > > src/java.base/share/classes/java/lang/String.java line 4238: > >> 4236: * @return String with escape sequences and unicode escapes >> translated. >> 4237: * >> 4238: * @implNote Normally, unicode escapes are translated by the >> compiler before string > > A minor comment on the implNote is that it better to drop "Normally," from > the beginning of this sentence and "However," from the second sentence. I > think it would read a bit better. Changed - PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467635735
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v7]
On Fri, 19 Jan 2024 18:27:24 GMT, Roger Riggs wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update Copyright
>
> test/jdk/java/lang/String/TranslateEscapes.java line 127:
>
>> 125: } catch (IllegalArgumentException ex) {
>> 126: }
>> 127: }
>
> The method name implies valid unicode escape sequences, but they are all
> invalid.
> The method name could be "verifyIllegalUnicodeEscape`.
Changed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467632496
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v6]
On Fri, 19 Jan 2024 17:39:57 GMT, Raffaello Giulietti wrote: >> Jim Laskey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update copyright year of test > > test/jdk/java/lang/String/TranslateEscapes.java line 2: > >> 1: /* >> 2: * Copyright (c) 2019, 2024 Oracle and/or its affiliates. All rights >> reserved. > > Sorry for this late note. > Seems like each copyright year must be followed by a comma `,`, otherwise > validate-source fails. See, for example, my own oversight > [here](https://github.com/openjdk/jdk/pull/17490/files). Changed - PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467631294
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 26 Jan 2024 10:17:16 GMT, Alexander Kriegisch wrote: >>> @AlanBateman, the AspectJ weaving agent creates an auxiliary class to >>> implement an "around" advice for a method, i.e. a method execution is >>> intercepted and the user has options to do something before optionally >>> calling the target method and then do something afterwards too. In doing >>> so, she can modify method arguments before calling the target method, then >>> also modify the result. Instead of calling the target method, she may also >>> return a result of the expected type instead. Before, after or instead of >>> calling the target method, she can also throw an exception. >>> >>> The target class is transformed in such a way to call the auxiliary class, >>> which necessitates the the aux-class to be in the same classloader as the >>> target class. But because the aux-class is defined while the target class >>> is still being transformed during class-loading, we cannot have any look-up >>> instance pointinmg to it yet. >> >> Right, this is what JDK-8200559 was originally about. Mandy and I discussed >> it several times and load-time instrumentation that defines auxiliary >> classes in the same run-time package is a reasonable addition. >> >> The more general request for an "unrestricted defineClass" conflicts with >> several ongoing efforts so we've had to kick that to touch. > >> load-time instrumentation that defines auxiliary classes in the same >> run-time package is a reasonable addition > > Thanks for finding some common ground. I appreciate it. > >> The more general request for an "unrestricted defineClass" conflicts with >> several ongoing efforts so we've had to kick that to touch. > > I better do not start to share my general opinion about JMS _(edit: sorry, > couldn't resist in the end)_ - I mean the ongoing effort that started with > Jigsaw - in detail, because that would end up in a flame war. > > Let me just say, that "well-meant" is not the necessarily same as > "constructive", "helpful" or "necessary". Back when Jigsaw was designed, it > seemed to be a good idea. But it has been introduced a long time ago, and all > my enterprise customers since then are trying to ignore its existence as much > as they can, mostly seeing it as an impediment or at least a major annoyance. > I think, one of the reasons why Python gained so much traction for > enterprise-level big data and machine learning stuff is that you have a > dynamic language environment in which you can pretty much do whatever you > want and just get things done. > > Those same enterprise customers want to use the tools of their choosing with > as many and as easy to use features as possible. They do not trade those > features for security, but implement security in a different way, such as > devising a micro service architecture and isolating containers from each > other in OpenShift or what have you, defining service mesh rules on top of > that, if necessary. > > Deprecating security managers was a laudable idea, but adding those > unnecessary restrictions in the JVM in exchange was kind of one step forward, > two steps back. They just get in the way of getting useful things done. This > is what a language and a VM are for: getting things done in a productive way > with as few barriers as possible. Instead, with every new Java release, users > and tool providers alike are forced to jump through more hoops. There is so > much great, innovative stuff in the JVM and Java SE API. E.g., virtual > threads are the greatest thing since bread came sliced. We want more of that > and fewer barriers. Streams, NIO, the whole lambda and functional bunch of > stuff, structured concurrency, better GC, records and neat ways to > deconstruct them etc. - wow, great stuff. But new "security features" (a.k.a. > restrictions) like modules and sealed classes - not so great from a > productivity perspective. Since JDK 9, I have seen generations of developer... @kriegaex Luckily, you and your customers are not obliged to use the JPMS, nor find it useful for whatever libraries or apps you write or deploy. However, the fact that you or many other programmers do not use it does not mean it has not been a success. Anyone deeply involved with JDK and/or JVM development in recent years knows that it has been and continues to be critical to maintaining and extending the Java platform. Regarding my previous comment about Byteman using its own dedicated, dynamic module to provide secure access to MethodLookup instance you might want to look at the relevant code. It relies on a sanctioned API of Instrumentation that was introduced as part of the negotiation of JPMS integration precisely to allow agents to interact with and reconfigurethe module system at runtime. The resulting Byteman code provides a simple API that allows methods to be executed indirectly, either via reflection in jdk8- or via method handles in jdk9+.
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 26 Jan 2024 09:12:53 GMT, Alan Bateman wrote: > load-time instrumentation that defines auxiliary classes in the same run-time > package is a reasonable addition Thanks for finding some common ground. I appreciate it. > The more general request for an "unrestricted defineClass" conflicts with > several ongoing efforts so we've had to kick that to touch. I better do not start to share my general opinion about JMS _(edit: sorry, couldn't resist in the end)_ - I mean the ongoing effort that started with Jigsaw - in detail, because that would end up in a flame war. Let me just say, that "well-meant" is not the necessarily same as "constructive", "helpful" or "necessary". Back when Jigsaw was designed, it seemed to be a good idea. But it has been introduced a long time ago, and all my enterprise customers since then are trying to ignore its excistence as much as they can, mostly seeing it as an impediment or at least a major annoyance. I think, one of the reasons why Python gained so much traction for enterprise-level big data and machine learning stuff is that you have a dynamic language environment in which you can pretty much do whatever you want and just get things done. Those same enterprise customers want to use the tools of their choosing with as many and as easy to use features as possible. They do not trade those features for security, but implement security in a different way, such as devising a micro service architecture and isolating containers from each other in OpenShift or what have you, defining service mesh rules on top of that, if necessary. Deprecating security managers was a laudable idea, but adding those unnecessary restrictions in the JVM in exchange was kind of one step forward, two steps back. They just get in the way of getting useful things done. This is what a language and a VM are for: getting things done in a productive way with as few barriers as possible. Instead, with every new Java release, users and tool providers alike are forced to jump through more hoops. There is so much great, innovative stuff in the JVM and Java SE API. E.g., virtual threads are the greatest thing since bread came sliced. We want more of that and fewer barriers. Streams, NIO, the whole lambda and functional bunch of stuff, structured concurrency, better GC, records and neat ways to deconstruct them etc. - wow, great stuff. But new "security features" (a.k.a. restrictions) like modules and sealed classes - not so great from a productivity perspective. Since JDK 9, I have seen generations of developers trying to be more productive no t because but despite those new JVM "features". I know that my perspective is quite subjective and might be biased. But as an agile coach dealing mostly with developers in the JVM ecosystem, I have yet to find a team in which more than a tiny minority thinks that JMS is useful. That is not because they are lazy to learn or do not want to leave their confort zones. It is, because the JVM makes it progressively harder to get things done instead of treating users like adults and letting them take responsibility for security as they see fit. Enforcing things like a helicopter parent is not the smartest move. I apologise for polluting the thread with what ended up to be too much text that is related to the topic, but not exactly focused on the problem at hand. - PR Comment: https://git.openjdk.org/jdk/pull/3546#issuecomment-1911803302
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v11]
On Thu, 18 Jan 2024 21:36:22 GMT, Vladimir Yaroslavskiy wrote: >> Hi Vladimir (@iaroslavski) >> >> Please see the data below using the latest version of AVX512 sort that got >> integrated into OpenJDK. >> >> > xmlns:o="urn:schemas-microsoft-com:office:office" >> xmlns:x="urn:schemas-microsoft-com:office:excel" >> xmlns="http://www.w3.org/TR/REC-html40;> >> >> >> >> >> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip.htm"> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml"> >> >> >> >> >> >> >> >> >> Benchmark (us/op) | (builder) | Stock JDK | a10 | r14 | r17 | r18 >> -- | -- | -- | -- | -- | -- | -- >> ArraysSort.Int.testSort | RANDOM | 2.202 | 2.226 | 1.535 | 1.556 | 1.546 >> ArraysSort.Int.testSort | RANDOM | 35.128 | 34.804 | 30.808 | 30.914 | 31.284 >> ArraysSort.Int.testSort | RANDOM | 78.571 | 77.224 | 72.567 | 73.098 | 73.337 >> ArraysSort.Int.testSort | RANDOM | 2466.487 | 2470.66 | 2504.654 | 2494.051 >> | 2499.746 >> ArraysSort.Int.testSort | RANDOM | 20704.14 | 20668.19 | 21377.73 | 21362.63 >> | 21278.94 >> ArraysSort.Int.testSort | REPEATED | 0.877 | 0.892 | 0.74 | 0.724 | 0.718 >> ArraysSort.Int.testSort | REPEATED | 4.789 | 4.788 | 4.92 | 4.721 | 4.891 >> ArraysSort.Int.testSort | REPEATED | 11.172 | 11.778 | 11.53 | 11.467 | >> 11.406 >> ArraysSort.Int.testSort | REPEATED | 207.212 | 207.292 | 255.46 | 258.832 | >> 254.44 >> ArraysSort.Int.testSort | REPEATED | 1862.544 | 1876.759 | 1952.646 | >> 1957.978 | 1981.906 >> ArraysSort.Int.testSort | STAGGER | 2.092 | 2.137 | 1.999 | 2.031 | 2.015 >> ArraysSort.Int.testSort | STAGGER | 29.891 | 30.321 | 25.626 | 26.318 | >> 26.396 >> ArraysSort.Int.testSort | STAGGER | 60.979 | 83.439 | 57.864 | 57.213 | >> 79.762 >> ArraysSort.Int.testSort | STAGGER | 1227.933 | 1224.495 | 1236.133 | >> 1229.773 | 1228.877 >> ArraysSort.Int.testSort | STAGGER | 9514.873 | 9565.599 | 9491.509 | >> 9481.147 | 9481.905 >> ArraysSort.Int.testSort | SHUFFLE | 1.608 | 1.595 | 1.419 | 1.442 | 1.491 >> ArraysSort.Int.testSort | SHUFFLE | 31.566 | 32.789 | 28.718 | 28.768 | >> 28.671 >> ArraysSort.Int.testSort | SHUFFLE | 82.157 | 83.741 | 70.889 | 69.951 | >> 71.196 >> ArraysSort.Int.testSort | SHUFFLE | 2251.219 | 2248.496 | 2184.459 | >> 2163.969 | 2156.239 >> ArraysSort.Int.testSort | SHUFFLE | 18211.05 | 18223.24 | 17987.4 | 18114.26 >> | 17994.98 > ... > > Hello Vamsi (@vamsi-parasa), > > Could you please run the benchmarking of new DQPS in your environment with > AVX? > > Take all classes below and put them in the package > org.openjdk.bench.java.util. > ArraysSort class contains all tests for the new versions and ready to use. > (it will run all tests in one execution). > > https://github.com/iaroslavski/sorting/blob/master/radixsort/ArraysSort.java > https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_a15.java > https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r20s.java > https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r20p.java > > Many thanks, > Vladimir Hi Vladimir (@iaroslavski), I was able to figure out the issue and started the benchmarking JMH run. It's night time here, will provide the data Friday morning (US PST) Thanks, Vamsi - PR Comment: https://git.openjdk.org/jdk/pull/13568#issuecomment-1911741126
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 26 Jan 2024 08:27:41 GMT, Alan Bateman wrote: >>> BB currently opens the jdk.internal.misc.Unsafe class to a module on a >>> seperate class loader that is not reachable outside an agent, using >>> Instrumentation. >> >> @raphw, may I ask how? Is there any sample code that is not connected to the >> BB code base with its nested classes, interfaces etc.? I know, that caters >> nicely to the fluent DSL BB provides, but to me it is just a maze of code >> that is hard to comprehend. > >> > BB currently opens the jdk.internal.misc.Unsafe class to a module on a >> > seperate class loader that is not reachable outside an agent, using >> > Instrumentation. >> >> @raphw, may I ask how? Is there any sample code that is not connected to the >> BB code base with its nested classes, interfaces etc.? I know, that caters >> nicely to the fluent DSL BB provides, but to me it is just a maze of code >> that is hard to comprehend. > > Agents should not be using the JDK's internal Unsafe. This needs to said in > the strongest possible terms. > > For the discussion, it would be useful to provide a brief summary on what > AspectJ is trying to do with this weaving. This PR was originally about load > time instrumentation defining auxiliary classes, as you might get at compile > time when compiling a source file containing more than one class.. I can't > tell from your comments here or in Eclipse 546305 if this is relevant to what > you are trying to do or not. It may even be better to start a new discussion > on serviceability-dev. > @AlanBateman, the AspectJ weaving agent creates an auxiliary class to > implement an "around" advice for a method, i.e. a method execution is > intercepted and the user has options to do something before optionally > calling the target method and then do something afterwards too. In doing so, > she can modify method arguments before calling the target method, then also > modify the result. Instead of calling the target method, she may also return > a result of the expected type instead. Before, after or instead of calling > the target method, she can also throw an exception. > > The target class is transformed in such a way to call the auxiliary class, > which necessitates the the aux-class to be in the same classloader as the > target class. But because the aux-class is defined while the target class is > still being transformed during class-loading, we cannot have any look-up > instance pointinmg to it yet. Right, this is what JDK-8200559 was originally about. Mandy and I discussed it several times and load-time instrumentation that defines auxiliary classes in the same run-time package is a reasonable addition. The more general request for an "unrestricted defineClass" conflicts with several ongoing efforts so we've had to kick that to touch. - PR Comment: https://git.openjdk.org/jdk/pull/3546#issuecomment-1911716353
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v11]
On Thu, 18 Jan 2024 21:36:22 GMT, Vladimir Yaroslavskiy wrote: >> Hi Vladimir (@iaroslavski) >> >> Please see the data below using the latest version of AVX512 sort that got >> integrated into OpenJDK. >> >> > xmlns:o="urn:schemas-microsoft-com:office:office" >> xmlns:x="urn:schemas-microsoft-com:office:excel" >> xmlns="http://www.w3.org/TR/REC-html40;> >> >> >> >> >> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip.htm"> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml"> >> >> >> >> >> >> >> >> >> Benchmark (us/op) | (builder) | Stock JDK | a10 | r14 | r17 | r18 >> -- | -- | -- | -- | -- | -- | -- >> ArraysSort.Int.testSort | RANDOM | 2.202 | 2.226 | 1.535 | 1.556 | 1.546 >> ArraysSort.Int.testSort | RANDOM | 35.128 | 34.804 | 30.808 | 30.914 | 31.284 >> ArraysSort.Int.testSort | RANDOM | 78.571 | 77.224 | 72.567 | 73.098 | 73.337 >> ArraysSort.Int.testSort | RANDOM | 2466.487 | 2470.66 | 2504.654 | 2494.051 >> | 2499.746 >> ArraysSort.Int.testSort | RANDOM | 20704.14 | 20668.19 | 21377.73 | 21362.63 >> | 21278.94 >> ArraysSort.Int.testSort | REPEATED | 0.877 | 0.892 | 0.74 | 0.724 | 0.718 >> ArraysSort.Int.testSort | REPEATED | 4.789 | 4.788 | 4.92 | 4.721 | 4.891 >> ArraysSort.Int.testSort | REPEATED | 11.172 | 11.778 | 11.53 | 11.467 | >> 11.406 >> ArraysSort.Int.testSort | REPEATED | 207.212 | 207.292 | 255.46 | 258.832 | >> 254.44 >> ArraysSort.Int.testSort | REPEATED | 1862.544 | 1876.759 | 1952.646 | >> 1957.978 | 1981.906 >> ArraysSort.Int.testSort | STAGGER | 2.092 | 2.137 | 1.999 | 2.031 | 2.015 >> ArraysSort.Int.testSort | STAGGER | 29.891 | 30.321 | 25.626 | 26.318 | >> 26.396 >> ArraysSort.Int.testSort | STAGGER | 60.979 | 83.439 | 57.864 | 57.213 | >> 79.762 >> ArraysSort.Int.testSort | STAGGER | 1227.933 | 1224.495 | 1236.133 | >> 1229.773 | 1228.877 >> ArraysSort.Int.testSort | STAGGER | 9514.873 | 9565.599 | 9491.509 | >> 9481.147 | 9481.905 >> ArraysSort.Int.testSort | SHUFFLE | 1.608 | 1.595 | 1.419 | 1.442 | 1.491 >> ArraysSort.Int.testSort | SHUFFLE | 31.566 | 32.789 | 28.718 | 28.768 | >> 28.671 >> ArraysSort.Int.testSort | SHUFFLE | 82.157 | 83.741 | 70.889 | 69.951 | >> 71.196 >> ArraysSort.Int.testSort | SHUFFLE | 2251.219 | 2248.496 | 2184.459 | >> 2163.969 | 2156.239 >> ArraysSort.Int.testSort | SHUFFLE | 18211.05 | 18223.24 | 17987.4 | 18114.26 >> | 17994.98 > ... > > Hello Vamsi (@vamsi-parasa), > > Could you please run the benchmarking of new DQPS in your environment with > AVX? > > Take all classes below and put them in the package > org.openjdk.bench.java.util. > ArraysSort class contains all tests for the new versions and ready to use. > (it will run all tests in one execution). > > https://github.com/iaroslavski/sorting/blob/master/radixsort/ArraysSort.java > https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_a15.java > https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r20s.java > https://github.com/iaroslavski/sorting/blob/master/radixsort/DualPivotQuicksort_r20p.java > > Many thanks, > Vladimir Hello Vladimir (@iaroslavski), Could you please share your pom.xml as am running into issues when the JHM benchmark is run: `java.lang.IllegalAccessError: class org.openjdk.bench.java.util.DualPivotQuicksort_a15 (in unnamed module @0x520a3426) cannot access class jdk.internal.misc.Unsafe (in module java.base) because module java.base does not export jdk.internal.misc to unnamed module @0x520a3426` Added the following add-exports in pom.xml, but it's still not working. org.apache.maven.plugins maven-compiler-plugin 3.8.1 --add-exports java.base/jdk.internal.misc=ALL-UNNAMED --add-exports java.base/jdk.internal.vm.annotation=ALL-UNNAMED Thanks, Vamsi - PR Comment: https://git.openjdk.org/jdk/pull/13568#issuecomment-1911712234
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter wrote: >> To allow agents the definition of auxiliary classes, an API is needed to >> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or >> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed >> from `sun.misc.Unsafe`. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. Incremental views are not available. The > pull request now contains one commit: > > 8200559: Java agents doing instrumentation need a means to define auxiliary > classes I migrate the day https://bugs.openjdk.org/browse/JDK-8200559 finds a solution (and fallback for older JDKs). I can start a new discussion, but briefly summarized from the last time this was on the mailing list: - Agents sometimes need to add classes, for example when intercepting a reactive API where a modified callback subclass is injected, with an extra field, for instance. - The suggestion was that an argument would be provided to ClassFileTransformer which would allow adding classes to the current package. A prototype was created. - The (my) counter argument was that communication between two packages would be difficult then, as the agent does not control class loading order. If the reactive receiver would need to cast a callback object to read the field that was added, the agent's class might not yet have been instrumented and the class would not exist, yielding an error. The receiver instrumebtation can neither inject the class. - Often there is also a need to inject infrastructure into class loaders/modules that the instrumentation relies on, so I argued that the facility shouldn't be added to ClassFileTransformer to begin with, but to Instrumentation. Otherwise one could also "pseudo-transform" classes to inject infrastructure, but that felt unnecessary. - The discussion staled thereafter. - PR Comment: https://git.openjdk.org/jdk/pull/3546#issuecomment-1911679253
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v9]
On Wed, 24 Jan 2024 13:23:49 GMT, Jim Laskey wrote:
>> Currently String::translateEscapes does not support unicode escapes,
>> reported as a IllegalArgumentException("Invalid escape sequence: ...").
>> String::translateEscapes should translate unicode escape sequences to
>> provide full coverage,
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update String.java
src/java.base/share/classes/java/lang/String.java line 4238:
> 4236: * @return String with escape sequences and unicode escapes
> translated.
> 4237: *
> 4238: * @implNote Normally, unicode escapes are translated by the
> compiler before string
A minor comment on the implNote is that it better to drop "Normally," from the
beginning of this sentence and "However," from the second sentence. I think it
would read a bit better.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467358224
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 26 Jan 2024 08:27:41 GMT, Alan Bateman wrote: >>> BB currently opens the jdk.internal.misc.Unsafe class to a module on a >>> seperate class loader that is not reachable outside an agent, using >>> Instrumentation. >> >> @raphw, may I ask how? Is there any sample code that is not connected to the >> BB code base with its nested classes, interfaces etc.? I know, that caters >> nicely to the fluent DSL BB provides, but to me it is just a maze of code >> that is hard to comprehend. > >> > BB currently opens the jdk.internal.misc.Unsafe class to a module on a >> > seperate class loader that is not reachable outside an agent, using >> > Instrumentation. >> >> @raphw, may I ask how? Is there any sample code that is not connected to the >> BB code base with its nested classes, interfaces etc.? I know, that caters >> nicely to the fluent DSL BB provides, but to me it is just a maze of code >> that is hard to comprehend. > > Agents should not be using the JDK's internal Unsafe. This needs to said in > the strongest possible terms. > > For the discussion, it would be useful to provide a brief summary on what > AspectJ is trying to do with this weaving. This PR was originally about load > time instrumentation defining auxiliary classes, as you might get at compile > time when compiling a source file containing more than one class.. I can't > tell from your comments here or in Eclipse 546305 if this is relevant to what > you are trying to do or not. It may even be better to start a new discussion > on serviceability-dev. @AlanBateman, the AspectJ weaving agent creates an auxiliary class to implement an "around" advice for a method, i.e. a method execution is intercepted and the user has options to do something before optionally calling the target method and then do something afterwards too. In doing so, she can modify method arguments before calling the target method, then also modify the result. Instead of calling the target method, she may also return a result of the expected type instead. Before, after or instead of calling the target method, she can also throw an exception. The target class is transformed in such a way to call the auxiliary class, which necessitates the the aux-class to be in the same classloader as the target class. But because the aux-class is defined while the target class is still being transformed during class-loading, we cannot have any look-up instance pointinmg to it yet. IMO, this is absolutely on topic, which is why the Bugzilla bug is linked to the JDK issue and was discussed with Mandy in this context. @AlanBateman: As for using means to achieve user benefit that the JDK team deems unfit, nobody would do that, if there was an adequate way to provide such user value. Neither AspectJ nor Byte Buddy have a reputation of being some kind of shady hacker tools. - PR Comment: https://git.openjdk.org/jdk/pull/3546#issuecomment-1911676808 PR Comment: https://git.openjdk.org/jdk/pull/3546#issuecomment-1911680020
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 26 Jan 2024 08:01:59 GMT, Alexander Kriegisch wrote: > > BB currently opens the jdk.internal.misc.Unsafe class to a module on a > > seperate class loader that is not reachable outside an agent, using > > Instrumentation. > > @raphw, may I ask how? Is there any sample code that is not connected to the > BB code base with its nested classes, interfaces etc.? I know, that caters > nicely to the fluent DSL BB provides, but to me it is just a maze of code > that is hard to comprehend. Agents should not be using the JDK's internal Unsafe. This needs to said in the strongest possible terms. For the discussion, it would be useful to provide a brief summary on what AspectJ is trying to do with this weaving. This PR was originally about load time instrumentation defining auxiliary classes, as you might get at compile time when compiling a source file containing more than one class.. I can't tell from your comments here or in Eclipse 546305 if this is relevant to what you are trying to do or not. It may even be better to start a new discussion on serviceability-dev. - PR Comment: https://git.openjdk.org/jdk/pull/3546#issuecomment-1911661044
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Thu, 25 Jan 2024 12:16:13 GMT, Rafael Winterhalter wrote: > BB currently opens the jdk.internal.misc.Unsafe class to a module on a > seperate class loader that is not reachable outside an agent, using > Instrumentation. @raphw, may I ask how? Is there any sample code that is not connected to the BB code base with its nested classes, interfaces etc.? I know, that caters nicely to the fluent DSL BB provides, but to me it is just a maze of code that is hard to comprehend. - PR Comment: https://git.openjdk.org/jdk/pull/3546#issuecomment-1911633482
