On Wed, 20 Dec 2023 16:29:59 GMT, Weibing Xiao wrote:
>> Better Error Handling for Jar Tool Processing of "@File"
>>
>> This is a new PR for this PR since the original developer left the team. See
>> all of the review history at https://github.com/openjdk/jdk/pull/16423.
>>
>> Thank you.
>
>
On Thu, 21 Dec 2023 01:12:07 GMT, Adam Sotona wrote:
>> java.base java.lang.reflect.ProxyGenerator uses ASM to generate proxy
>> classes.
>>
>> This patch converts it to use Classfile API.
>>
>> It is continuation of https://github.com/openjdk/jdk/pull/10991
>>
>> Any comments and suggestions
On Wed, 20 Dec 2023 20:39:55 GMT, Mandy Chung wrote:
> Can you include the performance comparison before and after this change and
> what performance benchmarks you run?
I've run two batches on Aurora with mixed results (linked to
[JDK-8294960](https://bugs.openjdk.org/browse/JDK-8294960)):
`
On Thu, 21 Dec 2023 01:12:07 GMT, Adam Sotona wrote:
>> java.base java.lang.reflect.ProxyGenerator uses ASM to generate proxy
>> classes.
>>
>> This patch converts it to use Classfile API.
>>
>> It is continuation of https://github.com/openjdk/jdk/pull/10991
>>
>> Any comments and suggestions
> java.base java.lang.reflect.ProxyGenerator uses ASM to generate proxy classes.
>
> This patch converts it to use Classfile API.
>
> It is continuation of https://github.com/openjdk/jdk/pull/10991
>
> Any comments and suggestions are welcome.
>
> Please review.
>
> Thank you,
> Adam
Adam Sot
On Wed, 20 Dec 2023 22:46:32 GMT, Markus KARG wrote:
> So I did not do something wrong but finally I learned Github forensics now.
If no crime was committed, I would call it archaeology.
-
PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1865249242
On Wed, 20 Dec 2023 18:49:39 GMT, Brian Burkhalter wrote:
> > this icon only says that Alan made change requests
>
> I think it's if the "changes requested" button is selected in the "Submit
> Review" popup under "Files Changed."
Typically yes. But not in the current case: Alan pressed "approv
On Wed, 29 Nov 2023 15:01:32 GMT, Scott Gibbons wrote:
>> Re-write the IndexOf code without the use of the pcmpestri instruction, only
>> using AVX2 instructions. This change accelerates String.IndexOf on average
>> 1.3x for AVX2. The benchmark numbers:
>>
>>
>> Benchmark
On Tue, 19 Dec 2023 18:42:19 GMT, Scott Gibbons wrote:
>> Re-write the IndexOf code without the use of the pcmpestri instruction, only
>> using AVX2 instructions. This change accelerates String.IndexOf on average
>> 1.3x for AVX2. The benchmark numbers:
>>
>>
>> Benchmark
On Wed, 20 Dec 2023 21:52:40 GMT, Roger Riggs wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Clear sooner
>
> src/java.base/share/classes/java/lang/StringBuffer.java line 719:
>
>> 717: public synchronized Stri
> The new repeat methods were not clearing the toStringCache.
Jim Laskey has updated the pull request incrementally with one additional
commit since the last revision:
Clear sooner
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17172/files
- new: https://git.openjdk.org/j
On Wed, 20 Dec 2023 22:15:04 GMT, Jim Laskey wrote:
>> The new repeat methods were not clearing the toStringCache.
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clear sooner
LGTM.
-
Marked as reviewed by mk..
On Wed, 20 Dec 2023 22:15:04 GMT, Jim Laskey wrote:
>> The new repeat methods were not clearing the toStringCache.
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clear sooner
LGTM
-
Marked as reviewed by rrigg
On Wed, 20 Dec 2023 15:37:37 GMT, Sean Coffey wrote:
> trivial test edit to use FileUtils.deleteFileIfExistsWithRetry test library
> API
>
> tested on CI system with test-repeat = 20 - no issues seen.
This pull request has now been integrated.
Changeset: f6fe39ff
Author:Sean Coffey
URL:
On Wed, 20 Dec 2023 15:37:37 GMT, Sean Coffey wrote:
> trivial test edit to use FileUtils.deleteFileIfExistsWithRetry test library
> API
>
> tested on CI system with test-repeat = 20 - no issues seen.
Thanks for the review Lance. I didn't use try-with-resources during test
creation in those l
On Wed, 20 Dec 2023 20:25:07 GMT, Jim Laskey wrote:
> The new repeat methods were not clearing the toStringCache.
src/java.base/share/classes/java/lang/StringBuffer.java line 719:
> 717: public synchronized StringBuffer repeat(int codePoint, int count) {
> 718: super.repeat(codePoin
On Wed, 20 Dec 2023 19:00:50 GMT, Alisen Chung wrote:
> Backport of JDK-8322041
This pull request has now been integrated.
Changeset: a05f3d10
Author:Alisen Chung
URL:
https://git.openjdk.org/jdk22/commit/a05f3d10cc374297fd9b0fa27305c9b26f8081d2
Stats: 290 lines in 36 files chan
Hi all,
This pull request contains a backport of commit
[0f8e4e0a](https://github.com/openjdk/jdk/commit/0f8e4e0a81257c678e948c341a241dc0b810494f)
from the [openjdk/jdk](https://git.openjdk.org/jdk) repository.
The commit being backported was authored by Serguei Spitsyn on 19 Dec 2023 and
was
> This PR suggests we retire the binary test vectors `ZipFile/input.zip`,
> `ZipFile/input.jar` and `ZipFile/crash.jar`
>
> Binary test vectors are harder to analyze, and sharing test vectors across
> unrelated tests increases maintenance burden. It would be better to have each
> test produce
This PR suggests that `Files.setPosixPermissions`as implemented by
`ZipFileSystem` should preserve the leading seven bits of the 'external file
attributes' field. These bits contain the 'file type', 'setuid', 'setgid', and
'sticky' bits. These are unrelated to permissions and should not be modif
On Wed, 20 Dec 2023 14:15:48 GMT, Alan Bateman wrote:
> Update: ignore this I mis-read that it updates the current thread's suspend
> value, not the thread's suspend value.
Thanks, Alan. I've also got confused with this and even filed a follow up bug.
:)
Yes, the initial design was the `_is_di
On Wed, 20 Dec 2023 19:00:50 GMT, Alisen Chung wrote:
> Backport of JDK-8322041
Marked as reviewed by naoto (Reviewer).
-
PR Review: https://git.openjdk.org/jdk22/pull/22#pullrequestreview-1791633961
On Mon, 18 Dec 2023 11:03:10 GMT, Adam Sotona wrote:
>> java.base java.lang.invoke package heavily uses ASM to generate lambdas and
>> method handles.
>>
>> This patch converts ASM calls to Classfile API.
>>
>> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
>>
>> Any com
On Wed, 20 Dec 2023 20:08:08 GMT, Adam Sotona wrote:
>> java.base java.lang.reflect.ProxyGenerator uses ASM to generate proxy
>> classes.
>>
>> This patch converts it to use Classfile API.
>>
>> It is continuation of https://github.com/openjdk/jdk/pull/10991
>>
>> Any comments and suggestions
The new repeat methods were not clearing the toStringCache.
-
Commit messages:
- Clear toStringCache when calling StringBuffer::repeat
Changes: https://git.openjdk.org/jdk/pull/17172/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17172&range=00
Issue: https://bugs.openjdk
On Tue, 19 Dec 2023 20:45:58 GMT, Mandy Chung wrote:
> Can you include a summary of the performance comparison before and after the
> patch per the
> [comment](https://github.com/openjdk/jdk/pull/10991#pullrequestreview-1333426016)
> in #10991.
I've found significant performance regressions w
> java.base java.lang.reflect.ProxyGenerator uses ASM to generate proxy classes.
>
> This patch converts it to use Classfile API.
>
> It is continuation of https://github.com/openjdk/jdk/pull/10991
>
> Any comments and suggestions are welcome.
>
> Please review.
>
> Thank you,
> Adam
Adam Sot
On Wed, 20 Dec 2023 19:00:50 GMT, Alisen Chung wrote:
> Backport of JDK-8322041
The patch looks good. Additionally, I confirmed that the files in question are
identical to those in jdk mainline after this patch is applied.
It will need a "R"eviewer to approve.
-
Marked as reviewe
Backport of JDK-8322041
-
Commit messages:
- Backport b061b6678fde891974d5b58cec963b3481099a8d
Changes: https://git.openjdk.org/jdk22/pull/22/files
Webrev: https://webrevs.openjdk.org/?repo=jdk22&pr=22&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8322041
Stats: 290 lines
On Wed, 20 Dec 2023 18:28:27 GMT, Markus KARG wrote:
> this icon only says that Alan made change requests
I think it's if the "changes requested" button is selected in the "Submit
Review" popup under "Files Changed."
-
PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecommen
On Wed, 20 Dec 2023 17:57:57 GMT, Brian Burkhalter wrote:
> > But where do you see a change request from Alan _before_ I posted
> > `/integrate`?
>
> Mouse-over the icon to the right of his name at the top right under
> "Reviewers."
I might be wrong, but IMHO this icon only says that Alan mad
> Adds serialization misdeclaration events to JFR.
Raffaello Giulietti 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 11 additional commits
since the
On Sat, 16 Dec 2023 14:07:52 GMT, Markus KARG wrote:
>> Fixes JDK-8322141
>>
>> As suggested by @vlsi and documented by @bplb this PR does not return, but
>> only sets the maximum result value.
>
> Markus KARG has updated the pull request incrementally with one additional
> commit since the la
On Wed, 20 Dec 2023 16:52:38 GMT, Brian Burkhalter wrote:
> > Alan actually did approve them on December 15.
>
> I know, with requested changes. I didn't see the request transition to
> "Ready" which is where it needs to be to `/integrate`.
Looking at the Github history I need to say that I ca
On Wed, 20 Dec 2023 15:37:37 GMT, Sean Coffey wrote:
> trivial test edit to use FileUtils.deleteFileIfExistsWithRetry test library
> API
>
> tested on CI system with test-repeat = 20 - no issues seen.
Hi Sean,
Thank you for tackling this.
Would it be beneficial to use a try-with-resources fo
> Adds serialization misdeclaration events to JFR.
Raffaello Giulietti has updated the pull request incrementally with one
additional commit since the last revision:
Minor changes.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17129/files
- new: https://git.openjdk.org/j
On Fri, 15 Dec 2023 08:23:58 GMT, Markus KARG wrote:
> Fixes JDK-8322141
>
> As suggested by @vlsi and documented by @bplb this PR does not return, but
> only sets the maximum result value.
This pull request has now been integrated.
Changeset: 2d609557
Author:Markus KARG
Committer: Brian
On Wed, 20 Dec 2023 00:56:16 GMT, Jaikiran Pai wrote:
> The updated source change looks fine to me.
@jaikiran Thanks for the corroboration.
-
PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1864827299
> Adds serialization misdeclaration events to JFR.
Raffaello Giulietti has updated the pull request incrementally with one
additional commit since the last revision:
Removed event kind.
serialVersionUID must have type long.
Test now base on keyword search in event message.
Commented test
On Wed, 20 Dec 2023 16:52:38 GMT, Brian Burkhalter wrote:
>>> Approved.
>>>
>>> Please note in future that it is better not to `/integrate` until the
>>> request has actually been approved by a Reviewer.
>>
>> Alan actually did approve them on December 15.
>
>> Alan actually did approve them o
On Wed, 20 Dec 2023 09:47:05 GMT, Markus KARG wrote:
> Alan actually did approve them on December 15.
I know, with requested changes. I didn't see the request transition to "Ready"
which is where it needs to be to `/integrate`.
-
PR Comment: https://git.openjdk.org/jdk/pull/17119#
> Better Error Handling for Jar Tool Processing of "@File"
>
> This is a new PR for this PR since the original developer left the team. See
> all of the review history at https://github.com/openjdk/jdk/pull/16423.
>
> Thank you.
Weibing Xiao has updated the pull request incrementally with one
trivial test edit to use FileUtils.deleteFileIfExistsWithRetry test library API
tested on CI system with test-repeat = 20 - no issues seen.
-
Commit messages:
- 8322078
Changes: https://git.openjdk.org/jdk/pull/17169/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17169&ran
On Wed, 20 Dec 2023 14:28:39 GMT, Roger Riggs wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Changes according to reviewer's comments.
>
> It would also be useful/interesting to include a test that checks e
On Wed, 20 Dec 2023 15:01:02 GMT, Raffaello Giulietti
wrote:
>> src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java
>> line 113:
>>
>>> 111: if (longFromStatic(f) == null) {
>>> 112: commitEvent(SUID_CONVERTIBLE_TO_LONG,
>>> 113:
On Wed, 20 Dec 2023 14:17:19 GMT, Roger Riggs wrote:
>> Same remark here about finality as
>> https://github.com/openjdk/jdk/pull/17129#discussion_r1432400888. public
>> statics should be final.
>
> I'd also remove the error codes, the string messages will be pretty stable
> and the extra surf
On Wed, 20 Dec 2023 14:28:39 GMT, Roger Riggs wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Changes according to reviewer's comments.
>
> It would also be useful/interesting to include a test that checks e
On Tue, 19 Dec 2023 16:45:04 GMT, Raffaello Giulietti
wrote:
>> Adds serialization misdeclaration events to JFR.
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Changes according to reviewer's comments.
It would al
On Tue, 19 Dec 2023 16:45:04 GMT, Raffaello Giulietti
wrote:
>> Adds serialization misdeclaration events to JFR.
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Changes according to reviewer's comments.
src/java.ba
On Wed, 20 Dec 2023 08:29:19 GMT, Daniel Fuchs wrote:
>> You could define them with an Enum but use the ordinal as the value for JFR.
>
> Same remark here about finality as
> https://github.com/openjdk/jdk/pull/17129#discussion_r1432400888. public
> statics should be final.
I'd also remove the
On Mon, 18 Dec 2023 17:09:59 GMT, Serguei Spitsyn wrote:
>> This fix is for JDK 23 but the intention is to back port it to 22 in RDP-1
>> time frame.
>> It is fixing a deadlock issue between `VirtualThread` class critical
>> sections with the `interruptLock` (in methods: `unpark()`, `interrupt(
On Tue, 19 Dec 2023 19:19:35 GMT, Roger Riggs wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Better name for a label, corrected name of removed field.
>
> src/java.base/share/classes/java/io/SerializationMi
On Tue, 12 Dec 2023 15:45:47 GMT, Per Minborg wrote:
>> This PR proposes to change the specification for some methods that take
>> `long` offsets so that they will throw an `IllegalArgumentException` rather
>> than an `IndexOutOfBoundsException` for negative values.
>>
>> The PR also proposes
On Wed, 20 Dec 2023 08:02:14 GMT, Alan Bateman wrote:
>> src/hotspot/share/prims/jvm.cpp line 4024:
>>
>>> 4022: #else
>>> 4023: fatal("Should only be called with JVMTI enabled");
>>> 4024: #endif
>>
>> You can't do this! The Java code knows nothing about JVM TI being
>> enabled/disabled and
On Tue, 19 Dec 2023 18:31:19 GMT, Brian Burkhalter wrote:
>> Markus KARG has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Test was using Integer but must use Long
>
> I verified that the test now fails without the source change but passes
> Currently if we create a record it's fields are compared in their declaration
> order. This might be ineffective in cases when two objects have "heavy"
> fields equals to each other, but different "lightweight" fields (heavy and
> lightweight in terms of comparison) e.g. primitives, enums,
>
On Tue, 19 Dec 2023 18:20:05 GMT, Brian Burkhalter wrote:
> Approved.
>
> Please note in future that it is better not to `/integrate` until the request
> has actually been approved by a Reviewer.
Alan actually did approve them on December 15.
-
PR Comment: https://git.openjdk.org
On Tue, 19 Dec 2023 19:39:22 GMT, Roger Riggs wrote:
>> What about fixed `String`s rather than `int`s for the kind of error?
>> Something like `"SUID_INEFFECTIVE_ON_ENUM"`, and so on?
>> It would be nice to be able to use enums, but AFAIK that's not supported in
>> JFR.
>
> You could define them
On Tue, 19 Dec 2023 16:00:09 GMT, Raffaello Giulietti
wrote:
>> src/java.base/share/classes/jdk/internal/event/SerializationMisdeclarationEvent.java
>> line 94:
>>
>>> 92:
>>> 93: /*
>>> 94: * These constants are not final on purpose.
>>
>> Just curious - what's the purpose? I don't
On Tue, 19 Dec 2023 12:47:53 GMT, Goetz Lindenmaier wrote:
> …g exception
>
> After leaving the method by throwing an exception the data can not be cleaned
> any more.
src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java line 122:
> 120: ioe.addSuppresse
On Tue, 19 Dec 2023 12:47:53 GMT, Goetz Lindenmaier wrote:
> …g exception
>
> After leaving the method by throwing an exception the data can not be cleaned
> any more.
This pull request has now been integrated.
Changeset: 2f917bff
Author:Goetz Lindenmaier
URL:
https://git.openjdk.
On Tue, 19 Dec 2023 12:47:53 GMT, Goetz Lindenmaier wrote:
> …g exception
>
> After leaving the method by throwing an exception the data can not be cleaned
> any more.
Thanks for the reviews!
SAPs nigthly testing passed for this change.
Sorry, I saw the java.uti. comment too late :(
---
On Wed, 20 Dec 2023 04:44:35 GMT, David Holmes wrote:
> You can't do this! The Java code knows nothing about JVM TI being
> enabled/disabled and will call this function unconditionally.
Indeed. I wonder if anyone is testing minimal builds to catch issues like this.
-
PR Review Com
63 matches
Mail list logo
