Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v4]

2022-04-29 Thread Kim Barrett
On Thu, 28 Apr 2022 20:06:47 GMT, Albert Mingkun Yang wrote: >> src/hotspot/share/oops/instanceKlass.cpp line 497: >> >>> 495: _nest_host_index(0), >>> 496: _init_state(allocated), >>> 497: _reference_type(REF_NONE), >> >> This is initializing `_reference_type` to the wrong value for a >

Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v4]

2022-04-28 Thread Kim Barrett
On Thu, 28 Apr 2022 09:15:33 GMT, Albert Mingkun Yang wrote: >> Simple rename and some comments update. >> >> Test: build > > Albert Mingkun Yang has updated the pull request incrementally with two > additional commits since the last revision: > > - comment > - Rework reference type initiali

Re: RFR: 8285364: Use more precise name for ReferenceType::REF_OTHER [v2]

2022-04-22 Thread Kim Barrett
On Thu, 21 Apr 2022 11:30:20 GMT, Albert Mingkun Yang wrote: >> Simple rename and some comments update. >> >> Test: build > > Albert Mingkun Yang has updated the pull request incrementally with one > additional commit since the last revision: > > review > > Using REF_SOFT seems too hacky. >

Re: RFR: 8285364: Use more precise name for ReferenceType::REF_OTHER

2022-04-21 Thread Kim Barrett
On Thu, 21 Apr 2022 10:48:06 GMT, Albert Mingkun Yang wrote: > Simple rename and some comments update. > > Test: build Changes requested by kbarrett (Reviewer). src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.cpp line 214: > 212: } > 213: > 214: static const char* reference_type_to_s

Re: RFR: 8283710: JVMTI: Use BitSet for object marking [v6]

2022-04-10 Thread Kim Barrett
> On Apr 9, 2022, at 2:44 AM, Thomas Stuefe wrote: > > On Fri, 8 Apr 2022 17:34:57 GMT, Roman Kennke wrote: > >> Yes, I get that. But the fragments and fragment-table are themselves inner >> classes that take a MEMFLAGS template. I could (perhaps) either use a >> constexpr MEMFLAGS arg and pa

Re: RFR: 8283710: JVMTI: Use BitSet for object marking [v6]

