Re: RFR: 8330694: Rename 'HeapRegion' to 'G1HeapRegion' [v13]

2024-05-24 Thread Kim Barrett
On Fri, 24 May 2024 13:04:14 GMT, Lei Zaakjyu wrote: >> follow up 8267941 > > Lei Zaakjyu has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 10 commits: > > - review > - Merge branch 'master' of https://git.openjdk.org/jdk into

Re: RFR: 8326716: JVMTI spec: clarify what nullptr means for C/C++ developers [v2]

2024-05-16 Thread Kim Barrett
On Fri, 17 May 2024 02:00:29 GMT, David Holmes wrote: > But this clarification doesn't actually clarify that the rest of the spec > uses `nullptr`. Based on the proposed wording I would expect things like: > > ``` > The function may return nullptr > ``` > > to say > > ``` > The function may

Re: RFR: 8326716: JVMTI spec: clarify what nullptr means for C/C++ developers [v2]

2024-05-16 Thread Kim Barrett
On Fri, 17 May 2024 00:43:18 GMT, Serguei Spitsyn wrote: >> The following RFE was fixed recently: >> [8324680](https://bugs.openjdk.org/browse/JDK-8324680): Replace NULL with >> nullptr in JVMTI generated code >> >> It replaced all the `NULL`'s in the generated spec with`nullptr`. JVMTI >>

Re: RFR: 8326716: JVMTI spec: clarify what nullptr means for C/C++ developers

2024-05-16 Thread Kim Barrett
On Thu, 16 May 2024 02:37:40 GMT, Serguei Spitsyn wrote: > The following RFE was fixed recently: > [8324680](https://bugs.openjdk.org/browse/JDK-8324680): Replace NULL with > nullptr in JVMTI generated code > > It replaced all the `NULL`'s in the generated spec with`nullptr`. JVMTI > agents

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Kim Barrett
On Mon, 13 May 2024 04:55:24 GMT, Thomas Stuefe wrote: >> MEMFLAGS, as well as its enum constants, should live in its own include. >> >> The constants are used throughout the code base, often without needing the >> allocation APIs exposed through allocation.hpp. >> >> The MEMFLAGS enum def

Re: RFR: 8330694: Rename 'HeapRegion' to 'G1HeapRegion' [v4]

2024-04-29 Thread Kim Barrett
On Mon, 29 Apr 2024 08:14:03 GMT, Thomas Schatzl wrote: > mach5 higher tier SA tests are fine. What are your plans for the remaining SA > renames (would highly recommend to add) and the G1HeapRegion related helper > classes? I suggest the related helper classes be done in further followups,

Re: RFR: 8330694: Rename 'HeapRegion' to 'G1HeapRegion'

2024-04-19 Thread Kim Barrett
On Sat, 20 Apr 2024 02:04:20 GMT, Lei Zaakjyu wrote: > follow up 8267941 So much scrolling :) Looks good. Just a few very minor nits for which I don't need to re-review. src/hotspot/share/cds/archiveHeapWriter.cpp line 90: > 88: > 89: guarantee(UseG1GC, "implementation limitation"); >

Re: RFR: 8236736: Change notproduct JVM flags to develop flags [v4]

2024-04-02 Thread Kim Barrett
On Tue, 2 Apr 2024 19:51:03 GMT, Coleen Phillimore wrote: > Thanks for reviewing, Kim. Is your suggestion to not have a JVMFlag object > for develop flags in PRODUCT builds? Presumably to save some footprint? I'm > not sure we would win fighting the macros to accomplish this. Yes, that's the

Re: RFR: 8236736: Change notproduct JVM flags to develop flags [v3]

2024-04-02 Thread Kim Barrett
On Tue, 2 Apr 2024 17:25:12 GMT, Coleen Phillimore wrote: >> Remove the notproduct distinction for command line options, rather than >> trying to wrestle the macros to fix the bug that they've been treated as >> develop options for some time now. This simplifies the command line option >>

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. >&g

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: > >

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
ly, > 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 busted

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

Integrated: 8326524: Rename agent_common.h

2024-02-27 Thread Kim Barrett
On Thu, 22 Feb 2024 19:38:26 GMT, Kim Barrett wrote: > Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/agent_common/agent_common.h to > agent_common.hpp. > > The #include updates were performed mechanically, and bu

Re: RFR: 8326524: Rename agent_common.h

2024-02-27 Thread Kim Barrett
On Thu, 22 Feb 2024 19:38:26 GMT, Kim Barrett wrote: > Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/agent_common/agent_common.h to > agent_common.hpp. > > The #include updates were performed mechanically, and bu

Re: RFR: 8326524: Rename agent_common.h

2024-02-23 Thread Kim Barrett
On Fri, 23 Feb 2024 06:11:29 GMT, Julian Waters wrote: > > Please review this trivial change > > This doesn't look trivial at all :P While there are a lot of files being touched, the changes to all but one are purely mechanical, no thinking required. - PR Comment:

Re: RFR: 8326524: Rename agent_common.h

2024-02-22 Thread Kim Barrett
On Thu, 22 Feb 2024 19:38:26 GMT, Kim Barrett wrote: > Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/agent_common/agent_common.h to > agent_common.hpp. > > The #include updates were performed mechanically, and bu

Re: RFR: 8326524: Rename agent_common.h

2024-02-22 Thread Kim Barrett
On Thu, 22 Feb 2024 20:06:50 GMT, Coleen Phillimore wrote: >> Please review this trivial change that renames the file >> test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/agent_common/agent_common.h to >> agent_common.hpp. >> >> The #include updates were performed mechanically, and builds would

Re: RFR: 8326524: Rename agent_common.h

2024-02-22 Thread Kim Barrett
On Fri, 23 Feb 2024 02:07:02 GMT, Dean Long wrote: > If we wanted to minimize changes, we could have agent_common.h include > agent_common.hpp. Then we don't have to change all the .cpp files, which have > other problems, like the includes not being sorted. The purpose of this exercise is to

Re: RFR: 8326524: Rename agent_common.h

2024-02-22 Thread Kim Barrett
On Fri, 23 Feb 2024 00:36:43 GMT, Serguei Spitsyn wrote: > Looks good. I have a minor concern about the Copyright headers in 547 files. > What is the ways to make sure they are updated? Here's the bash script I used: #!/usr/bin/env bash for f in `git diff master --name-only`; do sed -i \

RFR: 8326524: Rename agent_common.h

2024-02-22 Thread Kim Barrett
Please review this trivial change that renames the file test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/agent_common/agent_common.h to agent_common.hpp. The #include updates were performed mechanically, and builds would fail if there were mistakes. The copyright updates were similarly performed

Re: RFR: 8326090: Rename jvmti_aod.h [v2]

2024-02-20 Thread Kim Barrett
re were mistakes. All of the including files have already had their > copyrights updated recently, as part of JDK-8324681. So the only interesting > (for some minimal value of "interesting") changes are in the renamed file. > > Testing: mach5 tier1 Kim Barrett has updated the pull re

Integrated: 8326090: Rename jvmti_aod.h

2024-02-20 Thread Kim Barrett
On Fri, 16 Feb 2024 21:42:18 GMT, Kim Barrett wrote: > Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/aod/jvmti_aod.h to > jvmti_aod.hpp, > and replace uses of NULL in the file. > > The #include updates were perf

Re: RFR: 8326090: Rename jvmti_aod.h [v2]

2024-02-20 Thread Kim Barrett
On Tue, 20 Feb 2024 15:15:35 GMT, Coleen Phillimore wrote: >> Kim Barrett 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 contain

RFR: 8326090: Rename jvmti_aod.h

2024-02-16 Thread Kim Barrett
Please review this trivial change that renames the file test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/aod/jvmti_aod.h to jvmti_aod.hpp, and replace uses of NULL in the file. The #include updates were performed mechanically, and build would fail if there were mistakes. All of the including files

Re: RFR: 8252136: Several methods in hotspot are missing "static" [v2]

2024-02-14 Thread Kim Barrett
On Tue, 13 Feb 2024 10:29:48 GMT, Magnus Ihse Bursie wrote: >> src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleWriter.cpp line >> 202: >> >>> 200: static RootDescriptionInfo* root_infos = nullptr; >>> 201: >>> 202: static int __write_sample_info__(JfrCheckpointWriter* writer, const

Re: RFR: 8252136: Several methods in hotspot are missing "static" [v2]

2024-02-14 Thread Kim Barrett
On Tue, 13 Feb 2024 11:05:30 GMT, Magnus Ihse Bursie wrote: >> There are several places in hotspot where an internal function should have >> been declared static, but isn't. >> >> These were discovered by trying to use the gcc option >> `-Wmissing-declarations` and the corresponding clang

Re: RFR: 8252136: Several methods in hotspot are missing "static"

2024-02-12 Thread Kim Barrett
On Mon, 12 Feb 2024 12:43:09 GMT, Magnus Ihse Bursie wrote: > There are several places in hotspot where an internal function should have > been declared static, but isn't. > > These were discovered by trying to use the gcc option > `-Wmissing-declarations` and the corresponding clang option

Re: RFR: 8325367: Rename nsk_list.h

2024-02-07 Thread Kim Barrett
On Wed, 7 Feb 2024 22:01:13 GMT, Coleen Phillimore wrote: >> Please review this trivial change that renames the file >> test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_list.h to nsk_list.hpp. >> >> Testing: mach5 tier1 >> Note that build would fail if #include updates were incorrect or

Integrated: 8325367: Rename nsk_list.h

2024-02-07 Thread Kim Barrett
On Wed, 7 Feb 2024 20:48:18 GMT, Kim Barrett wrote: > Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_list.h to nsk_list.hpp. > > Testing: mach5 tier1 > Note that build would fail if #include updates were incorrec

RFR: 8325367: Rename nsk_list.h

2024-02-07 Thread Kim Barrett
Please review this trivial change that renames the file test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_list.h to nsk_list.hpp. Testing: mach5 tier1 Note that build would fail if #include updates were incorrect or incomplete. - Commit messages: - rename nsk_list.h Changes:

Re: RFR: 8325347: Rename native_thread.h

2024-02-06 Thread Kim Barrett
On Tue, 6 Feb 2024 22:30:18 GMT, Coleen Phillimore wrote: >> Please review this trivial change that renames the file >> test/hotspot/jtreg/vmTestbase/nsk/share/native/native_thread.h >> to native_thread.hpp. and replaces uses of NULL in that file. >> >> Testing: mach5 tier1 > > Agree this looks

Integrated: 8325347: Rename native_thread.h

2024-02-06 Thread Kim Barrett
On Tue, 6 Feb 2024 22:08:18 GMT, Kim Barrett wrote: > Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/native/native_thread.h > to native_thread.hpp. and replaces uses of NULL in that file. > > Testing: mach5 tier1 This pull reque

RFR: 8325347: Rename native_thread.h

2024-02-06 Thread Kim Barrett
Please review this trivial change that renames the file test/hotspot/jtreg/vmTestbase/nsk/share/native/native_thread.h to native_thread.hpp. and replaces uses of NULL in that file. Testing: mach5 tier1 - Commit messages: - rename NULLs in native_thread.hpp - rename native_thread.h

Integrated: 8325180: Rename jvmti_FollowRefObjects.h

2024-02-06 Thread Kim Barrett
On Fri, 2 Feb 2024 16:34:19 GMT, Kim Barrett wrote: > Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_FollowRefObjects.h > to jvmti_FollowRefObjects.hpp, and replaces uses of NULL in the file. > > Testing: mach5 t

Re: RFR: 8325180: Rename jvmti_FollowRefObjects.h [v2]

2024-02-06 Thread Kim Barrett
> Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_FollowRefObjects.h > to jvmti_FollowRefObjects.hpp, and replaces uses of NULL in the file. > > Testing: mach5 tier1 Kim Barrett has updated the pull request with a new

Re: RFR: 8325180: Rename jvmti_FollowRefObjects.h [v2]

2024-02-06 Thread Kim Barrett
On Fri, 2 Feb 2024 17:49:22 GMT, Serguei Spitsyn wrote: >> Kim Barrett 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 contain

RFR: 8325180: Rename jvmti_FollowRefObjects.h

2024-02-02 Thread Kim Barrett
Please review this trivial change that renames the file test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_FollowRefObjects.h to jvmti_FollowRefObjects.hpp, and replaces uses of NULL in the file. Testing: mach5 tier1 - Commit messages: - rename NULLs in jvmti_FollwRefObjects.hpp

Integrated: 8325055: Rename Injector.h

2024-02-02 Thread Kim Barrett
On Wed, 31 Jan 2024 15:08:14 GMT, Kim Barrett wrote: > Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/Injector.h to Injector.hpp. > > Testing: mach5 tier1 This pull request has now been integrated. Changeset: 6787c4c3 Au

Re: RFR: 8325055: Rename Injector.h [v3]

2024-02-02 Thread Kim Barrett
On Thu, 1 Feb 2024 07:12:08 GMT, David Holmes wrote: >> Kim Barrett 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 three addi

Re: RFR: 8325055: Rename Injector.h [v3]

2024-02-02 Thread Kim Barrett
> Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/Injector.h to Injector.hpp. > > Testing: mach5 tier1 Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The incremental web

Re: RFR: 8325055: Rename Injector.h [v2]

2024-02-01 Thread Kim Barrett
On Thu, 1 Feb 2024 07:12:08 GMT, David Holmes wrote: > Seems fine. Thanks Trivial? - PR Comment: https://git.openjdk.org/jdk/pull/17656#issuecomment-1922036034

Re: RFR: 8325055: Rename Injector.h [v2]

2024-01-31 Thread Kim Barrett
> Please review this trivial change that renames the file > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/Injector.h to Injector.hpp. > > Testing: mach5 tier1 Kim Barrett has updated the pull request incrementally with one additional commit since the last revision: fix na

RFR: 8325055: Rename Injector.h

2024-01-31 Thread Kim Barrett
Please review this trivial change that renames the file test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/Injector.h to Injector.hpp. Testing: mach5 tier1 - Commit messages: - rename Injector.h Changes: https://git.openjdk.org/jdk/pull/17656/files Webrev:

Integrated: 8324880: Rename get_stack_trace.h

2024-01-30 Thread Kim Barrett
On Tue, 30 Jan 2024 03:43:15 GMT, Kim Barrett wrote: > Please review this trivial change that renames the file > test/hotspot/jtreg/serviceability/jvmti/thread/GetStackTrace/get_stack_trace.h > to get_stack_trace.hpp, and renames uses of NULL in that file. > > The updates to

Re: RFR: 8324880: Rename get_stack_trace.h

2024-01-30 Thread Kim Barrett
On Tue, 30 Jan 2024 07:40:05 GMT, David Holmes wrote: >> Please review this trivial change that renames the file >> test/hotspot/jtreg/serviceability/jvmti/thread/GetStackTrace/get_stack_trace.h >> to get_stack_trace.hpp, and renames uses of NULL in that file. >> >> The updates to #include

RFR: 8324880: Rename get_stack_trace.h

2024-01-29 Thread Kim Barrett
Please review this trivial change that renames the file test/hotspot/jtreg/serviceability/jvmti/thread/GetStackTrace/get_stack_trace.h to get_stack_trace.hpp, and renames uses of NULL in that file. The updates to #include lines in using files were performed mechanically. Testing: mach5 tier1

Re: RFR: 8324681: Replace NULL with nullptr in HotSpot jtreg test native code files [v4]

2024-01-28 Thread Kim Barrett
On Sat, 27 Jan 2024 18:24:45 GMT, Coleen Phillimore wrote: >> This mechanically replaces NULL with nullptr in hpp/cpp native files in test >> native code. This didn't attempt to change NULL in comments to say null >> because nullptr is generally the right thing for the comment to say. It >>

Re: RFR: 8309044: Replace NULL with nullptr, final sweep of hotspot code [v2]

2024-01-22 Thread Kim Barrett
On Tue, 30 May 2023 19:15:38 GMT, Johan Sjölen wrote: >> A final sweep of Hotspot to remove all re-added NULLs. With only 110 changes >> I'd appreciate if this was considered trivial. > > Johan Sjölen has updated the pull request incrementally with two additional > commits since the last

Re: RFR: 8323660: Serial: Fix header ordering and indentation [v4]

2024-01-13 Thread Kim Barrett
On Sun, 14 Jan 2024 01:33:45 GMT, Lei Zaakjyu wrote: >> follow up 8234502. > > Lei Zaakjyu has updated the pull request incrementally with one additional > commit since the last revision: > > fix Marked as reviewed by kbarrett (Reviewer). - PR Review:

Re: RFR: 8323660: Serial: Fix header ordering and indentation [v3]

2024-01-13 Thread Kim Barrett
On Fri, 12 Jan 2024 15:26:34 GMT, Lei Zaakjyu wrote: >> follow up 8234502. > > Lei Zaakjyu has updated the pull request incrementally with one additional > commit since the last revision: > > fix Changes requested by kbarrett (Reviewer). src/hotspot/share/gc/serial/serialHeap.cpp line 105:

Re: RFR: 8322476: Remove GrowableArray C-Heap version, replace usages with GrowableArrayCHeap

2023-12-22 Thread Kim Barrett
[Kim Barret wrote:] >>> pre-existing: There are a lot of non-static class data members that are >>> pointers to >>> GrowableArray that seem like they would be better as direct, e.g. >>> non-pointers. >>> >>> pre-existing: There are a lot of iterations over GrowableArray's that would >>> be >>>

Re: RFR: 8322476: Remove GrowableArray C-Heap version, replace usages with GrowableArrayCHeap

2023-12-21 Thread Kim Barrett
On Thu, 21 Dec 2023 06:11:03 GMT, Emanuel Peter wrote: >> src/hotspot/share/memory/arena.hpp line 209: >> >>> 207: >>> 208: #ifdef ASSERT >>> 209: bool Arena_contains(const Arena* arena, const void* ptr); >> >> This function doesn't seem necessary. Directly calling arena->contains(ptr) >>

Re: RFR: 8322476: Remove GrowableArray C-Heap version, replace usages with GrowableArrayCHeap

2023-12-21 Thread Kim Barrett
On Thu, 21 Dec 2023 11:10:43 GMT, Johan Sjölen wrote: > ... I also asked for some "length"/"size" naming to be changed to "capacity", > you don't have to do this as it's pre-existing, but it would make that code > clearer. I think I only commented on one in my pass over the code, but I agree

Re: RFR: 8322476: Remove GrowableArray C-Heap version, replace usages with GrowableArrayCHeap

2023-12-20 Thread Kim Barrett
On Wed, 20 Dec 2023 19:37:52 GMT, Kim Barrett wrote: >> [JDK-8247755](https://bugs.openjdk.org/browse/JDK-8247755) introduced the >> `GrowableArrayCHeap`. This duplicates the current C-Heap allocation >> capability in `GrowableArray`. I now remove that from `GrowableArray

Re: RFR: 8322476: Remove GrowableArray C-Heap version, replace usages with GrowableArrayCHeap

2023-12-20 Thread Kim Barrett
On Tue, 19 Dec 2023 16:59:05 GMT, Emanuel Peter wrote: > [JDK-8247755](https://bugs.openjdk.org/browse/JDK-8247755) introduced the > `GrowableArrayCHeap`. This duplicates the current C-Heap allocation > capability in `GrowableArray`. I now remove that from `GrowableArray` and > move all

Re: RFR: 8319115: GrowableArray: Do not initialize up to capacity

2023-12-18 Thread Kim Barrett
On Fri, 1 Dec 2023 07:56:04 GMT, Emanuel Peter wrote: > Before this patch, we always initialized the GrowableArray up to its > `capacity`, and not just up to `length`. This is problematic for a few > reasons: > > - It is not expected. `std::vector` also only initializes the elements up to >

Re: RFR: 8319115: GrowableArray: Do not initialize up to capacity

2023-12-18 Thread Kim Barrett
On Mon, 18 Dec 2023 07:49:04 GMT, David Holmes wrote: >> Before this patch, we always initialized the GrowableArray up to its >> `capacity`, and not just up to `length`. This is problematic for a few >> reasons: >> >> - It is not expected. `std::vector` also only initializes the elements up

Re: RFR: 8314502: Change the comparator taking version of GrowableArray::find to be a template method [v7]

2023-10-30 Thread Kim Barrett
On Tue, 31 Oct 2023 00:57:02 GMT, John R Rose wrote: >> Using a reference here leads to unnecessary overhead when `E` is small and >> trivially copyable, unless the predicate function is inlined. Pass by value >> is >> better in that case. Of course, as noted above, if `E` is "expensive" to

Re: RFR: 8314502: Change the comparator taking version of GrowableArray::find to be a template method [v3]

2023-10-29 Thread Kim Barrett
On Tue, 29 Aug 2023 09:29:56 GMT, Johan Sjölen wrote: > I still approve of this patch as it's better than what we had before. There > are a lot of suggested improvements that can be done either in this PR or in > a future RFE. `git blame` shows that this hasn't been touched since 2008, so > I

Re: RFR: 8314502: Change the comparator taking version of GrowableArray::find to be a template method [v7]

2023-10-29 Thread Kim Barrett
On Tue, 24 Oct 2023 10:48:01 GMT, Afshin Zafari wrote: >> The `find` method now is >> ```C++ >> template >> int find(T* token, bool f(T*, E)) const { >> ... >> >> Any other functions which use this are also changed. >> Local linux-x64-debug hotspot:tier1 passed. Mach5 tier1 build on linux and

Re: RFR: 8314502: Change the comparator taking version of GrowableArray::find to be a template method [v7]

2023-10-29 Thread Kim Barrett
On Sat, 28 Oct 2023 15:27:20 GMT, Quan Anh Mai wrote: >> Sorry I was unclear: what is the advantage of a reference here? Is it just >> to avoid copying > > @dholmes-ora Yes it helps avoid copying, especially if the copy constructor > is non-trivial. And I think it is more idiomatic in C++ to

Re: RFR: 8309599: WeakHandle and OopHandle release should clear obj pointer

2023-09-27 Thread Kim Barrett
On Tue, 26 Sep 2023 12:47:42 GMT, Coleen Phillimore wrote: > This change makes WeakHandle and OopHandle release null out the obj pointer, > at the cost of making the release function non-const and some changes that > propagated from that. This enables ObjectMonitor code to test for null to >

Re: Integrated: 8316695: ProblemList serviceability/jvmti/RedefineClasses/RedefineLeakThrowable.java

2023-09-21 Thread Kim Barrett
On Thu, 21 Sep 2023 20:39:32 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList > serviceability/jvmti/RedefineClasses/RedefineLeakThrowable.java > on all platforms. It's a new test and we already have 8 failure sightings in > the JDK22 CI. Looks good. - Marked as

Re: RFR: 8314502: Change the comparator taking version of GrowableArray::find to be a template method [v2]

2023-08-27 Thread Kim Barrett
On Sat, 26 Aug 2023 18:48:22 GMT, Kim Barrett wrote: >> Updated in-place > > Not a review, just agreeing with @stefank and @jdksjolen . What they > describe is idiomatic C++. Also, why isn't this change also being applied to `find_from_end` - PR Review

Re: RFR: 8314502: Change the comparator taking version of GrowableArray::find to be a template method [v2]

2023-08-26 Thread Kim Barrett
On Fri, 25 Aug 2023 11:02:24 GMT, Stefan Karlsson wrote: >> We could just as well do a capturing lambda here, yes. Then we'd have: >> >> ```c++ >> template >> int find(F finder); >> >> >> It'd be a template instead of function pointer since it's a capturing lambda >> and `std::function` is

Integrated: 8314694: Separate checked_cast from globalDefinitions.hpp

2023-08-23 Thread Kim Barrett
On Tue, 22 Aug 2023 04:13:13 GMT, Kim Barrett wrote: > Please review this change which moves checked_cast from globalDefinitions.hpp > to a separate file. As part of this change we modify files that use > checked_cast to directly include that new file. There are around 80 suc

Re: RFR: 8314694: Separate checked_cast from globalDefinitions.hpp [v3]

2023-08-23 Thread Kim Barrett
o the hotspot directory: > > > egrep -r --files-with-matches --exclude-dir=.git " checked_cast<" . | \ > xargs egrep --files-without-match "utilities/checkedCast.hpp" > > > So perhaps this change is trivial, despite the number of files. > > Testing:

Re: RFR: 8314694: Separate checked_cast from globalDefinitions.hpp [v2]

2023-08-22 Thread Kim Barrett
o the hotspot directory: > > > egrep -r --files-with-matches --exclude-dir=.git " checked_cast<" . | \ > xargs egrep --files-without-match "utilities/checkedCast.hpp" > > > So perhaps this change is trivial, despite the number of files. > > Testing:

Re: RFR: 8314694: Separate checked_cast from globalDefinitions.hpp

2023-08-22 Thread Kim Barrett
On Tue, 22 Aug 2023 06:54:20 GMT, Thomas Stuefe wrote: >> Please review this change which moves checked_cast from globalDefinitions.hpp >> to a separate file. As part of this change we modify files that use >> checked_cast to directly include that new file. There are around 80 such >> files,

RFR: 8314694: Separate checked_cast from globalDefinitions.hpp

2023-08-21 Thread Kim Barrett
Please review this change which moves checked_cast from globalDefinitions.hpp to a separate file. As part of this change we modify files that use checked_cast to directly include that new file. There are around 80 such files, and that change constitutes the majority of the changed files and

Re: RFR: 8310948: Fix ignored-qualifiers warning in Hotspot

2023-06-28 Thread Kim Barrett
On Tue, 27 Jun 2023 12:22:43 GMT, Daniel Jeliński wrote: > Please review this attempt to fix ignored-qualifiers warning. > > Example warnings: > > src/hotspot/share/oops/method.hpp:413:19: warning: 'volatile' type qualifier > on return type has no effect [-Wignored-qualifiers] >

Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code [v2]

2023-05-30 Thread Kim Barrett
On Sat, 27 May 2023 15:33:37 GMT, Kim Barrett wrote: >> I am basically worried that undefining malloc, even if it seems harmless >> now, exposes us to difficult-to-investigate problems down the road, since it >> depends on how the libc devs will reform those macros in t

Re: RFR: 8308286 Fix clang warnings in linux code

2023-05-27 Thread Kim Barrett
On Wed, 17 May 2023 12:28:47 GMT, Artem Semenov wrote: > When using the clang compiler to build OpenJDk on Linux, we encounter various > "warnings as errors". > They can be fixed with small changes. All of the -Wformat-nonliteral changes make me wonder why we're seeing these with clang but not

Re: RFR: 8308286 Fix clang warnings in linux code

2023-05-27 Thread Kim Barrett
On Fri, 26 May 2023 07:48:06 GMT, Daniel Jeliński wrote: > According to our docs, [clang is a supported compiler for > Linux](https://github.com/openjdk/jdk/blob/master/doc/building.md#native-compiler-toolchain-requirements). I think that's aspirational rather than actual. Clang has been

Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code [v2]

2023-05-27 Thread Kim Barrett
On Sat, 27 May 2023 11:50:11 GMT, Thomas Stuefe wrote: >>> This one is not just to get rid of a warning. We get real test errors >>> because malloc gets replaced by vec_malloc in log tags. >> >> That does not invalidate my argument, nor does it answer my question. Those >> test errors could

Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code

2023-05-25 Thread Kim Barrett
On Thu, 25 May 2023 09:14:14 GMT, JoKern65 wrote: > When using the new xlc17 compiler (based on a recent clang) to build OpenJDk > on AIX , we run into various "warnings as errors". > Some of those are in shared codebase and could be addressed by small > adjustments. > A lot of those changes

Re: RFR: 8307058: Implementation of Generational ZGC [v10]

2023-05-09 Thread Kim Barrett
On Tue, 9 May 2023 06:06:11 GMT, Stefan Karlsson wrote: >> Hi all, >> >> Please review the implementation of Generational ZGC, which can be turned on >> by adding -XX:+ZGenerational in addition to using -XX:+UseZGC. Generational >> ZGC is a major rewrite of the non-generational ZGC version

Re: RFR: 8306836: Remove pinned tag for G1 heap regions [v6]

2023-05-03 Thread Kim Barrett
On Tue, 2 May 2023 16:47:06 GMT, Thomas Schatzl wrote: >> Hi all, >> >> please review this change that removes the pinned tag from `HeapRegion`. >> >> So that "pinned" tag for G1 heap regions indicates that the region should >> not move during (young) gc. This applies to now removed archive

Re: RFR: 8306836: Remove pinned tag for G1 heap regions [v4]

2023-05-03 Thread Kim Barrett
On Thu, 27 Apr 2023 12:31:24 GMT, Thomas Schatzl wrote: >> Hi all, >> >> please review this change that removes the pinned tag from `HeapRegion`. >> >> So that "pinned" tag for G1 heap regions indicates that the region should >> not move during (young) gc. This applies to now removed

Integrated: 8305566: Change StringDedup thread to derive from JavaThread

2023-04-27 Thread Kim Barrett
On Mon, 24 Apr 2023 08:24:53 GMT, Kim Barrett wrote: > Please review this change to the string deduplication thread to make it a kind > of JavaThread rather than a ConcurrentGCThread. There are several pieces to > this change: > > (1) New class StringDedupThread (derived

Re: RFR: 8305566: Change StringDedup thread to derive from JavaThread [v3]

2023-04-27 Thread Kim Barrett
1-3 with -XX:+UseStringDeduplication. > The test runtime/cds/DeterministicDump.java fails intermittently with that > option, which is not surprising - see JDK-8306712. > > I was never able to reproduce the failure; it's likely quite timing sensitive. > The fix of changing the type is ba

Re: RFR: 8305566: Change StringDedup thread to derive from JavaThread [v2]

2023-04-27 Thread Kim Barrett
On Mon, 24 Apr 2023 09:25:01 GMT, Kim Barrett wrote: >> Please review this change to the string deduplication thread to make it a >> kind >> of JavaThread rather than a ConcurrentGCThread. There are several pieces to >> this change: >> >> (1) New c

Re: RFR: 8306858: Remove some remnants of CMS from SA agent

2023-04-25 Thread Kim Barrett
On Tue, 25 Apr 2023 16:25:40 GMT, Thomas Schatzl wrote: > Hi all, > > please review this change that removes some remaining CMS related stuff. > > Testing: tier1-3, gha > > Thanks, > Thomas Looks good. - Marked as reviewed by kbarrett (Reviewer). PR Review:

Re: RFR: 8305566: ZGC: gc/stringdedup/TestStringDeduplicationFullGC.java#Z failed with SIGSEGV in ZBarrier::weak_load_barrier_on_phantom_oop_slow_path [v2]

2023-04-24 Thread Kim Barrett
1-3 with -XX:+UseStringDeduplication. > The test runtime/cds/DeterministicDump.java fails intermittently with that > option, which is not surprising - see JDK-8306712. > > I was never able to reproduce the failure; it's likely quite timing sensitive. > The fix of changing the type is ba

Re: RFR: 8305566: ZGC: gc/stringdedup/TestStringDeduplicationFullGC.java#Z failed with SIGSEGV in ZBarrier::weak_load_barrier_on_phantom_oop_slow_path [v2]

2023-04-24 Thread Kim Barrett
On Mon, 24 Apr 2023 09:00:53 GMT, Stefan Karlsson wrote: >> Kim Barrett has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix include order > > src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor

RFR: 8305566: ZGC: gc/stringdedup/TestStringDeduplicationFullGC.java#Z failed with SIGSEGV in ZBarrier::weak_load_barrier_on_phantom_oop_slow_path

2023-04-24 Thread Kim Barrett
Please review this change to the string deduplication thread to make it a kind of JavaThread rather than a ConcurrentGCThread. There are several pieces to this change: (1) New class StringDedupThread (derived from JavaThread), separate from StringDedup::Processor (which is now just a CHeapObj

Re: RFR: 8305590: Remove nothrow exception specifications from operator new [v3]

2023-04-23 Thread Kim Barrett
On Sun, 23 Apr 2023 18:31:57 GMT, Julian Waters wrote: > I believe this may have missed removing the exception specifier from an > operator new inside AnyObj, allocation.cpp, since gcc 12 and up on my end now > refuses to compile HotSpot with this change. I'll create a cleanup change for >

Re: RFR: 8305590: Remove nothrow exception specifications from operator new [v3]

2023-04-20 Thread Kim Barrett
On Thu, 20 Apr 2023 08:41:58 GMT, Afshin Zafari wrote: >> - The `throw()` (i.e., no throw) specifications are removed from the >> instances of `operator new` where _do not_ return `nullptr`. >> >> - The `-fcheck-new` is removed from the gcc compile flags. >> >> - The `operator new` and

Re: RFR: 8305590: Remove nothrow exception specifications from operator new [v2]

2023-04-19 Thread Kim Barrett
On Wed, 19 Apr 2023 14:46:48 GMT, Afshin Zafari wrote: >> src/hotspot/share/prims/jvmtiRawMonitor.hpp line 114: >> >>> 112: >>> 113: // Non-aborting operator new >>> 114: void* operator new(size_t size, const std::nothrow_t& >>> nothrow_constant) throw() { >> >> Hm, now I'm wondering

Re: RFR: 8305590: Remove nothrow exception specifications from operator new [v2]

2023-04-19 Thread Kim Barrett
On Wed, 19 Apr 2023 10:25:49 GMT, Afshin Zafari wrote: >> - The `throw()` (i.e., no throw) specifications are removed from the >> instances of `operator new` where _do not_ return `nullptr`. >> >> - The `-fcheck-new` is removed from the gcc compile flags. >> >> - The `operator new` and

Re: RFR: 8305590: Remove nothrow exception specifications from operator new

2023-04-18 Thread Kim Barrett
On Mon, 17 Apr 2023 17:09:44 GMT, Afshin Zafari wrote: > - The `throw()` (i.e., no throw) specifications are removed from the > instances of `operator new` where _do not_ return `nullptr`. > > - The `-fcheck-new` is removed from the gcc compile flags. > > - The `operator new` and `operator

Re: RFR: 8305590: Remove nothrow exception specifications from operator new

2023-04-18 Thread Kim Barrett
On Tue, 18 Apr 2023 15:18:34 GMT, Coleen Phillimore wrote: >> - The `throw()` (i.e., no throw) specifications are removed from the >> instances of `operator new` where _do not_ return `nullptr`. >> >> - The `-fcheck-new` is removed from the gcc compile flags. >> >> - The `operator new` and

Re: RFR: 8305936: JavaThread::create_system_thread_object has unused is_visible argument

2023-04-13 Thread Kim Barrett
On Thu, 13 Apr 2023 05:41:31 GMT, David Holmes wrote: > Please review this simple cleanup of an unused parameter in > `create_system_thread_object`. Details are in JBS. > > Testing: tiers 1-3 > > Thanks. Looks good. - Marked as reviewed by kbarrett (Reviewer). PR Review:

Re: RFR: 8305341: Alignment should be enforced by alignas instead of compiler specific attributes [v3]

2023-04-11 Thread Kim Barrett
On Sat, 8 Apr 2023 13:24:37 GMT, Julian Waters wrote: >> C11 has been stable for a long time on all platforms, so native code can use >> the standard alignas operator for alignment requirements > > Julian Waters has updated the pull request incrementally with one additional > commit since the

  1   2   >