Re: RFR: 8355632: WhiteBox.waitForReferenceProcessing() fails assert for return type

2025-04-26 Thread Kim Barrett
On Fri, 25 Apr 2025 23:37:13 GMT, Brent Christian wrote: > The newly-added `WhiteBox.waitForReferenceProcessing()` (see > [8305186](https://bugs.openjdk.org/browse/JDK-8305186)) always fails with > assertions enabled. > I've updated the assertion, and also added the test I used locally to test

Re: RFR: 8355632: WhiteBox.waitForReferenceProcessing() fails assert for return type

2025-04-26 Thread Kim Barrett
On Sat, 26 Apr 2025 09:18:35 GMT, Kim Barrett wrote: >> The newly-added `WhiteBox.waitForReferenceProcessing()` (see >> [8305186](https://bugs.openjdk.org/browse/JDK-8305186)) always fails with >> assertions enabled. >> I've updated the assertion, and also added th

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests [v9]

2025-04-23 Thread Kim Barrett
On Wed, 23 Apr 2025 17:45:29 GMT, Brent Christian wrote: >> Certain specific types of tests involving GC and reference processing need >> to account for the delay between a GC completing (during which the GC clears >> a Reference), and the Reference being added to its associated queue. At >> p

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests [v8]

2025-04-22 Thread Kim Barrett
On Wed, 23 Apr 2025 00:23:57 GMT, Brent Christian wrote: >> Certain specific types of tests involving GC and reference processing need >> to account for the delay between a GC completing (during which the GC clears >> a Reference), and the Reference being added to its associated queue. At >> p

Re: RFR: 8354565: jtreg failure handler GatherProcessInfoTimeoutHandler has a leftover call to System.loadLibrary

2025-04-14 Thread Kim Barrett
On Tue, 15 Apr 2025 05:50:03 GMT, Jaikiran Pai wrote: > Can I please get a review of this cleanup in the jtreg timeout handler > `GatherProcessInfoTimeoutHandler`? > > As noted in https://bugs.openjdk.org/browse/JDK-8354565, this is a leftover > after the change done in https://bugs.openjdk.ja

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests

2025-04-14 Thread Kim Barrett
On Tue, 15 Apr 2025 00:00:35 GMT, Hans Boehm wrote: > > Racy initialization is fine, the field can be static and the last one (any > > racy init would all the same) wins: > > How did you conclude that? From what I can see, Method has non-final fields. > If another thread reads waitForReferenceP

Re: RFR: 8301971: Make JDK source code UTF-8 [v3]

2025-04-14 Thread Kim Barrett
On Mon, 14 Apr 2025 12:53:35 GMT, Magnus Ihse Bursie wrote: >> Most of the JDK code base has been transitioned to UTF-8, but not all. This >> has recently become an acute problem, since our mixing of iso-8859-1 and >> utf-8 in properties files confused the version of `sed` that is shipped with

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests [v6]

2025-04-14 Thread Kim Barrett
On Fri, 11 Apr 2025 23:31:16 GMT, Brent Christian wrote: >> Certain specific types of tests involving GC and reference processing need >> to account for the delay between a GC completing (during which the GC clears >> a Reference), and the Reference being added to its associated queue. At >> p

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests [v6]

2025-04-14 Thread Kim Barrett
On Sat, 12 Apr 2025 05:47:06 GMT, Kim Barrett wrote: >> Brent Christian has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Make *the method* static, also >> - Make Method static > > test/lib/jdk/

Re: RFR: 8301971: Make JDK source code UTF-8 [v2]

2025-04-14 Thread Kim Barrett
On Sun, 13 Apr 2025 23:14:41 GMT, Magnus Ihse Bursie wrote: >> Most of the JDK code base has been transitioned to UTF-8, but not all. This >> has recently become an acute problem, since our mixing of iso-8859-1 and >> utf-8 in properties files confused the version of `sed` that is shipped with

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests [v6]

2025-04-11 Thread Kim Barrett
On Fri, 11 Apr 2025 23:31:16 GMT, Brent Christian wrote: >> Certain specific types of tests involving GC and reference processing need >> to account for the delay between a GC completing (during which the GC clears >> a Reference), and the Reference being added to its associated queue. At >> p

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests [v5]

2025-04-11 Thread Kim Barrett
On Fri, 11 Apr 2025 23:26:45 GMT, Brent Christian wrote: >> test/lib/jdk/test/whitebox/WhiteBox.java line 568: >> >>> 566: private Method getWaitForReferenceProcessingMethod() { >>> 567: Method wfrp = waitForReferenceProcessingMethod; >>> 568: if (wfrp == null) { >> >> Racy initializa

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-11 Thread Kim Barrett
On Fri, 11 Apr 2025 08:51:56 GMT, Aleksey Shipilev wrote: >> I think it would be "easier" to shift `@IntrinsicCandidate` to a private >> `Reference.getImpl/get0` pair of methods, and intrinsify one of those >> instead. Pretty much like current `clear` and `refersTo` are doing it now. >> And it

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-11 Thread Kim Barrett
On Wed, 2 Apr 2025 18:38:17 GMT, Kim Barrett wrote: >> Kim Barrett has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - remove timeout by using waitForReferenceProcessing >> - make ill-timed gc in n

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests [v4]

2025-04-10 Thread Kim Barrett
On Thu, 10 Apr 2025 22:24:04 GMT, Brent Christian wrote: >> Certain specific types of tests involving GC and reference processing need >> to account for the delay between a GC completing (during which the GC clears >> a Reference), and the Reference being added to its associated queue. At >> p

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests [v3]

2025-04-10 Thread Kim Barrett
On Wed, 9 Apr 2025 23:12:00 GMT, Brent Christian wrote: >> Certain specific types of tests involving GC and reference processing need >> to account for the delay between a GC completing (during which the GC clears >> a Reference), and the Reference being added to its associated queue. At >> pr

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests [v2]

2025-04-09 Thread Kim Barrett
On Wed, 9 Apr 2025 19:54:55 GMT, Brent Christian wrote: >> Certain specific types of tests involving GC and reference processing need >> to account for the delay between a GC completing (during which the GC clears >> a Reference), and the Reference being added to its associated queue. At >> pr

Re: RFR: 8305186: Reference.waitForReferenceProcessing should be more accessible to tests

2025-04-08 Thread Kim Barrett
On Tue, 8 Apr 2025 20:20:56 GMT, Brent Christian wrote: > Certain specific types of tests involving GC and reference processing need to > account for the delay between a GC completing (during which the GC clears a > Reference), and the Reference being added to its associated queue. At > presen

Re: RFR: 8352565: Add native method implementation of Reference.get() [v3]

2025-04-05 Thread Kim Barrett
insics). > (2) mach5 tier1-6 with interpreter and C1 Reference::get intrinsics disabled. Kim Barrett has updated the pull request incrementally with one additional commit since the last revision: add package decl to test - Changes: - all: https://git.openjdk.org/jdk/pull/24315/fi

Re: RFR: 8352565: Add native method implementation of Reference.get() [v2]

2025-04-05 Thread Kim Barrett
On Tue, 1 Apr 2025 22:01:55 GMT, Brent Christian wrote: >> Kim Barrett has updated the pull request incrementally with one additional >> commit since the last revision: >> >> parameterized return type of native get0 > > test/hotspot/jtreg/gc/TestNativeReferen

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-02 Thread Kim Barrett
On Wed, 2 Apr 2025 18:33:16 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used w

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-02 Thread Kim Barrett
insics). > (2) mach5 tier1-6 with interpreter and C1 Reference::get intrinsics disabled. Kim Barrett has updated the pull request incrementally with three additional commits since the last revision: - remove timeout by using waitForReferenceProcessing - make ill-timed gc in non-concurrent case

Re: RFR: 8352565: Add native method implementation of Reference.get() [v2]

2025-04-01 Thread Kim Barrett
On Tue, 1 Apr 2025 01:54:13 GMT, Chen Liang wrote: >> Kim Barrett has updated the pull request incrementally with one additional >> commit since the last revision: >> >> parameterized return type of native get0 > > src/java.base/share/classes/java/lang/ref/Refe

Re: RFR: 8352565: Add native method implementation of Reference.get() [v2]

2025-04-01 Thread Kim Barrett
insics). > (2) mach5 tier1-6 with interpreter and C1 Reference::get intrinsics disabled. Kim Barrett has updated the pull request incrementally with one additional commit since the last revision: parameterized return type of native get0 - Changes: - all: https://git.openjdk.org

RFR: 8352565: Add native method implementation of Reference.get()

2025-03-29 Thread Kim Barrett
Please review this change which adds a native method providing the implementation of Reference::get. Referece::get is an intrinsic candidate, so this native method implementation is only used when the intrinsic is not. Currently there is intrinsic support by the interpreter, C1, C2, and graal, wh

Re: RFR: 8351374: Improve comment about queue.remove timeout in CleanerImpl.run [v2]

2025-03-18 Thread Kim Barrett
On Mon, 17 Mar 2025 17:22:10 GMT, Kim Barrett wrote: >> Please review this revision of a previously puzzling comment intending to >> provide the rationale for a bit of non-obvious code. > > Kim Barrett has updated the pull request incrementally with one additional >

Integrated: 8351374: Improve comment about queue.remove timeout in CleanerImpl.run

2025-03-18 Thread Kim Barrett
On Fri, 7 Mar 2025 23:17:53 GMT, Kim Barrett wrote: > Please review this revision of a previously puzzling comment intending to > provide the rationale for a bit of non-obvious code. This pull request has now been integrated. Changeset: 355b2f3b Author: Kim Barrett URL:

Re: RFR: 8351374: Improve comment about queue.remove timeout in CleanerImpl.run [v2]

2025-03-17 Thread Kim Barrett
> Please review this revision of a previously puzzling comment intending to > provide the rationale for a bit of non-obvious code. Kim Barrett has updated the pull request incrementally with one additional commit since the last revision: bchristi-git review - Changes:

Re: RFR: 8351374: Improve comment about queue.remove timeout in CleanerImpl.run

2025-03-12 Thread Kim Barrett
On Mon, 10 Mar 2025 23:14:23 GMT, Brent Christian wrote: >> Please review this revision of a previously puzzling comment intending to >> provide the rationale for a bit of non-obvious code. > > src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java line 142: > >> 140: // w

RFR: 8351374: Improve comment about queue.remove timeout in CleanerImpl.run

2025-03-07 Thread Kim Barrett
Please review this revision of a previously puzzling comment intending to provide the rationale for a bit of non-obvious code. - Commit messages: - improve comment Changes: https://git.openjdk.org/jdk/pull/23952/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23952&range=00

Re: RFR: 8344332: (bf) Migrate DirectByteBuffer to use java.lang.ref.Cleaner [v13]

2025-02-24 Thread Kim Barrett
On Mon, 24 Feb 2025 07:25:41 GMT, Alan Bateman wrote: > If you are suggesting exposing this as a public API then I don't think we > should do this, Not at this time, quite possibly never. As mentioned, I made some things public for ease of prototyping. I even commented it as such. > I actual

Re: RFR: 8204868: java/util/zip/ZipFile/TestCleaner.java still fails with "cleaner failed to clean zipfile."

2025-02-23 Thread Kim Barrett
On Mon, 24 Feb 2025 05:50:40 GMT, Jaikiran Pai wrote: > Can I please get a review of this test-only change which proposes to address > an intermittent failure in the > `test/jdk/java/util/zip/ZipFile/TestCleaner.java` test? > > This test does operations on Inflater/Deflater/ZipFile and closes

Re: RFR: 8344332: (bf) Migrate DirectByteBuffer to use java.lang.ref.Cleaner [v13]

2025-02-23 Thread Kim Barrett
On Tue, 18 Feb 2025 14:33:16 GMT, Aleksey Shipilev wrote: >> DirectByteBuffers are still using old `jdk.internal.ref.Cleaner` >> implementation. That implementation carries a doubly-linked list, and so >> makes DBB suffer from the same issue fixed for generic >> `java.lang.ref.Cleaner` users w

Re: RFR: 8344332: (bf) Migrate DirectByteBuffer to use java.lang.ref.Cleaner [v13]

2025-02-23 Thread Kim Barrett
On Tue, 18 Feb 2025 14:33:16 GMT, Aleksey Shipilev wrote: >> DirectByteBuffers are still using old `jdk.internal.ref.Cleaner` >> implementation. That implementation carries a doubly-linked list, and so >> makes DBB suffer from the same issue fixed for generic >> `java.lang.ref.Cleaner` users w

Re: RFR: 8344332: (bf) Migrate DirectByteBuffer to use java.lang.ref.Cleaner [v11]

2025-02-18 Thread Kim Barrett
On Wed, 29 Jan 2025 19:56:00 GMT, Aleksey Shipilev wrote: >> DirectByteBuffers are still using old `jdk.internal.ref.Cleaner` >> implementation. That implementation carries a doubly-linked list, and so >> makes DBB suffer from the same issue fixed for generic >> `java.lang.ref.Cleaner` users w

Re: RFR: 8344332: (bf) Migrate DirectByteBuffer to use java.lang.ref.Cleaner [v11]

2025-02-13 Thread Kim Barrett
On Mon, 10 Feb 2025 09:28:45 GMT, Aleksey Shipilev wrote: >> @AlanBateman I've not done any work on JDK-8305186. There has also been >> discussion about making >> that function non-private or even public (though with concerns about >> specification difficulty) for use in >> places like this. >

Re: RFR: 8344332: (bf) Migrate DirectByteBuffer to use java.lang.ref.Cleaner [v11]

2025-01-30 Thread Kim Barrett
On Thu, 30 Jan 2025 08:36:34 GMT, Alan Bateman wrote: >> src/java.base/share/classes/java/nio/Bits.java line 146: >> >>> 144: } >>> 145: >>> 146: if (canary == null || canary.isDead()) { >> >> If we're keeping Reference.waitForPendingReferences, why not continue

Re: RFR: 8344332: (bf) Migrate DirectByteBuffer to use java.lang.ref.Cleaner [v11]

2025-01-29 Thread Kim Barrett
On Wed, 29 Jan 2025 19:56:00 GMT, Aleksey Shipilev wrote: >> DirectByteBuffers are still using old `jdk.internal.ref.Cleaner` >> implementation. That implementation carries a doubly-linked list, and so >> makes DBB suffer from the same issue fixed for generic >> `java.lang.ref.Cleaner` users w

Re: RFR: 8344332: (bf) Migrate DirectByteBuffer to use java.lang.ref.Cleaner [v10]

2025-01-29 Thread Kim Barrett
On Sat, 25 Jan 2025 07:14:51 GMT, Aleksey Shipilev wrote: >> DirectByteBuffers are still using old `jdk.internal.ref.Cleaner` >> implementation. That implementation carries a doubly-linked list, and so >> makes DBB suffer from the same issue fixed for generic >> `java.lang.ref.Cleaner` users w

Re: RFR: 8346773: Fix unmatched brackets in source files [v3]

2024-12-24 Thread Kim Barrett
On Wed, 25 Dec 2024 02:34:16 GMT, Qizheng Xing wrote: >> This patch fixes unmatched brackets in some source files. > > Qizheng Xing has updated the pull request incrementally with one additional > commit since the last revision: > > Revert fix in the CTW Makefile. Looks good. -

Re: RFR: 8346773: Fix unmatched brackets in source files [v2]

2024-12-24 Thread Kim Barrett
On Tue, 24 Dec 2024 03:26:23 GMT, Qizheng Xing wrote: >> This patch fixes unmatched brackets in some source files. > > Qizheng Xing has updated the pull request incrementally with three additional > commits since the last revision: > > - Update `hotspot-unit-tests.md` and HTML (using Pandoc 2.

Re: RFR: 8346773: Fix unmatched brackets in source files [v2]

2024-12-24 Thread Kim Barrett
On Tue, 24 Dec 2024 20:27:37 GMT, Kim Barrett wrote: >> Hmm, apparently this is not run as part of tier 1-3, which I ran in my >> patch. I wonder how this test is run. And it means this patch needs to be >> backported to 24. > > Only this small piece would be approp

Re: RFR: 8346773: Fix unmatched brackets in source files [v2]

2024-12-24 Thread Kim Barrett
On Tue, 24 Dec 2024 15:45:24 GMT, Chen Liang wrote: >> I've tested this by running `make`, and the previous Makefile reports an >> error like file `test/lib/jtreg/SkippedException.java)` not found. So this >> fix is necessary. >> >> I also checked the git log, this change was introduced in >>

Re: RFR: 8346773: Fix unmatched brackets in source files

2024-12-23 Thread Kim Barrett
On Mon, 23 Dec 2024 19:33:36 GMT, Kim Barrett wrote: > That makes it harder to review and to manage the review, because it is large > and affects a wide range of areas, so may need many reviewers. And many of the appropriate reviewers might be unavailable for a while, since it's r

Re: RFR: 8346773: Fix unmatched brackets in source files

2024-12-23 Thread Kim Barrett
On Mon, 23 Dec 2024 09:45:00 GMT, Qizheng Xing wrote: > This patch fixes unmatched brackets in some source files. I strongly suggest avoiding omnibus changes like this when possible (which it is here). Just because it's all about one "kind" of change doesn't make it a cohesive change. This is to

Re: RFR: 8345016: [ASAN] java.c false positive ‘%s’ directive argument is null [-Werror=format-truncation=]

2024-12-13 Thread Kim Barrett
On Fri, 13 Dec 2024 09:45:27 GMT, SendaoYan wrote: > Can you try with add configure options `--enable-asan --enable-ubsan` Does either asan or ubsan alone have this problem? I was under the impression that using both at the same time could lead to strange conflicts between them, but that's an

Re: JPackage does very weird things and it doesn't work with SNAP

2024-12-08 Thread Kim Barrett
On 12/8/24 1:56 PM, Davide Perini wrote: I tried attaching gdb, but it’s clear that jvm doesn’t like debuggers attached. Right from the start it got sigstop handlers invoked, and then a segfault in libjvm. I can't comment on the rest of your message, but I might be able to help with this. T

Re: RFR: 8343703: Symbol name cleanups after JEP 479 [v3]

2024-11-27 Thread Kim Barrett
On Thu, 28 Nov 2024 02:21:55 GMT, David Holmes wrote: >> After JEP 479 ([JDK-8339783](https://bugs.openjdk.org/browse/JDK-8339783) >> was integrated, the handling of certain symbol lookup code can be >> simplified. The old code needed to support 32-bit Windows, where names had a >> trailing `@

Re: RFR: 8343703: Symbol name cleanups after JEP 479 [v2]

2024-11-27 Thread Kim Barrett
On Wed, 27 Nov 2024 05:20:16 GMT, David Holmes wrote: >> After JEP 479 ([JDK-8339783](https://bugs.openjdk.org/browse/JDK-8339783) >> was integrated, the handling of certain symbol lookup code can be >> simplified. The old code needed to support 32-bit Windows, where names had a >> trailing `@

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v33]

2024-11-08 Thread Kim Barrett
On Fri, 8 Nov 2024 18:26:25 GMT, Saint Wesonga wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Inline buildJniFunctionName > > src/hotspot/os/posix/include/jvm_md.h line 41: > >> 39: >> 40: #define JNI_ONLO

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v33]

2024-11-08 Thread Kim Barrett
On Fri, 8 Nov 2024 11:31:40 GMT, Magnus Ihse Bursie wrote: >> This is the implementation of [JEP 479: _Remove the Windows 32-bit x86 >> Port_](https://openjdk.org/jeps/479). >> >> This is the summary of JEP 479: >>> Remove the source code and build support for the Windows 32-bit x86 port. >>>

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v31]

2024-11-07 Thread Kim Barrett
On Thu, 7 Nov 2024 12:16:23 GMT, Aleksey Shipilev wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove FIXME > > I really wish we did not mess with `_stdcall` and `_cdecl` in this PR. A > future me chasing

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v31]

2024-11-06 Thread Kim Barrett
On Wed, 6 Nov 2024 15:21:10 GMT, Magnus Ihse Bursie wrote: >> This is the implementation of [JEP 479: _Remove the Windows 32-bit x86 >> Port_](https://openjdk.org/jeps/479). >> >> This is the summary of JEP 479: >>> Remove the source code and build support for the Windows 32-bit x86 port. >>>

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v30]

2024-11-06 Thread Kim Barrett
On Wed, 6 Nov 2024 15:27:16 GMT, Magnus Ihse Bursie wrote: >> src/java.base/share/native/libjava/NativeLibraries.c line 67: >> >>> 65: strcat(jniEntryName, "_"); >>> 66: strcat(jniEntryName, cname); >>> 67: } >> >> I would prefer this be directly inlined at the sole call (in

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v30]

2024-11-05 Thread Kim Barrett
On Mon, 4 Nov 2024 20:42:59 GMT, Magnus Ihse Bursie wrote: >> This is the implementation of [JEP 479: _Remove the Windows 32-bit x86 >> Port_](https://openjdk.org/jeps/479). >> >> This is the summary of JEP 479: >>> Remove the source code and build support for the Windows 32-bit x86 port. >>>

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v17]

2024-11-04 Thread Kim Barrett
On Mon, 4 Nov 2024 09:11:16 GMT, David Holmes wrote: >> The deletion is apparently working, else we'd be getting build failures. So >> while there are some potential issues and opportunities for further cleanup >> in >> this file, I think they ought to be addressed separately from this PR. See >

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v17]

2024-11-04 Thread Kim Barrett
On Mon, 4 Nov 2024 02:34:13 GMT, David Holmes wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Remove superfluous check for 64-bit on Windows in >> MacroAssembler::call_clobbered_xmm_registers >> - Remove

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v17]

2024-11-01 Thread Kim Barrett
On Fri, 1 Nov 2024 16:04:55 GMT, Magnus Ihse Bursie wrote: >> This is the implementation of [JEP 479: _Remove the Windows 32-bit x86 >> Port_](https://openjdk.org/jeps/479). >> >> This is the summary of JEP 479: >>> Remove the source code and build support for the Windows 32-bit x86 port. >>>

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v13]

2024-10-29 Thread Kim Barrett
On Tue, 29 Oct 2024 09:51:16 GMT, Julian Waters wrote: >> src/hotspot/share/prims/jvm.cpp line 381: >> >>> 379: { >>> 380: #undef CSIZE >>> 381: #if defined(_LP64) >> >> Windows is actually LLP64 programming model not LP64. Does Windows x64 >> define _LP64 or is that something we do in our b

Re: RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v13]

2024-10-29 Thread Kim Barrett
On Tue, 29 Oct 2024 20:22:03 GMT, Magnus Ihse Bursie wrote: >> This is the implementation of [JEP 479: _Remove the Windows 32-bit x86 >> Port_](https://openjdk.org/jeps/479). >> >> This is the summary of JEP 479: >>> Remove the source code and build support for the Windows 32-bit x86 port. >>>

Re: RFR: 8340801: Disable ubsan checks in some awt/2d coding [v4]

2024-10-11 Thread Kim Barrett
On Wed, 9 Oct 2024 07:50:20 GMT, Matthias Baesken wrote: >> There is some old awt/2d coding where warnings occur when running with ubsan >> enabled binaries. >> However at most of these locations the coding should work (at least on our >> supported platform set) so the warnings can be disabled

Re: RFR: 8341722: Fix some warnings as errors when building on Linux with toolchain clang [v2]

2024-10-10 Thread Kim Barrett
On Wed, 9 Oct 2024 11:44:35 GMT, Matthias Baesken wrote: >> There are a few warnings as errors occurring when building on Linux with >> clang (clang15). Mostly these are some kind of 'unused' warnings. >> Might be related to https://bugs.openjdk.org/browse/JDK-8339156 . > > Matthias Baesken has

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v6]

2024-09-30 Thread Kim Barrett
On Mon, 30 Sep 2024 16:59:16 GMT, Aleksey Shipilev wrote: >> [JDK-8240696](https://bugs.openjdk.org/browse/JDK-8240696) added the native >> method for `Reference.clear`. The original patch skipped intrinsification of >> this method, because we thought `Reference.clear` is not on a performance

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v3]

2024-09-30 Thread Kim Barrett
On Mon, 30 Sep 2024 16:45:12 GMT, Aleksey Shipilev wrote: >> src/java.base/share/classes/java/lang/ref/Reference.java line 420: >> >>> 418: /* Implementation of clear(), also used by enqueue(). A simple >>> 419: * assignment of the referent field won't do for some garbage >>> 420:

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v3]

2024-09-27 Thread Kim Barrett
On Fri, 19 Jul 2024 15:52:14 GMT, Aleksey Shipilev wrote: >> [JDK-8240696](https://bugs.openjdk.org/browse/JDK-8240696) added the native >> method for `Reference.clear`. The original patch skipped intrinsification of >> this method, because we thought `Reference.clear` is not on a performance

Re: RFR: 8339687: Rearrange reachabilityFence()s in jdk.test.lib.util.ForceGC

2024-09-09 Thread Kim Barrett
On Mon, 9 Sep 2024 16:25:15 GMT, Stuart Marks wrote: >> @dholmes-ora Is this really possible? The `obj` ref is passed to the >> PhantomReference constructor, which stores it in a field, the constructed >> PhantomReference is returned, and it's then used in a reachabilityFence call >> below. So

Re: RFR: 8339687: Rearrange reachabilityFence()s in jdk.test.lib.util.ForceGC

2024-09-09 Thread Kim Barrett
On Fri, 6 Sep 2024 19:57:41 GMT, Brent Christian wrote: > From the bug description: > ForceGC would be improved by moving the Reference.reachabilityFence() calls > for 'obj' and 'ref'. > > Reference.reachabilityFence(obj) is currently placed after 'obj' has been set > to null, so effectively d

Re: RFR: 8339687: Rearrange reachabilityFence()s in jdk.test.lib.util.ForceGC

2024-09-09 Thread Kim Barrett
On Mon, 9 Sep 2024 14:40:12 GMT, Daniel Fuchs wrote: >> test/lib/jdk/test/lib/util/ForceGC.java line 102: >> >>> 100: } >>> 101: } >>> 102: Reference.reachabilityFence(ref); >> >> I think everything from the creation of ref to the line above needs to >> enclosed in

Re: RFR: 8339156: Use more fine-granular clang unused warnings

2024-08-29 Thread Kim Barrett
On Fri, 30 Aug 2024 04:37:55 GMT, Alan Bateman wrote: > I wonder if there should be JBS issues for the specific source files/warnings > so they can be looked at more closely (or maybe you've looked at them all > already). Just wondering about the maintainability of DISABLED_WARNINGS_xxx > valu

Re: RFR: 8339156: Use more fine-granular clang unused warnings

2024-08-29 Thread Kim Barrett
On Thu, 29 Aug 2024 13:14:35 GMT, Magnus Ihse Bursie wrote: > Currently, we issue -Wno-unused for all files in clang, which is a rather big > sledgehammer to get rid of some warnings that proliferate in a few areas of > the build. > > We should instead leave -Wunused turned on (as done by -Wal

Re: RFR: 8339120: Use more fine-granular gcc unused warnings [v6]

2024-08-28 Thread Kim Barrett
On Wed, 28 Aug 2024 15:15:03 GMT, Magnus Ihse Bursie wrote: >> Currently, we issue -Wno-unused for all files in gcc, which is a rather big >> sledgehammer to get rid of some warnings that proliferate in a few areas of >> the build. >> >> We should instead leave -Wunused turned on (as done by -

Re: RFR: 8339120: Use more fine-granular gcc unused warnings [v3]

2024-08-28 Thread Kim Barrett
On Wed, 28 Aug 2024 13:02:55 GMT, Magnus Ihse Bursie wrote: >> Currently, we issue -Wno-unused for all files in gcc, which is a rather big >> sledgehammer to get rid of some warnings that proliferate in a few areas of >> the build. >> >> We should instead leave -Wunused turned on (as done by -

Re: RFR: 8339120: Use more fine-granular gcc unused warnings

2024-08-27 Thread Kim Barrett
On Tue, 27 Aug 2024 20:04:21 GMT, Magnus Ihse Bursie wrote: > Currently, we issue -Wno-unused for all files in gcc, which is a rather big > sledgehammer to get rid of some warnings that proliferate in a few areas of > the build. > > We should instead leave -Wunused turned on (as done by -Wall)

Re: RFR: 8336289: Obliterate most references to _snprintf in the Windows JDK [v5]

2024-08-25 Thread Kim Barrett
On Sat, 24 Aug 2024 05:12:42 GMT, Julian Waters wrote: >> snprintf has been available for all officially and unofficially supported >> compilers for Windows, Visual Studio since version 2015 and gcc since, well, >> forever. snprintf is conforming to C99 since the start when compiling using >>

Re: RFR: 8336289: Obliterate most references to _snprintf in the Windows JDK

2024-08-01 Thread Kim Barrett
On Sat, 13 Jul 2024 05:34:00 GMT, Julian Waters wrote: >> snprintf has been available for all officially and unofficially supported >> compilers for Windows, Visual Studio since version 2015 and gcc since, well, >> forever. snprintf is conforming to C99 since the start when compiling using >>

Re: RFR: 8329597: C2: Intrinsify Reference.clear

2024-07-17 Thread Kim Barrett
On Mon, 15 Jul 2024 16:09:39 GMT, Kim Barrett wrote: > > Aw, nice usability landmine. I thought C2 barrier set would assert on me if > > it cannot deliver. Apparently not, [...] > > Reference.refersTo has similar issues. See refersToImpl and refersTo0 in both > Reference

Re: RFR: 8329597: C2: Intrinsify Reference.clear

2024-07-17 Thread Kim Barrett
On Fri, 12 Jul 2024 13:19:31 GMT, Aleksey Shipilev wrote: > > The runtime use of the Access API knows how to resolve an unknown oop ref > > strength using AccessBarrierSupport::resolve_unknown_oop_ref_strength. > > However, we do not have support for that in the C2 backend. In fact, it > > doe

Re: RFR: 8336289: Obliterate most references to _snprintf in the Windows JDK [v2]

2024-07-16 Thread Kim Barrett
On Tue, 16 Jul 2024 08:52:01 GMT, Julian Waters wrote: >> src/jdk.jdwp.agent/windows/native/libjdwp/util_md.h line 32: >> >>> 30: #include /* for _MAx_PATH */ >>> 31: >>> 32: typedef unsigned long long UNSIGNED_JLONG; >> >> This change has nothing to do with _sprintf. Not sure why it's

Re: RFR: 8336289: Obliterate most references to _snprintf in the Windows JDK [v3]

2024-07-16 Thread Kim Barrett
On Tue, 16 Jul 2024 08:59:20 GMT, Julian Waters wrote: >> snprintf has been available for all officially and unofficially supported >> compilers for Windows, Visual Studio since version 2015 and gcc since, well, >> forever. snprintf is conforming to C99 since the start when compiling using >>

Re: RFR: 8336289: Obliterate most references to _snprintf in the Windows JDK [v2]

2024-07-15 Thread Kim Barrett
On Sat, 13 Jul 2024 05:34:24 GMT, Julian Waters wrote: >> snprintf has been available for all officially and unofficially supported >> compilers for Windows, Visual Studio since version 2015 and gcc since, well, >> forever. snprintf is conforming to C99 since the start when compiling using >>

Re: RFR: 8336289: Obliterate most references to _snprintf in the Windows JDK

2024-07-12 Thread Kim Barrett
On Fri, 12 Jul 2024 06:29:34 GMT, Julian Waters wrote: > snprintf has been available for all officially and unofficially supported > compilers for Windows, Visual Studio since version 2015 and gcc since, well, > forever. snprintf is conforming to C99 since the start when compiling using > gcc,

Re: RFR: 8328812: Update and move siphash license

2024-03-25 Thread Kim Barrett
On Mon, 25 Mar 2024 20:10:45 GMT, Bernd wrote: >> To match the license claim in the code we are using: >> https://github.com/openjdk/jdk/blob/fb8f2a0a929ebe7f65c69741712b89bbb403ade9/src/hotspot/share/classfile/altHashing.cpp#L32-L43 > > The header file contains more claims > https://github.com/

Re: RFR: 8328812: Update and move siphash license

2024-03-25 Thread Kim Barrett
On Sun, 24 Mar 2024 17:24:49 GMT, Bernd wrote: >> Updated and moved the license file. > > src/hotspot/share/legal/siphash.md line 9: > >> 7:Copyright (c) 2012-2021 Jean-Philippe Aumasson >> 8: >> 9:Copyright (c) 2012-2014 Daniel J. Bernstein > > Why would you remove a author or yea

Integrated: 8324799: Use correct extension for C++ test headers

2024-02-28 Thread Kim Barrett
On Wed, 28 Feb 2024 01:18:50 GMT, Kim Barrett wrote: > Please review this change that renames some test .h files to .hpp. These > files contain C++ code and should be named accordingly. Some of them contain > uses of NULL, which we change to nullptr. > > The renamed files

Re: RFR: 8324799: Use correct extension for C++ test headers [v2]

2024-02-28 Thread Kim Barrett
On Thu, 29 Feb 2024 00:16:28 GMT, Kim Barrett wrote: >> Please review this change that renames some test .h files to .hpp. These >> files contain C++ code and should be named accordingly. Some of them contain >> uses of NULL, which we change to nullptr. >>

Re: RFR: 8324799: Use correct extension for C++ test headers [v2]

2024-02-28 Thread Kim Barrett
On Wed, 28 Feb 2024 14:15:25 GMT, Coleen Phillimore wrote: >> test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp >> line 26: >> >>> 24: #include >>> 25: #include >>> 26: #include >> >> Should this be in quotes rather than <> ? > > Suggestion: > > #i

Re: RFR: 8324799: Use correct extension for C++ test headers [v2]

2024-02-28 Thread Kim Barrett
On Wed, 28 Feb 2024 14:18:58 GMT, Coleen Phillimore wrote: >> Kim Barrett has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - add Oracle copyright >> - fix busted copyright text > > test/hotspot/jtreg

Re: RFR: 8324799: Use correct extension for C++ test headers [v2]

2024-02-28 Thread Kim Barrett
On Wed, 28 Feb 2024 04:06:08 GMT, Guoxiong Li wrote: >> Kim Barrett has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - add Oracle copyright >> - fix busted copyright text > > test/hotspot/jtreg/servi

Re: RFR: 8324799: Use correct extension for C++ test headers [v2]

2024-02-28 Thread Kim Barrett
On Wed, 28 Feb 2024 14:11:49 GMT, Coleen Phillimore wrote: >> test/hotspot/jtreg/serviceability/jvmti/events/SingleStep/singlestep01/libsinglestep01.cpp >> line 28: >> >>> 26: #include >>> 27: #include >>> 28: #include "jvmti_common.hpp" >> >> The copyright of this file is wrong. >> >>> *

Re: RFR: 8324799: Use correct extension for C++ test headers [v2]

2024-02-28 Thread Kim Barrett
ecently, > likely as part of JDK-8324681. > > Thus, the only "interesting" changes are to the renamed files. > > Testing: mach5 tier1 Kim Barrett has updated the pull request incrementally with two additional commits since the last revision: - add Oracle copyright - fix b

Re: RFR: 8324799: Use correct extension for C++ test headers

2024-02-27 Thread Kim Barrett
On Wed, 28 Feb 2024 06:12:17 GMT, Guoxiong Li wrote: > So large patch. In order to easy to review, it is good to separate such large > patch into several patches in the future. I was doing that in prior related changes (see the subtasks of JDK-8324799). But reviewers requested I batch up the

RFR: 8324799: Use correct extension for C++ test headers

2024-02-27 Thread Kim Barrett
Please review this change that renames some test .h files to .hpp. These files contain C++ code and should be named accordingly. Some of them contain uses of NULL, which we change to nullptr. The renamed files are: test/hotspot/jtreg/vmTestbase/nsk/share/aod/aod.h test/hotspot/jtreg/vmTestbase/

Re: RFR: 8325163: Enable -Wpedantic on clang [v2]

2024-02-16 Thread Kim Barrett
On Mon, 5 Feb 2024 21:39:06 GMT, Magnus Ihse Bursie wrote: > Here is the full list: > https://clang.llvm.org/docs/DiagnosticsReference.html#wpedantic I know about that list, and that's not what I was asking for. I want to understand the impact on *our* code. What warnings are arising from *our*

Re: RFR: 8325163: Enable -Wpedantic on clang [v2]

2024-02-05 Thread Kim Barrett
On Mon, 5 Feb 2024 16:19:59 GMT, Magnus Ihse Bursie wrote: > Is there anything in this proposed PR that you gentlemen disagree with or > object to? Or is this fine to push as a step in our ongoing pursuit of > increasing the code quality, that can (and will) be followed by many more? Yes. As

Re: RFR: 8325163: Enable -Wpedantic on clang

2024-02-05 Thread Kim Barrett
On Mon, 5 Feb 2024 15:42:40 GMT, Kim Barrett wrote: >>> I consider the "format '%p' expects argument of type 'void*" warnings to be >>> not at all helpful. Fortunately we don't use '%p' in HotSpot, >> >> We do use it in hots

Re: RFR: 8325163: Enable -Wpedantic on clang

2024-02-05 Thread Kim Barrett
On Mon, 5 Feb 2024 06:15:08 GMT, David Holmes wrote: > > I consider the "format '%p' expects argument of type 'void*" warnings to be > > not at all helpful. Fortunately we don't use '%p' in HotSpot, > > We do use it in hotspot. Not a huge amount as we have the legacy format > specifiers for PT

Re: RFR: 8325163: Enable -Wpedantic on clang

2024-02-05 Thread Kim Barrett
> On Feb 5, 2024, at 4:31 AM, Magnus Ihse Bursie wrote: > > On Mon, 5 Feb 2024 03:21:35 GMT, Kim Barrett wrote: > >>> Inspired by (the later backed-out) >>> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >>> enable `-Wpedantic`

Re: RFR: 8325163: Enable -Wpedantic on clang

2024-02-05 Thread Kim Barrett
On Mon, 5 Feb 2024 03:21:35 GMT, Kim Barrett wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like m

Re: RFR: 8325163: Enable -Wpedantic on clang

2024-02-04 Thread Kim Barrett
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import`

Re: RFR: 8325163: Enable -Wpedantic on clang

2024-02-04 Thread Kim Barrett
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > ``` > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is

  1   2   >