2022-04-07 Thread Kim Barrett
On Thu, 7 Apr 2022 18:28:42 GMT, Roman Kennke wrote: >> JVMTI heap walking marks objects in order to track which have been visited >> already. In order to do that, it uses bits in the object header. Those are >> the same bits that are also used by some GCs to mark objects (the lowest two >> bi

Re: RFR: 8283710: JVMTI: Use BitSet for object marking

2022-04-07 Thread Kim Barrett
On Mon, 4 Apr 2022 21:53:28 GMT, Roman Kennke wrote: > > One open question is which MEMFLAGS to use. mtTracing doesn't seem to be > > exactly right. Should I templatize BitSet and make JVMTI use > > mtServiceability and JRF use mtTracing as it did before? > > Yes, I think templatizing for MEMF

Re: RFR: 8269537: memset() is called after operator new [v3]

2021-10-06 Thread Kim Barrett
On Tue, 5 Oct 2021 20:39:30 GMT, Leo Korinth wrote: >> The basic problem is that we are relying on undefined behaviour, as >> documented in the code: >> >> // This whole business of passing information from ResourceObj::operator new >> // to the ResourceObj constructor via fields in the "object

Re: RFR: 8269537: memset() is called after operator new [v2]

2021-09-25 Thread Kim Barrett
On Fri, 24 Sep 2021 08:50:18 GMT, Leo Korinth wrote: >> I hit the new assert when not on Linux, I guess it has to do with the >> initialization of the thread local variable. > > Thanks Ioi for making me adding the assert!!! The sequencing of the > allocation function and the arguments to the co

Re: RFR: 8273482: Remove "foreground work" concept from WorkGang

2021-09-08 Thread Kim Barrett
On Wed, 8 Sep 2021 09:56:21 GMT, Per Liden wrote: > JDK-8237354 introduced the concept of "foreground work" in WorkGang, as a > special case for use by the HeapDumper. I propose that we remove this code, > since this special use case can be solved without the need for the concept of > "foregro

Re: RFR: 8271514: support JFR use of new ThreadsList::Iterator [v2]

2021-08-05 Thread Kim Barrett
On Thu, 5 Aug 2021 05:44:18 GMT, David Holmes wrote: >> The `_tlist` is a handle that ensures the captured ThreadsList remains live >> while the iterator (or a copy) exists. Dropping it would leave `_it` and >> `_end` potentially dangling. > > Okay I had assumed there was a ThreadsList/Handle

Re: RFR: 8271514: support JFR use of new ThreadsList::Iterator [v2]

2021-08-04 Thread Kim Barrett
On Mon, 2 Aug 2021 04:32:31 GMT, David Holmes wrote: > I'm assuming that the intent of this conversion is that > `JavaThreadIteratorWithHandle` is going to be removed - is that right? That was my intent. But this is a somewhat complicated use, so dealing with it separately. And I agree this

Re: RFR: 8271514: support JFR use of new ThreadsList::Iterator [v2]

2021-08-04 Thread Kim Barrett
On Wed, 4 Aug 2021 22:56:15 GMT, Serguei Spitsyn wrote: >> src/hotspot/share/jfr/utilities/jfrThreadIterator.hpp line 50: >> >>> 48: class JfrJavaThreadIteratorAdapter { >>> 49: private: >>> 50: ThreadsListHandle _tlist; >> >> Why do we need to store this? >> >> It looks very suspiocious to

Re: [jdk17] Integrated: 8268830: ProblemList 3 serviceability/dcmd/framework tests with ZGC on win-x64

2021-06-15 Thread Kim Barrett
On Tue, 15 Jun 2021 21:38:17 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList 3 serviceability/dcmd/framework tests with ZGC > on win-x64. Changes requested by kbarrett (Reviewer). Looks good. test/hotspot/jtreg/ProblemList-zgc.txt line 46: > 44: serviceability/dcmd/framework/

Re: RFR: 8267464: Circular-dependency resilient inline headers [v6]

2021-05-26 Thread Kim Barrett
On Wed, 26 May 2021 18:00:54 GMT, Stefan Karlsson wrote: >> I'd like to propose a small adjustment to how we write .inline.hpp files, in >> the hope to reduce include problems because of circular dependencies between >> inline headers. >> >> This is a small, contrived example to show the probl

Re: RFR: 8267464: Circular-dependency resilient inline headers [v5]

2021-05-26 Thread Kim Barrett
On Wed, 26 May 2021 06:38:47 GMT, Stefan Karlsson wrote: >> I'd like to propose a small adjustment to how we write .inline.hpp files, in >> the hope to reduce include problems because of circular dependencies between >> inline headers. >> >> This is a small, contrived example to show the probl

Re: RFR: 8267464: Circular-dependency resilient inline headers [v2]

2021-05-23 Thread Kim Barrett
On Fri, 21 May 2021 09:11:09 GMT, Stefan Karlsson wrote: >> I'd like to propose a small adjustment to how we write .inline.hpp files, in >> the hope to reduce include problems because of circular dependencies between >> inline headers. >> >> This is a small, contrived example to show the probl

Re: RFR: 8267468: Rename refill waster counters in ThreadLocalAllocBuffer

2021-05-21 Thread Kim Barrett
On Fri, 21 May 2021 14:15:11 GMT, Albert Mingkun Yang wrote: > Followup of https://bugs.openjdk.java.net/browse/JDK-8234532 > (https://github.com/openjdk/jdk/pull/3756), dropping the "slow" segment in > various counter names. Looks good. - Marked as reviewed by kbarrett (Reviewer

Re: RFR: 8264166: OopStorage should support specifying MEMFLAGS for allocations [v2]

2021-03-26 Thread Kim Barrett
tracking and > reporting. > > Testing: > mach5 tier1. > Manually compared NMT output before and after this change for a test that > generated a lot of one particular OopStorage entries. Kim Barrett has updated the pull request with a new target base due to a merge or a

Integrated: 8264166: OopStorage should support specifying MEMFLAGS for allocations

2021-03-26 Thread Kim Barrett
On Thu, 25 Mar 2021 07:27:58 GMT, Kim Barrett wrote: > Please review this change to OopStorage to allow the MEMFLAGS value for > associated allocations to be specified when the storage object is > constructed. This allows a subsystem that needs an OopStorage object to > a

Re: RFR: 8264166: OopStorage should support specifying MEMFLAGS for allocations

2021-03-25 Thread Kim Barrett
On Thu, 25 Mar 2021 08:42:06 GMT, Stefan Karlsson wrote: >> Please review this change to OopStorage to allow the MEMFLAGS value for >> associated allocations to be specified when the storage object is >> constructed. This allows a subsystem that needs an OopStorage object to >> associate its

RFR: 8264166: OopStorage should support specifying MEMFLAGS for allocations

2021-03-25 Thread Kim Barrett
Please review this change to OopStorage to allow the MEMFLAGS value for associated allocations to be specified when the storage object is constructed. This allows a subsystem that needs an OopStorage object to associate its allocation with others for that subsystem in NMT tracking and reporting

Re: RFR: 8261998: Remove unused shared entry support from utilities/hashtable [v2]

2021-02-19 Thread Kim Barrett
> Please review this small cleanup in the utilities/hashtable facility. The > support for "shared" entries is no longer needed or used, so is being deleted. > > Testing: > mach5 tier1-4 (some CDS tests are in tier4) Kim Barrett has updated the pull request with a new t

Integrated: 8261998: Remove unused shared entry support from utilities/hashtable

2021-02-19 Thread Kim Barrett
On Fri, 19 Feb 2021 02:39:20 GMT, Kim Barrett wrote: > Please review this small cleanup in the utilities/hashtable facility. The > support for "shared" entries is no longer needed or used, so is being deleted. > > Testing: > mach5 tier1-4 (some CDS tests are in tier

Re: RFR: 8261998: Remove unused shared entry support from utilities/hashtable

2021-02-19 Thread Kim Barrett
On Fri, 19 Feb 2021 02:56:14 GMT, Coleen Phillimore wrote: >> Please review this small cleanup in the utilities/hashtable facility. The >> support for "shared" entries is no longer needed or used, so is being >> deleted. >> >> Testing: >> mach5 tier1-4 (some CDS tests are in tier4) > > We migh

RFR: 8261998: Remove unused shared entry support from utilities/hashtable

2021-02-18 Thread Kim Barrett
Please review this small cleanup in the utilities/hashtable facility. The support for "shared" entries is no longer needed or used, so is being deleted. Testing: mach5 tier1-4 (some CDS tests are in tier4) - Commit messages: - remove shared entry support Changes: https://git.openj

Integrated: 8259778: Merge MutableSpace and ImmutableSpace

2021-01-28 Thread Kim Barrett
On Wed, 27 Jan 2021 23:06:41 GMT, Kim Barrett wrote: > Please review this change which merges ImmutableSpace into MutableSpace, > eliminating the former. There were no interesting uses of ImmutableSpace, > other than as the base class for MutableSpace. The name ImmutableSpace is &g

Re: RFR: 8259778: Merge MutableSpace and ImmutableSpace [v3]

2021-01-28 Thread Kim Barrett
On Thu, 28 Jan 2021 12:07:52 GMT, Thomas Schatzl wrote: >> I assume that tier1-3 includes the SA tests :) >> >> Looks good other than that nit. > > Hi David, > >> _Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) on >> [serviceability-dev](mailto:serviceability-dev@open

Re: RFR: 8259778: Merge MutableSpace and ImmutableSpace [v3]

2021-01-28 Thread Kim Barrett
g: > mach5 tier1-3 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 additional commits since the last revision: - Merge

Re: RFR: 8259778: Merge MutableSpace and ImmutableSpace [v2]

2021-01-28 Thread Kim Barrett
g: > mach5 tier1-3 Kim Barrett has updated the pull request incrementally with one additional commit since the last revision: fix comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/2271/files - new: https://git.openjdk.java.net/jdk/pull/2271/files/e1d954

Re: RFR: 8259778: Merge MutableSpace and ImmutableSpace

2021-01-28 Thread Kim Barrett
On Thu, 28 Jan 2021 12:07:52 GMT, Thomas Schatzl wrote: >> I assume that tier1-3 includes the SA tests :) >> >> Looks good other than that nit. > > Hi David, > >> _Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) on >> [serviceability-dev](mailto:serviceability-dev@open

RFR: 8259778: Merge MutableSpace and ImmutableSpace

2021-01-27 Thread Kim Barrett
Please review this change which merges ImmutableSpace into MutableSpace, eliminating the former. There were no interesting uses of ImmutableSpace, other than as the base class for MutableSpace. The name ImmutableSpace is kind of a misnomer given that usage. Testing: mach5 tier1-3 -

Re: RFR: 8256167: Convert JDK use of `Reference::get` to `Reference::refersTo` [v2]

2020-12-04 Thread Kim Barrett
On Fri, 4 Dec 2020 22:38:24 GMT, Mandy Chung wrote: >> This patch replaces some uses of `Reference::get` to `Reference::refersTo` >> to avoid keeping a referent strongly reachable that could cause unnecessary >> delay in collecting such object. I only made change in some but not all >> class

Re: RFR: 8256167: Convert JDK use of `Reference::get` to `Reference::refersTo`

2020-12-04 Thread Kim Barrett
On Thu, 3 Dec 2020 22:54:54 GMT, Mandy Chung wrote: > This patch replaces some uses of `Reference::get` to `Reference::refersTo` to > avoid keeping a referent strongly reachable that could cause unnecessary > delay in collecting such object. I only made change in some but not all > classes i

Re: RFR: 8257140: Crash in JvmtiTagMap::flush_object_free_events() [v2]

2020-12-02 Thread Kim Barrett
On Tue, 1 Dec 2020 14:19:15 GMT, Coleen Phillimore wrote: >> The JvmtiTagMap can be deleted while it is posting. The JvmtiEnv is still >> on the list but the env is "disposed" of and the tag_map is deleted to save >> memory until the JvmtiEnv can be reclaimed at a safepoint and taken off the

Re: RFR: 8256830: misc tests failed with "assert(env->is_enabled(JVMTI_EVENT_OBJECT_FREE)) failed: checking" [v2]

2020-12-01 Thread Kim Barrett
On Tue, 1 Dec 2020 02:22:57 GMT, David Holmes wrote: >> void JvmtiTagMap::flush_object_free_events() { >> assert_not_at_safepoint(); >> if (env()->is_enabled(JVMTI_EVENT_OBJECT_FREE)) { >> >> Called by JVMTI to disable events and called by the service thread. And >> here for get_objects_wi

Re: RFR: 8256830: misc tests failed with "assert(env->is_enabled(JVMTI_EVENT_OBJECT_FREE)) failed: checking"

2020-11-25 Thread Kim Barrett
On Wed, 25 Nov 2020 18:40:49 GMT, Coleen Phillimore wrote: > The ServiceThread cleaning used a stale ObjectFree state when calling > remove_dead_entries, because another thread had concurrently set is_enabled > to false. Add a lock around setting/resetting the lock event state and > retest th

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v11]

2020-11-18 Thread Kim Barrett
On Wed, 18 Nov 2020 19:31:44 GMT, Serguei Spitsyn wrote: > test/hotspot/jtreg/vmTestbase/nsk/jvmti/AttachOnDemand/attach021/attach021Agent00.cpp: > > The change below is not needed as the call to > nsk_jvmti_aod_disableEventAndFinish() does exactly the same: > > ``` > -nsk_jvmti_aod_disabl

Re: RFR: 8253081: G1 fails on stale objects in archived module graph in Open Archive regions [v4]

2020-11-17 Thread Kim Barrett
On Tue, 17 Nov 2020 10:55:19 GMT, Thomas Schatzl wrote: >> Hi all, >> >> can I have reviews for this change that changes the way how archive >> regions are managed in general and specifically by the G1 collector, fixing >> the crashes caused by adding the module graph into the archive in >>

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v11]

2020-11-16 Thread Kim Barrett
On Mon, 16 Nov 2020 23:30:25 GMT, Coleen Phillimore wrote: >> This change turns the HashTable that JVMTI uses for object tagging into a >> regular Hotspot hashtable - the one in hashtable.hpp with resizing and >> rehashing. Instead of pointing directly to oops so that GC has to walk the >> t

Re: RFR: 8253081: G1 fails on stale objects in archived module graph in Open Archive regions

2020-11-16 Thread Kim Barrett
On Wed, 11 Nov 2020 11:39:59 GMT, Thomas Schatzl wrote: > Hi all, > > can I have reviews for this change that changes the way how archive regions > are managed in general and specifically by the G1 collector, fixing the > crashes caused by adding the module graph into the archive in > [JDK-

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v4]

2020-11-04 Thread Kim Barrett
On Tue, 3 Nov 2020 21:40:39 GMT, Coleen Phillimore wrote: >> src/hotspot/share/prims/jvmtiTagMap.cpp line 127: >> >>> 125: // The table cleaning, posting and rehashing can race for >>> 126: // concurrent GCs. So fix it here once we have a lock or are >>> 127: // at a safepoint. >> >> I th

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v4]

2020-11-04 Thread Kim Barrett
On Wed, 4 Nov 2020 07:52:12 GMT, Kim Barrett wrote: >> So the design is that when the oops have new addresses, we set a flag in the >> table to rehash it. Not sure why this is wrong and why wouldn't it work for >> shenandoah? @zhengyu123 ? When we call WeakHandle

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v4]

2020-11-04 Thread Kim Barrett
On Tue, 3 Nov 2020 21:31:35 GMT, Coleen Phillimore wrote: >> src/hotspot/share/prims/jvmtiTagMap.cpp line 2979: >> >>> 2977: >>> 2978: // Concurrent GC needs to call this in relocation pause, so after the >>> objects are moved >>> 2979: // and have their new addresses, the table can be rehashe

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v5]

2020-11-04 Thread Kim Barrett
On Wed, 4 Nov 2020 00:08:10 GMT, Coleen Phillimore wrote: >> This change turns the HashTable that JVMTI uses for object tagging into a >> regular Hotspot hashtable - the one in hashtable.hpp with resizing and >> rehashing. Instead of pointing directly to oops so that GC has to walk the >> ta

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v3]

2020-11-04 Thread Kim Barrett
On Wed, 4 Nov 2020 07:41:39 GMT, Kim Barrett wrote: >>>Though it might be possible to go even further and eliminate >>>WeakProcessorPhases as a thing separate from OopStorageSet. >> >> This makes sense. Can we file another RFE for this? I was sort of surp

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v3]

2020-11-04 Thread Kim Barrett
On Tue, 3 Nov 2020 23:38:08 GMT, Coleen Phillimore wrote: >> Ok, so I'm not sure what to do with this: >> >> enum Phase { >> // Serial phase. >> JVMTI_ONLY(jvmti) >> // Additional implicit phase values follow for oopstorages. >> `};` >> >> I've removed the only thing in this enu

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v3]

2020-11-03 Thread Kim Barrett
On Tue, 3 Nov 2020 12:58:22 GMT, Coleen Phillimore wrote: >> This change turns the HashTable that JVMTI uses for object tagging into a >> regular Hotspot hashtable - the one in hashtable.hpp with resizing and >> rehashing. Instead of pointing directly to oops so that GC has to walk the >> ta

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v4]

2020-11-03 Thread Kim Barrett
On Tue, 3 Nov 2020 14:53:12 GMT, Coleen Phillimore wrote: >> This change turns the HashTable that JVMTI uses for object tagging into a >> regular Hotspot hashtable - the one in hashtable.hpp with resizing and >> rehashing. Instead of pointing directly to oops so that GC has to walk the >> ta

Re: RFR: 8253180: ZGC: Implementation of JEP 376: ZGC: Concurrent Thread-Stack Processing [v8]

2020-10-01 Thread Kim Barrett
> On Oct 1, 2020, at 5:53 AM, Erik Österlund > wrote: > > >> _Mailing list message from [Kim Barrett](mailto:kim.barr...@oracle.com) >> onsrc/hotspot/share/runtime/frame.cpp >> 466 StackFrameStream(JavaThread *thread, bool update, bool process_frames); >&g

Re: RFR: 8253180: ZGC: Implementation of JEP 376: ZGC: Concurrent Thread-Stack Processing [v8]

2020-09-29 Thread Kim Barrett
> On Sep 29, 2020, at 11:03 AM, Erik Österlund > wrote: > > Changes: https://git.openjdk.java.net/jdk/pull/296/files > Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=296&range=07 > Stats: 2705 lines in 129 files changed: 2137 ins; 310 del; 258 mod > Patch: https://git.openjdk.java.net/j

Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v6]

2020-09-16 Thread Kim Barrett
On Wed, 16 Sep 2020 16:13:23 GMT, Daniel D. Daugherty wrote: >> This RFE is to migrate the following field to OopStorage: >> >> class ObjectMonitor { >> >> void* volatile _object; // backward object pointer - strong root >> >> Unlike the previous patches in this series, there are a lot of c

Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v4]

2020-09-15 Thread Kim Barrett
On Mon, 14 Sep 2020 18:55:38 GMT, Daniel D. Daugherty wrote: >> This RFE is to migrate the following field to OopStorage: >> >> class ObjectMonitor { >> >> void* volatile _object; // backward object pointer - strong root >> >> Unlike the previous patches in this series, there are a lot of c

Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v5]

2020-09-15 Thread Kim Barrett
On Mon, 14 Sep 2020 21:15:00 GMT, Daniel D. Daugherty wrote: >> This RFE is to migrate the following field to OopStorage: >> >> class ObjectMonitor { >> >> void* volatile _object; // backward object pointer - strong root >> >> Unlike the previous patches in this series, there are a lot of c

Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v2]

2020-09-14 Thread Kim Barrett
> On Sep 14, 2020, at 2:39 PM, Daniel D.Daugherty > wrote: > > On Mon, 14 Sep 2020 18:33:17 GMT, Daniel D. Daugherty > wrote: > >> @kimbarrett: >> >>> src/hotspot/share/oops/weakHandle.cpp >>> 36 WeakHandle::WeakHandle(OopStorage* storage, oop obj) : >>> 37 _obj(storage->allocate()) { >>> 38

Re: RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v8]

2020-09-09 Thread Kim Barrett
On Thu, 10 Sep 2020 03:18:23 GMT, David Holmes wrote: >> This is a rather large but generally simple cleanup. >> >> We use a lot of raw C-style casts to "(JavaThread*)" typically after >> checking "thread->is_Java_thread()". To simplify >> this pattern, and make the code look somewhat cleaner w

Re: RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v7]

2020-09-09 Thread Kim Barrett
On Wed, 9 Sep 2020 12:48:11 GMT, David Holmes wrote: >> This is a rather large but generally simple cleanup. >> >> We use a lot of raw C-style casts to "(JavaThread*)" typically after >> checking "thread->is_Java_thread()". To simplify >> this pattern, and make the code look somewhat cleaner we

Re: RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v5]

2020-09-09 Thread Kim Barrett
Hi David. Thanks for clarifying some bits I was confused abut. >> src/hotspot/share/jvmci/jvmciEnv.cpp >> 243 void JVMCIEnv::describe_pending_exception(bool clear) { >> 244 JavaThread* THREAD = JavaThread::current(); >> This change looks suspicious. The old code used Thread::current() here a

Re: RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v5]

2020-09-08 Thread Kim Barrett
> On Sep 8, 2020, at 11:30 PM, Kim Barrett wrote: > > I reviewed all of v4, not just the gc parts this time. I guess it was v5 I was reviewing. There seems to be an inconsistency in numbering.

Re: RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v5]

2020-09-08 Thread Kim Barrett
> On Sep 8, 2020, at 9:27 AM, David Holmes wrote: > David Holmes has updated the pull request incrementally with one additional > commit since the last revision: > > This is a simpler approach to use the static_cast. Changes: > - Change C-style cast to static_cast and move function definitions

Re: RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v4]

2020-09-08 Thread Kim Barrett
> On Sep 8, 2020, at 6:06 AM, David Holmes wrote: > > Just to be clear please wait for v5 to appear before reviewing. I think you meant “v4”, i.e. the 5th zero-based version?

Re: RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v2]

2020-09-08 Thread Kim Barrett
> On Sep 8, 2020, at 3:00 AM, David Holmes wrote: > > On 8/09/2020 4:34 pm, Kim Barrett wrote: >> […] >> src/hotspot/share/runtime/thread.hpp >> 507 const JavaThread* as_const_Java_thread() const; >> This missed part of what I'd suggested. I don'

Re: RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v2]

2020-09-07 Thread Kim Barrett
> On Sep 8, 2020, at 12:09 AM, David Holmes wrote: > David Holmes has updated the pull request incrementally with one additional > commit since the last revision: > > Used static_cast instead of raw C-style cast > Moved inline functions to thread.inline.hpp > Updated #includes to deal with th

Re: RFR: 8252406: Introduce Thread::as_Java_thread() convenience function

2020-09-07 Thread Kim Barrett
> On Sep 7, 2020, at 5:47 PM, David Holmes wrote: > > […] > Commit messages: > - Initial changes for 8252406 > > Changes: https://git.openjdk.java.net/jdk/pull/37/files > Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=37&range=00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8252406

Re: RFR(S): 8252407: Build failure with gcc-8+ and asan

2020-09-01 Thread Kim Barrett
> On Sep 1, 2020, at 4:57 AM, David Holmes wrote: > > Hi Eric, > > On 1/09/2020 6:15 pm, Eric Liu wrote: >> Hi all, >> Please review this simple change to fix some compile warnings. >> The newer gcc (gcc-8 or higher) would warn for calls to bounded string >> manipulation functions such as 'strnc

Re: RFR (M) 8249650: Optimize JNIHandle::make_local thread variable usage

2020-07-19 Thread Kim Barrett
> On Jul 20, 2020, at 1:53 AM, David Holmes wrote: > > Hi Kim, > > Thanks for looking at this. > > Updated webrev at: > > http://cr.openjdk.java.net/~dholmes/8249650/webrev.v2/ Looks good. > > On 20/07/2020 3:22 pm, Kim Barrett wrote: >>> On Jul

Re: RFR 8247878: Move Management strong oops to OopStorage

2020-07-19 Thread Kim Barrett
> On Jul 20, 2020, at 12:47 AM, David Holmes wrote: > > Hi Coleen, > > On 18/07/2020 7:29 am, coleen.phillim...@oracle.com wrote: >> I've corrected this change with Kim's and David's feedback, and retested >> with tier1-3. This is much better. > > Yes this is better. :) Thanks to Kim for clar

Re: RFR (M) 8249650: Optimize JNIHandle::make_local thread variable usage

2020-07-19 Thread Kim Barrett
> On Jul 20, 2020, at 12:16 AM, David Holmes wrote: > > Subject line got truncated by accident ... > > On 20/07/2020 11:06 am, David Holmes wrote: >> Bug: https://bugs.openjdk.java.net/browse/JDK-8249650 >> webrev: http://cr.openjdk.java.net/~dholmes/8249650/webrev/ >> This is a simple cleanup t

Re: RFR 8247878: Move Management strong oops to OopStorage

2020-07-17 Thread Kim Barrett
> On Jul 16, 2020, at 11:01 AM, coleen.phillim...@oracle.com wrote: > > Summary: Use OopStorage for strong oops stored with memory and thread > sampling and dumping, and remove oops_do and GC calls. > > These use OopStorageSet::vm_global() OopStorage for now. I'll change the > thread sampling

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (hotspot)

2020-05-06 Thread Kim Barrett
> On May 7, 2020, at 1:35 AM, Mikael Vidstedt > wrote: > > > New webrev here: > > webrev: > http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/hotspot/open/webrev/ > incremental: > http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/hotspot.incr/open/webrev/ > > Remaining

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (hotspot)

2020-05-04 Thread Kim Barrett
> On May 4, 2020, at 1:12 AM, Mikael Vidstedt > wrote: > > > Please review this change which implements part of JEP 381: > > JBS: https://bugs.openjdk.java.net/browse/JDK-8244224 > webrev: > http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/hotspot/open/webrev/ > JEP: https://bugs.

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (hotspot)

2020-05-04 Thread Kim Barrett
> On May 4, 2020, at 4:28 AM, Stefan Karlsson > wrote: > > Hi Mikael, > > On 2020-05-04 07:12, Mikael Vidstedt wrote: >> Please review this change which implements part of JEP 381: >> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224 >> webrev: >> http://cr.openjdk.java.net/~mikael/webrevs

Re: [15] RFR 8238633: JVMTI heap walk should consult GC for marking oops

2020-02-21 Thread Kim Barrett
> On Feb 7, 2020, at 10:53 AM, Zhengyu Gu wrote: > > Hi, > > I would like purpose this change that allows GC to provide ObjectMarker > during JVMTI heap walk. > > Currently, JVMTI heap walk uses oop markword's 'marked' pattern to indicate > 'visited' oop. > > Unfortunately, it conflicts with

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

2019-10-11 Thread Kim Barrett
> On Oct 11, 2019, at 11:22 PM, Yasumasa Suenaga > wrote: > > Hi, > > I updated Plan B-1 (#pragma) due to the comment from Chris [1]. > > > A. Use memcpy() > http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.02/ > > B-1. Use #pragma > http://cr.openjdk.java.net/~ysuenaga/

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

2019-10-11 Thread Kim Barrett
> On Oct 11, 2019, at 6:18 PM, Kim Barrett wrote: > Right now it seems to me that -Wstringop-truncation is seriously > buggy, both in the false positive and false negative directions. This > seems to be true at least through 9.x for some recent x. And all of > the occurrences of

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

2019-10-11 Thread Kim Barrett
> On Oct 11, 2019, at 3:29 PM, Chris Plummer wrote: > Second, can the macros be used just before and after the strncpy reference > rather than around the whole function? It would clarify both that the strncpy > use has these macros in place and that the macros are there for the strncpy. > As it

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

2019-10-11 Thread Kim Barrett
> On Oct 11, 2019, at 3:29 PM, Chris Plummer wrote: > > Hi Yasumasa, > > A couple of comments on (B). First there is a typo in "stringop-truncatino". > Second, can the macros be used just before and after the strncpy reference > rather than around the whole function? It would clarify both that

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

2019-10-10 Thread Kim Barrett
> On Oct 10, 2019, at 4:54 PM, Kim Barrett wrote: > This is a > relatively recent warning option (introduced in gcc8, and included in > -Wall) Maybe it was added to -Wall in gcc9, since we aren’t seeing these warnings with gcc8?

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

2019-10-10 Thread Kim Barrett
> On Oct 10, 2019, at 3:03 AM, David Holmes wrote: > > On 10/10/2019 4:50 pm, Chris Plummer wrote: >> From JBS: >> >> /home/ysuenaga/OpenJDK/jdk/src/hotspot/share/services/diagnosticArgument.cpp:154:14: >> warning: 'char* strncpy(char*, const char*, size_t)' output truncated >> before termina

Re: RFR: 8229258: Rework markOop and markOopDesc into a simpler mark word value carrier

2019-08-16 Thread Kim Barrett
> On Aug 16, 2019, at 4:20 AM, Stefan Karlsson > wrote: > > On 2019-08-16 00:59, Kim Barrett wrote: >> src/hotspot/share/oops/markOop.hpp >> 109 template operator T(); >> My mistake in the earlier review comment. Function should be const >> qualifie

Re: RFR: 8229258: Rework markOop and markOopDesc into a simpler mark word value carrier

2019-08-15 Thread Kim Barrett
> On Aug 15, 2019, at 7:46 AM, Stefan Karlsson > wrote: > > Thanks Kim, Roman, Dan and Coleen for reviews and feedback. > > I rebased the patch, fixed more alignments, renamed the bug, and rerun the > test through tier1-3. > > https://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.

Re: RFR(XXS): 8227338: templateInterpreter.cpp: copy_table() needs to be safer

2019-07-09 Thread Kim Barrett
> On Jul 9, 2019, at 9:19 AM, Daniel D. Daugherty > wrote: > On 7/8/19 7:08 PM, Kim Barrett wrote: >> src/hotspot/share/interpreter/templateInterpreter.cpp >> 286 while (size-- > 0) *to++ = *from++; >> >> [pre-existing] >> >> This ought to

Re: RFR(XXS): 8227338: templateInterpreter.cpp: copy_table() needs to be safer

2019-07-09 Thread Kim Barrett
this change, but I don’t think you should count me toward the requisite number of reviewers. > On 7/8/19 7:00 PM, Kim Barrett wrote: >>> On Jul 7, 2019, at 8:08 PM, David Holmes wrote: >>> >>> On 7/07/2019 6:48 pm, Erik Osterlund wrote: >>>> The real dange

Re: RFR(XXS): 8227338: templateInterpreter.cpp: copy_table() needs to be safer

2019-07-08 Thread Kim Barrett
> On Jul 8, 2019, at 7:00 PM, Kim Barrett wrote: >Copy::disjoint_words_atomic(const HeapWord* from,volatile HeapWord* to, > size_t count) Or maybe Copy::disjoint_words_atomic(const volatile HeapWord* from, volatile HeapWord* to, size_t count)

Re: RFR(XXS): 8227338: templateInterpreter.cpp: copy_table() needs to be safer

2019-07-08 Thread Kim Barrett
> On Jul 6, 2019, at 9:53 AM, Daniel D. Daugherty > wrote: > > Greetings, > > During the code review for the following fix: > > JDK-8227117 normal interpreter table is not restored after single > stepping with TLH > https://bugs.openjdk.java.net/browse/JDK-8227117 > > Erik O. noticed

Re: RFR(XXS): 8227338: templateInterpreter.cpp: copy_table() needs to be safer

2019-07-08 Thread Kim Barrett
> On Jul 7, 2019, at 8:08 PM, David Holmes wrote: > > On 7/07/2019 6:48 pm, Erik Osterlund wrote: >> The real danger is SPARC though and its BIS instructions. I don’t have the >> code in front of me, but I really hope not to see that switch statement and >> non-volatile loop in that pd_disjoint

Re: RFR: 8227086: Use AS_NO_KEEPALIVE loads in HeapDumper

2019-07-02 Thread Kim Barrett
> On Jul 2, 2019, at 9:53 AM, Stefan Karlsson > wrote: > > Hi all, > > Please review this patch to read objects with AS_NO_KEEPALIVE in the > HeapDumper. > > http://cr.openjdk.java.net/~stefank/8227086/webrev.01/ > https://bugs.openjdk.java.net/browse/JDK-8227086 > > Found this while running

Re: RFR (round 6), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-12-08 Thread Kim Barrett
> On Dec 8, 2018, at 8:57 AM, Roman Kennke wrote: > > Anybody has an idea what's up with the test below? As far as I can tell, > it passes when I run locally. It appears to be an instance of https://bugs.openjdk.java.net/browse/JDK-8201248. > Thanks, > Roman > > mach5-one-rkennke-JDK-8214259-

Re: RFR (S) 8212207: runtime/InternalApi/ThreadCpuTimesDeadlock.java crashes with SEGV in pthread_getcpuclockid+0x0

2018-11-28 Thread Kim Barrett
> On Nov 28, 2018, at 2:26 AM, David Holmes wrote: > On 28/11/2018 4:30 pm, Thomas Stüfe wrote: >> P.s. >> ConcurrentGCThread::ConcurrentGCThread() : >> _should_terminate(false), _has_terminated(false) { >> }; >> I was surprised to see no invocation to the base class ctor in the >> initializer l

Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-27 Thread Kim Barrett
> On Nov 26, 2018, at 4:39 PM, Roman Kennke wrote: > *) shared-gc > - This is mostly boilerplate that is common to any GC > - referenceProcessor.cpp has a little change to make one assert not fail > (next to CMS and G1) > - taskqueue.hpp has some small adjustments to enable subclassi

Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-27 Thread Kim Barrett
> On Nov 27, 2018, at 4:46 AM, Roman Kennke wrote: > > Hi Kim, > >>> Sections to review (at this point) are the following: >>> >>> *) shared-gc >>>- This is mostly boilerplate that is common to any GC >>>- referenceProcessor.cpp has a little change to make one assert not fail >>> (next

Re: RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

2018-11-26 Thread Kim Barrett
A few quick comments (not a real review). > On Nov 26, 2018, at 4:39 PM, Roman Kennke wrote: > > Sections to review (at this point) are the following: > > *) shared-gc > - This is mostly boilerplate that is common to any GC > - referenceProcessor.cpp has a little change to make one ass

Re: RFR (S) 8212207: runtime/InternalApi/ThreadCpuTimesDeadlock.java crashes with SEGV in pthread_getcpuclockid+0x0

2018-11-20 Thread Kim Barrett
> On Nov 20, 2018, at 3:50 AM, David Holmes wrote: > > After discussions with Kim I've decided to split out the NJT list update into > a separate RFE: > > https://bugs.openjdk.java.net/browse/JDK-8214097 > > So only the change in management.cpp needs reviewing and testing. > > Updated webrev:

Re: RFR: 8183542: Factor out serial GC specific code from GenCollectedHeap into its own subclass

2017-11-01 Thread Kim Barrett
> On Oct 25, 2017, at 4:07 AM, Roman Kennke wrote: > > Hi Kim, hi Jini, > > thank you both for your reviews! > > I think I need a sponsor now. The final webrev (same as before plus > Reviewed-by line): > http://cr.openjdk.java.net/~rkennke/8183542/webrev.03/ >

Re: RFR: 8183542: Factor out serial GC specific code from GenCollectedHeap into its own subclass

2017-10-24 Thread Kim Barrett
> On Oct 19, 2017, at 12:39 PM, Roman Kennke wrote: > > Am 18.10.2017 um 22:41 schrieb Kim Barrett: >>> On Oct 18, 2017, at 4:04 PM, Roman Kennke wrote: >>> >>> Am 18.10.2017 um 20:41 schrieb Kim Barrett: >>>>> On Oct 18, 2017, at 8:08

Re: RFR(S): 8173013: JVMTI tagged object access needs G1 pre-barrier

2017-02-07 Thread Kim Barrett
> On Feb 6, 2017, at 10:44 PM, David Holmes wrote: >>> Does the SATB occur at a global safepoint? >> >> SATB is the approach G1 uses for a part of concurrent collection. It >> defines the invariants that must be maintained by the mutator so that >> the collector can find all the live objects. T

Re: RFR(S): 8173013: JVMTI tagged object access needs G1 pre-barrier

2017-02-06 Thread Kim Barrett
> On Feb 5, 2017, at 10:19 PM, David Holmes wrote: > Thanks for explaining. I must say that I find this treatment rather ad-hoc as > we seem to be discovering by trial-and-error where these places occur. I > would have hoped this was all encapsulated within the weakreference code > itself (in t

Re: RFR(S): 8173013: JVMTI tagged object access needs G1 pre-barrier

2017-02-05 Thread Kim Barrett
1543 G1SATBCardTableModRefBS::enqueue(o); > 1544 } > 1545 #endif > —— Looks good. > > Thanks, > Sangheon > > > On 02/03/2017 01:10 PM, sangheon wrote: >> Hi Kim, >> >> On 02/03/2017 01:05 PM, Kim Barrett wrote: >>>> On

Re: RFR(S): 8173013: JVMTI tagged object access needs G1 pre-barrier

2017-02-03 Thread Kim Barrett
> On Feb 3, 2017, at 1:41 PM, sangheon wrote: >> "The reference in this tag map could be the only (implicitly weak) >> reference to that object. If we hand it out we need to keep it live wrt >> SATB marking similar to other j.l.ref.Reference referents." > I like this comment. > If other reviewers

Re: RFR: 8158150: LogConfiguration::describe output can get truncated

2016-05-30 Thread Kim Barrett
> On May 30, 2016, at 10:52 AM, Robbin Ehn wrote: > > Hi all, > > The log configuration string for an output can be longer than O_BUFLEN. > The maximum size increase for each new tag set. > > Webrev: http://cr.openjdk.java.net/~rehn/8158150/webrev/ > Bug: https://bugs.openjdk.java.net/browse/JD

  1   2   >