On Tue, 14 May 2024 07:19:32 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
On Mon, 13 May 2024 15:36:13 GMT, Stefan Karlsson wrote:
>> To quote @robehn - Why write a comment for a rule if you can enforce it with
>> code instead...
>
> I tend to agree with that. My earlier question still stands: is there a
> better place to put it? Right now th
On Mon, 13 May 2024 15:26:18 GMT, Daniel D. Daugherty
wrote:
>> Could you instead put the static_assert near the code that will break? Right
>> now it looks obscure and weird to have this check when it is obviously
>> correct as long as no one changes the definition. Would it be enough to
>>
On Mon, 13 May 2024 14:31:22 GMT, Thomas Stuefe wrote:
>> src/hotspot/share/nmt/memflags.cpp line 31:
>>
>>> 29:
>>> 30: // Extra insurance that MEMFLAGS truly has the same size as uint8_t.
>>> 31: STATIC_ASSERT(sizeof(MEMFLAGS) == sizeof(uint8_t));
>>
>> I think you can remove this entire
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
On Thu, 2 May 2024 17:23:21 GMT, Aleksey Shipilev wrote:
>> src/hotspot/share/gc/parallel/psParallelCompact.cpp line 1270:
>>
>>> 1268:
>>> 1269: ParallelScavengeHeap* heap = ParallelScavengeHeap::heap();
>>> 1270: assert(!heap->is_stw_gc_active(), "not reentrant");
>>
>> While reading
On Thu, 2 May 2024 14:40:35 GMT, Aleksey Shipilev wrote:
> `CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ GC
> phase is running, while it actually only covers the STW GCs. It would be good
> to rename it for clarity. The freed-up name, `is_gc_active` could then be
On Thu, 2 May 2024 14:40:35 GMT, Aleksey Shipilev wrote:
> `CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ GC
> phase is running, while it actually only covers the STW GCs. It would be good
> to rename it for clarity. The freed-up name, `is_gc_active` could then be
On Fri, 5 Apr 2024 12:32:30 GMT, Stefan Karlsson wrote:
> The GCs scan and handles nmethods and ignores CodeBlobs of other kinds. The I
> propose that we stop sending in CodeBlobs to the GCs and make sure to only
> give them nmethods.
>
> I removed `void CodeCache::blobs_do(Cod
On Fri, 5 Apr 2024 12:32:30 GMT, Stefan Karlsson wrote:
> The GCs scan and handles nmethods and ignores CodeBlobs of other kinds. The I
> propose that we stop sending in CodeBlobs to the GCs and make sure to only
> give them nmethods.
>
> I removed `void CodeCache::blobs_do(Cod
n. If someone wants to keep this verification in their
> favorite GC I can add calls to this function where we used to call
> CodeCache::blobs_do.
>
> I've only done limited testing and will run extensive testing concurrent with
> the review.
Stefan Karlsson has updated the pul
The GCs scan and handles nmethods and ignores CodeBlobs of other kinds. The I
propose that we stop sending in CodeBlobs to the GCs and make sure to only give
them nmethods.
I removed `void CodeCache::blobs_do(CodeBlobClosure* f)` since there's no more
usage of that function. Is this OK?
I
On Thu, 4 Apr 2024 09:45:58 GMT, Stefan Karlsson wrote:
> We have a few places that uses the terms `KlassObj` and `klassOop` when
> referring to Klasses. This is old code from before the PermGen removal, when
> Klasses also were Java objects.
>
> These names tripped me up whe
On Thu, 4 Apr 2024 12:18:24 GMT, Stefan Karlsson wrote:
>> We have a few places that uses the terms `KlassObj` and `klassOop` when
>> referring to Klasses. This is old code from before the PermGen removal, when
>> Klasses also were Java objects.
>>
>> Th
On Thu, 4 Apr 2024 17:04:30 GMT, Vladimir Kozlov wrote:
>> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE
>> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365)
>> which was used for AOT [JEP 295](https://openjdk.org/jeps/295)
>>
On Thu, 4 Apr 2024 15:56:34 GMT, Vladimir Kozlov wrote:
>> src/hotspot/share/gc/shared/gcBehaviours.hpp line 31:
>>
>>> 29: #include "oops/oopsHierarchy.hpp"
>>> 30:
>>> 31: // This is the behaviour for checking if a nmethod is unloading
>>
>> Maybe this should be *an* nmethod?
>
> Quote:
On Thu, 4 Apr 2024 12:13:21 GMT, Roman Kennke wrote:
>> I think it is an old-style TODO. I'm considering if we shouldn't just remove
>> these comments. What do people think about that?
>
> I'm not even sure what they want to say, really. Should be good to remove,
> and if anybody can make
On Thu, 4 Apr 2024 10:07:11 GMT, Roman Kennke wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review Roman
>
> src/hotspot/share/memory/heapInspection.cpp line 173:
>
>&
> Tested with serviceability/dcmd/gc/ClassHistogramTest.java but will run this
> through our lower tiers.
Stefan Karlsson has updated the pull request incrementally with one additional
commit since the last revision:
Review Roman
-
Changes:
- all: https://git.openjdk.org/jdk/pull/186
On Thu, 4 Apr 2024 09:55:38 GMT, Roman Kennke wrote:
>> We have a few places that uses the terms `KlassObj` and `klassOop` when
>> referring to Klasses. This is old code from before the PermGen removal, when
>> Klasses also were Java objects.
>>
>> These names tripped me up when I was reading
We have a few places that uses the terms `KlassObj` and `klassOop` when
referring to Klasses. This is old code from before the PermGen removal, when
Klasses also were Java objects.
These names tripped me up when I was reading the heap heapInspection.cpp and
first though we were mixing the
On Thu, 4 Apr 2024 00:05:20 GMT, Vladimir Kozlov wrote:
>> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE
>> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365)
>> which was used for AOT [JEP 295](https://openjdk.org/jeps/295)
>>
On Wed, 3 Apr 2024 16:38:13 GMT, Vladimir Kozlov wrote:
>> src/hotspot/share/code/codeCache.cpp line 1009:
>>
>>> 1007: int CodeCache::nmethod_count() {
>>> 1008: int count = 0;
>>> 1009: for (GrowableArrayIterator heap =
>>> _nmethod_heaps->begin(); heap != _nmethod_heaps->end(); ++heap)
On Wed, 3 Apr 2024 16:29:03 GMT, Vladimir Kozlov wrote:
>> src/hotspot/share/code/codeBlob.hpp line 409:
>>
>>> 407:
>>> 408: // GC/Verification support
>>> 409: virtual void preserve_callee_argument_oops(frame fr, const
>>> RegisterMap *reg_map, OopClosure* f) override { /* nothing to do
On Mon, 1 Apr 2024 21:07:31 GMT, Vladimir Kozlov wrote:
>> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE
>> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365)
>> which was used for AOT [JEP 295](https://openjdk.org/jeps/295)
>>
On Tue, 2 Apr 2024 16:24:19 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
>>
On Tue, 12 Mar 2024 06:15:20 GMT, David Holmes wrote:
> GC folk should be reviewing this not runtime.
I don't fully agree with that. How these serviceability tools work, and their
interfaces, are usually not something that we GC devs are directly responsible
for. This change could be reviewed
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
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
On Fri, 9 Feb 2024 05:31:17 GMT, Yifeng Jin wrote:
>> These two files (`GCCause.java` and `gcCause.hpp`) should be in sync by
>> design, see comments in these two files. However, some recent changes (e.g.
>> [JDK-8240239](https://bugs.openjdk.org/browse/JDK-8240239)) to `gcCause.hpp`
>> were
On Thu, 8 Feb 2024 06:19:16 GMT, Yifeng Jin wrote:
> These two files (`GCCause.java` and `gcCause.hpp`) should be in sync by
> design, see comments in these two files. However, some recent changes (e.g.
> [JDK-8240239](https://bugs.openjdk.org/browse/JDK-8240239)) to `gcCause.hpp`
> were not
On Tue, 23 Jan 2024 10:20:46 GMT, Albert Mingkun Yang wrote:
> Trivial removing redundant code.
Marked as reviewed by stefank (Reviewer).
-
PR Review: https://git.openjdk.org/jdk/pull/17530#pullrequestreview-1838769192
On Fri, 5 Jan 2024 02:22:45 GMT, Denghui Dong wrote:
>> Hi,
>>
>> Please help review this patch that fixes the failures of
>> FullGCHeapDumpLimitTest.java caused by passing other gc flags.
>>
>> Thanks.
>
> Denghui Dong has updated the pull request incrementally with one additional
> commit
On Thu, 4 Jan 2024 11:56:45 GMT, David Holmes wrote:
> > For this test I think we can just add @requires vm.gc.Serial
>
> @stefank but it doesn't require that, it explicitly sets that. The test
> requires that no specific GC has been requested.
@dholmes-ora `@requires vm.gc.Serial` doesn't
On Thu, 4 Jan 2024 08:01:57 GMT, Denghui Dong wrote:
> Hi,
>
> Please help review this patch that fixes the failures of
> FullGCHeapDumpLimitTest.java caused by passing other gc flags.
>
> Thanks.
I prefer if we stay away from using `@requires vm.flagless` if possible. For
this test I think
On Mon, 11 Dec 2023 09:15:50 GMT, Stefan Karlsson wrote:
> [JDK-8315097](https://bugs.openjdk.org/browse/JDK-8315097): 'Rename
> createJavaProcessBuilder' changed the name of the ProcessTools helper
> functions used to create `ProcessBuilder`s used to spawn new java test
> proces
e that we name this functions using the same naming scheme we used
> for `createTestJavaProcessBuilder` and `createLimitedTestJavaProcessBuilder`.
> That is, we change `executeTestJvm` to `executeTestJava` and add a new
> `executeLimitedTestJava` function.
Stefan Karlsson has updated the pull request with a n
On Mon, 11 Dec 2023 14:06:43 GMT, Stefan Karlsson wrote:
>> [JDK-8315097](https://bugs.openjdk.org/browse/JDK-8315097): 'Rename
>> createJavaProcessBuilder' changed the name of the ProcessTools helper
>> functions used to create `ProcessBuilder`s used to spawn new java
e that we name this functions using the same naming scheme we used
> for `createTestJavaProcessBuilder` and `createLimitedTestJavaProcessBuilder`.
> That is, we change `executeTestJvm` to `executeTestJava` and add a new
> `executeLimitedTestJava` function.
Stefan Karlsson has updated the pull request with a n
On Wed, 20 Dec 2023 21:11:09 GMT, Kim Barrett wrote:
> I'm not a fan of the additional clutter in APIs that the static memory types
> add. If we had a variant of GrowableArrayCHeap that was not itself
> dynamically allocatable and took a memory type to use internally as a
> constructor
e that we name this functions using the same naming scheme we used
> for `createTestJavaProcessBuilder` and `createLimitedTestJavaProcessBuilder`.
> That is, we change `executeTestJvm` to `executeTestJava` and add a new
> `executeLimitedTestJava` function.
Stefan Karlsson has updated the pull request increme
e that we name this functions using the same naming scheme we used
> for `createTestJavaProcessBuilder` and `createLimitedTestJavaProcessBuilder`.
> That is, we change `executeTestJvm` to `executeTestJava` and add a new
> `executeLimitedTestJava` function.
Stefan Karlsson has updated the pull request incrementally wi
[JDK-8315097](https://bugs.openjdk.org/browse/JDK-8315097): 'Rename
createJavaProcessBuilder' changed the name of the ProcessTools helper functions
used to create `ProcessBuilder`s used to spawn new java test processes.
We now have `createTestJavaProcessBuilder` and
On Wed, 22 Nov 2023 15:00:29 GMT, Stefan Karlsson wrote:
> In the rewrites made for:
> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>
> I removed the filtering of *owned ObjectMonitors wi
On Wed, 29 Nov 2023 06:38:51 GMT, Stefan Karlsson wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *
On Tue, 28 Nov 2023 18:38:07 GMT, Daniel D. Daugherty
wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix indentation
>
> I'll re-review again once the last set of comments are a
On Mon, 27 Nov 2023 19:33:09 GMT, Serguei Spitsyn wrote:
> It will be completely safe to run mach5 tiers 1-4, tier5-svc and 6. In this
> particular case, the tier6 can be not necessary. It has some important
> -Xcomp/interp_only_mode related testing.
I've run the suggested testing and it
On Wed, 29 Nov 2023 07:00:06 GMT, David Holmes wrote:
> Updates look good. I think that is all from me. Thanks for your patience on
> the test issues.
Thanks for the in-depth review!
-
PR Comment: https://git.openjdk.org/jdk/pull/16783#issuecomment-1831327248
thread holds a lock
> with a dead object, and the code will return NULL as one of the "owned
> monitors" returned. I don't think that's a good idea, so I'm filtering out
> these ObjectMonitor for those calls.
>
> Test: the written tests with and without the fix. Tier1-Tier3, s
On Tue, 28 Nov 2023 05:38:33 GMT, David Holmes wrote:
>> test/hotspot/jtreg/serviceability/jvmti/GetOwnedMonitorInfo/GetOwnedMonitorInfoTest.java
>> line 1:
>>
>>> 1: /*
>>
>> Please update the `@bug` line and update the summary.
>
> This is still outstanding - thanks.
Done.
-
On Mon, 27 Nov 2023 22:42:41 GMT, Daniel D. Daugherty
wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix indentation
>
> test/hotspot/jtreg/runtime/Monitor/MonitorWithDeadO
On Tue, 28 Nov 2023 05:34:39 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix indentation
>
> test/hotspot/jtreg/runtime/Monitor/MonitorWithDeadObj
On Tue, 28 Nov 2023 05:26:32 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix indentation
>
> test/hotspot/jtreg/runtime/Monitor/MonitorWithDeadObjectTest.
On Tue, 28 Nov 2023 05:33:08 GMT, David Holmes wrote:
>> @dcubed-ojdk These are the thread routines passed to `pthread_create` and
>> must be typed as `void *(*start_routine)(void*)`.
>
> Though I just noticed the parameter is missing. As @stefank has pointed out
> this was copied from another
On Tue, 28 Nov 2023 05:31:11 GMT, David Holmes wrote:
>> test/hotspot/jtreg/runtime/Monitor/libMonitorWithDeadObjectTest.c line 110:
>>
>>> 108:
>>> 109: // Let the GC clear the weak reference to the object.
>>> 110: system_gc(env);
>>
>> A single GC may not be enough...
>
> @dcubed-ojdk
On Mon, 27 Nov 2023 22:46:15 GMT, Daniel D. Daugherty
wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix indentation
>
> test/hotspot/jtreg/runtime/Monitor/libMonitorWithDe
thread holds a lock
> with a dead object, and the code will return NULL as one of the "owned
> monitors" returned. I don't think that's a good idea, so I'm filtering out
> these ObjectMonitor for those calls.
>
> Test: the written tests with and without the fix. Tier1-Tier3, s
On Mon, 27 Nov 2023 19:04:42 GMT, Serguei Spitsyn wrote:
> The fix looks good to me. How was this tested?
Thanks. It was tested with the added and updated tests in GHA. Do you have any
suggestions for more tests to run?
> test/hotspot/jtreg/runtime/Monitor/libMonitorWithDeadObjectTest.c line
On Mon, 27 Nov 2023 18:58:14 GMT, Serguei Spitsyn wrote:
>> Yeah, I was thinking the same. Maybe @sspitsyn or @plummercj could give
>> guidance here?
>
> This looks okay. I see no problem with it.
OK. Thanks.
-
PR Review Comment:
thread holds a lock
> with a dead object, and the code will return NULL as one of the "owned
> monitors" returned. I don't think that's a good idea, so I'm filtering out
> these ObjectMonitor for those calls.
>
> Test: the written tests with and without the fix. Tier1-Tier3, s
thread holds a lock
> with a dead object, and the code will return NULL as one of the "owned
> monitors" returned. I don't think that's a good idea, so I'm filtering out
> these ObjectMonitor for those calls.
>
> Test: the written tests with and without the fix. Tier1-Tier3, s
On Mon, 27 Nov 2023 02:17:27 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Split test and use othervm
>
> test/hotspot/jtreg/serviceabi
On Mon, 27 Nov 2023 02:14:05 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Split test and use othervm
>
> test/hotspot/jtreg/runtime/Monitor/libMonito
On Mon, 27 Nov 2023 02:09:56 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Split test and use othervm
>
> test/hotspot/jtreg/serviceabi
On Mon, 27 Nov 2023 02:03:30 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Split test and use othervm
>
> test/hotspot/jtreg/runtime/Monitor/MonitorWithDeadObj
On Mon, 27 Nov 2023 01:56:01 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Split test and use othervm
>
> test/hotspot/jtreg/runtime/Monitor/libMonito
On Mon, 27 Nov 2023 01:52:29 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Split test and use othervm
>
> test/hotspot/jtreg/runtime/Monitor/libMonitorWithDead
On Mon, 27 Nov 2023 02:02:08 GMT, David Holmes wrote:
>> test/hotspot/jtreg/runtime/Monitor/MonitorWithDeadObjectTest.java line 75:
>>
>>> 73: private static void testDetachThread() {
>>> 74: // Create an ObjectMonitor with a dead object from an
>>> 75: // attached thread.
On Mon, 27 Nov 2023 01:43:07 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Split test and use othervm
>
> test/hotspot/jtreg/runtime/Monitor/MonitorWithDeadObj
thread holds a lock
> with a dead object, and the code will return NULL as one of the "owned
> monitors" returned. I don't think that's a good idea, so I'm filtering out
> these ObjectMonitor for those calls.
>
> Test: the written tests with and without the fix. Tier1-Tier3, s
On Fri, 24 Nov 2023 07:59:51 GMT, Afshin Zafari wrote:
> > @afshin-zafari I'm happy to hear the code is now cleaner and nicer, but
> > AFAICS this new version of the code has only had a single review. It should
> > really have been re-reviewed by at least one of the earlier reviewers.
> >
On Fri, 24 Nov 2023 06:43:35 GMT, David Holmes wrote:
>> I provoked test failures for all paths I filtered. If I remove this check
>> and run:
>>
>> make -C ../build/fastdebug test
>> TEST=test/hotspot/jtreg/runtime/Monitor/IterateMonitorWithDeadObjectTest.java
>>
On Thu, 23 Nov 2023 16:11:32 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
On Thu, 23 Nov 2023 11:52:38 GMT, Stefan Karlsson wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *
On Thu, 23 Nov 2023 14:17:18 GMT, Magnus Ihse Bursie wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Tweak test comments
>
> Build changes are trivially fine.
Thanks @magicus. I'
thread holds a lock
> with a dead object, and the code will return NULL as one of the "owned
> monitors" returned. I don't think that's a good idea, so I'm filtering out
> these ObjectMonitor for those calls.
>
> Test: the written tests with and without the fix. Tier1-Tier3, s
On Thu, 23 Nov 2023 11:52:38 GMT, Stefan Karlsson wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *
On Thu, 23 Nov 2023 13:32:23 GMT, Afshin Zafari wrote:
>>> > Thanks for making this change.
>>> > I'd like to suggest the following cleanups, some documentation, and a few
>>> > tests:
>>> > [20d4502](https://github.com/openjdk/jdk/commit/20d4502471ba396ae395512cfa3dab3f87555421)
>>> > I think
On Thu, 23 Nov 2023 11:00:32 GMT, Afshin Zafari wrote:
> > Thanks for making this change.
> > I'd like to suggest the following cleanups, some documentation, and a few
> > tests:
> > [20d4502](https://github.com/openjdk/jdk/commit/20d4502471ba396ae395512cfa3dab3f87555421)
> > I think it might
thread holds a lock
> with a dead object, and the code will return NULL as one of the "owned
> monitors" returned. I don't think that's a good idea, so I'm filtering out
> these ObjectMonitor for those calls.
>
> Test: the written tests with and without the fix. Tier1-Tier3, s
On Thu, 23 Nov 2023 00:48:19 GMT, David Holmes wrote:
> > Previously, the locked monitors never got unlocked because the
> > ObjectMonitor iterator never exposed these monitors to the JNI detach code
>
> I had not realized that. It explains some confusion in a separate issue I had
> been
On Thu, 23 Nov 2023 02:10:41 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Tweaked comment in test
>> - Rewrite tests
>
>
thread holds a lock
> with a dead object, and the code will return NULL as one of the "owned
> monitors" returned. I don't think that's a good idea, so I'm filtering out
> these ObjectMonitor for those calls.
>
> Test: the written tests with and without the fix. Tier1-Tier3, so
On Thu, 23 Nov 2023 02:07:59 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead
On Thu, 23 Nov 2023 00:48:19 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead
On Thu, 23 Nov 2023 01:38:57 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead
On Thu, 23 Nov 2023 01:29:24 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead
On Thu, 23 Nov 2023 01:28:32 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead
On Thu, 23 Nov 2023 01:27:24 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead
In the rewrites made for:
[JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
asserts in interleaved ObjectMonitor::deflate_monitor calls`
I removed the filtering of *owned ObjectMonitors with dead objects*. The
reasoning was that you should never have an owned
On Wed, 22 Nov 2023 08:41:35 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
On Fri, 17 Nov 2023 13:20:22 GMT, Afshin Zafari wrote:
>> @kimbarrett , @dholmes-ora , @merykitty
>> Is there any comment on this PR?
>
>> @afshin-zafari I will leave it to other to (re-) review the latest changes.
>> I don't grok this template stuff enough to pass judgement.
>
> Thank you
On Mon, 13 Nov 2023 10:11:26 GMT, Long Yang wrote:
>> I would like to fix this.
>>
>> Create 4096 threads, and the stack depth of each thread is 256.
>> After running jmx.dumpAllThreads(true, true), the RSS reaches 5.3GiB.
>> After optimization, the RSS is 250MiB.
>>
>> I would appreciate it
On Mon, 13 Nov 2023 02:01:09 GMT, David Holmes wrote:
>> Long Yang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> move ResourceMark to before start_vf
>
> src/hotspot/share/services/threadService.cpp line 698:
>
>> 696:
On Thu, 26 Oct 2023 09:51:15 GMT, Johan Sjölen wrote:
>> I think that NMT is deserving of its own subdirectory. Can we do a review of
>> the changes before I fix the merge conflicts?
>>
>> 1. Moved all the nmt source code from services/ to nmt/
>> 2. Renamed all the include statements and
On Tue, 24 Oct 2023 11:51:45 GMT, Johan Sjölen wrote:
>> I think that NMT is deserving of its own subdirectory. Can we do a review of
>> the changes before I fix the merge conflicts?
>>
>> 1. Moved all the nmt source code from services/ to nmt/
>> 2. Renamed all the include statements and
On Tue, 5 Sep 2023 18:05:34 GMT, Roger Riggs wrote:
>> I have created an alternative that uses enums to force the user to make a
>> decision:
>> https://github.com/openjdk/jdk/compare/master...lkorinth:jdk:+process_tools
>> . Another alternative is to do the same but instead using an enum (I
On Tue, 24 Oct 2023 07:49:30 GMT, Leo Korinth wrote:
>> Rename createJavaProcessBuilder so that it is not used by mistake instead of
>> createTestJvm.
>>
>> I have used the following sed script: `find -name "*.java" | xargs -n 1 sed
>> -i -e
>>
On Fri, 20 Oct 2023 11:31:11 GMT, Johan Sjölen wrote:
> For the gtest source files I separated the includes in a consistent manner,
> they all look like this pattern now:
That's not what I see in the latest patch. Could you revert that separation and
then we can consider that style change in
On Mon, 23 Oct 2023 08:34:59 GMT, Johan Sjölen wrote:
>> I think that NMT is deserving of its own subdirectory. Can we do a review of
>> the changes before I fix the merge conflicts?
>>
>> 1. Moved all the nmt source code from services/ to nmt/
>> 2. Renamed all the include statements and
1 - 100 of 197 matches
Mail list logo
