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

On Tue, 15 Sep 2020 01:55:39 GMT, David Holmes wrote: >> Daniel D. Daugherty has updated the pull request incrementally with one >> additional commit since the last revision: >> >> rkennke, coleenp, fisk CR - delete random assert() that knows too much >> about markWords. > > Marked as

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

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

Re: RFR: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents

On 14/09/2020 6:18 pm, Richard Reingruber wrote: On Mon, 14 Sep 2020 04:57:21 GMT, David Holmes wrote: Hi, this is the continuation of the review of the implementation for: https://bugs.openjdk.java.net/browse/JDK-8227745 https://bugs.openjdk.java.net/browse/JDK-8233915 It allows for JIT

Re: RFR: 8252933: com.sun.tools.jdi.ObjectReferenceImpl#validateAssignment always requests referenceType

On Fri, 11 Sep 2020 01:08:53 GMT, Daniil Titov wrote: > The change fixes the regression introduced by > https://bugs.openjdk.java.net/browse/JDK-8241080. > > Method validateAssignment() in com.sun.tools.jdi.ObjectReferenceImpl now > always retrieves the reference type and that > requires one

Re: RFR: 8247281: migrate ObjectMonitor::_object to OopStorage

Hi Dan, It is always nice to return from vacation a little bit refreshed. Thank you for good words! Yes, this is already on my list. Thanks! Serguei On 9/14/20 14:20, Daniel D. Daugherty wrote: Hi Serguei, Welcome back from your vacation. When you get the chance, we need you to chime in

RFR: 8247281: migrate ObjectMonitor::_object to OopStorage

Hi Serguei, Welcome back from your vacation. When you get the chance, we need you to chime in on the following review since there are changes that affect JVM/TI and heap dumps (HPROF output): https://git.openjdk.java.net/jdk/pull/135 Here's the bug:     JDK-8247281 migrate

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

> 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 collateral > changes so this is not a trivial review. Sorry for the

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

On Mon, 14 Sep 2020 21:07:23 GMT, Daniel D. Daugherty wrote: >> I am also 100% okay with just removing the random assertion. We have a whole >> bunch of other block iteration code in GC >> code that does not sprinkle random asserts about what patterns of markWords >> are supposedly good or

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

On Mon, 14 Sep 2020 21:02:51 GMT, Erik Österlund wrote: >> I agree. Also, the assert becomes true somewhat obviously because of its >> first clause, which is already guaranteed >> because of its placement in the surrounding else-branch (unless something >> really weird happens). > > I am also

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

On Mon, 14 Sep 2020 20:51:58 GMT, Roman Kennke wrote: >> @fisk - please chime in here... > > I agree. Also, the assert becomes true somewhat obviously because of its > first clause, which is already guaranteed > because of its placement in the surrounding else-branch (unless something > really

Re: Fatal errors when running JCK tests with JDK15/16 debug build

Hi Martin, This issue looks rather a test problem. I've filled the ticket JCK-7314897 . Best Regards, Leonid On 9/7/20 3:23 AM, Doerr, Martin wrote: Hi Leonid, the errors were observed in many more vm/jvmti/Get... tests like the following

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

On Mon, 14 Sep 2020 20:48:10 GMT, Coleen Phillimore wrote: >> I found my original preliminary code review comment and Erik's reply: >> >>> src/hotspot/share/gc/shared/space.inline.hpp >>> old L166: assert(!space->scanned_block_is_obj(cur_obj) || >>> old L167:

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

On Mon, 14 Sep 2020 20:51:34 GMT, Daniel D. Daugherty wrote: >> This code seems like something that doesn't belong here anymore. This code >> assumed synchronous scanning of oops in >> ObjectMonitor and scanning memory regions, and that's no longer the case >> with OopStorage. I think this

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

On Mon, 14 Sep 2020 20:45:29 GMT, Daniel D. Daugherty wrote: >> Sorry, I confused myself switching between this review and a >> preliminary review thread. >> >> Here's the original code: >> >> 165 while (cur_obj < scan_limit) { >> 166 assert(!space->scanned_block_is_obj(cur_obj) || >>

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

On Mon, 14 Sep 2020 20:34:13 GMT, Daniel D. Daugherty wrote: >> Sorry, no. Maybe it's too late here and I shall think about it tomorrow >> morning instead ;-) Or maybe you can explain it >> again in the context of that change. How's the assert even relevant when >> moved in the else-branch? >

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

On Mon, 14 Sep 2020 20:25:39 GMT, Roman Kennke wrote: >> See this comment from Coleen and the replies: >> https://github.com/openjdk/jdk/pull/135#discussion_r487300636 >> >> Please let me know if that resolved this comment for you. > > Sorry, no. Maybe it's too late here and I shall think about

Re: RFR: 8252537: Updated @exception with @throws

On Wed, 9 Sep 2020 19:29:30 GMT, Vipin Sharma wrote: > Updated @exception with @throws for core-libs, it fixes all open sub-tasks of > JDK-8252536. > > Open Subtasks part of this fix are: > 1. JDK-8252537 > 2. JDK-8252539 > 3. JDK-8252540 > 4. JDK-8252541 > > Previous conversation on this: >

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

On Mon, 14 Sep 2020 20:21:23 GMT, Daniel D. Daugherty wrote: >> src/hotspot/share/gc/shared/space.inline.hpp line 176: >> >>> 174: assert(!space->scanned_block_is_obj(cur_obj) || >>> oop(cur_obj)->mark_raw().is_unlocked() || >>> 175:

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

On Mon, 14 Sep 2020 20:05:06 GMT, Roman Kennke wrote: >> @coleenp, @rkennke, and @kimbarrett - I believe all of the changes that you >> have requested >> have been made. Please confirm by re-reviewing. >> >> @dholmes-ora - I don't think you asked for any specific code changes. I've >> taken a

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

On Mon, 14 Sep 2020 20:10:50 GMT, Roman Kennke wrote: >> Daniel D. Daugherty has updated the pull request incrementally with one >> additional commit since the last revision: >> >> kimbarrett CR - made minor changes to address Kim's code review. > >

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

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

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

On Mon, 14 Sep 2020 18:59:06 GMT, Daniel D. Daugherty wrote: > @coleenp, @rkennke, and @kimbarrett - I believe all of the changes that you > have requested > have been made. Please confirm by re-reviewing. It looks good to me now! I've also build with Shenandoah GC, and run our sanity-tests

Re: RFR: 8252933: com.sun.tools.jdi.ObjectReferenceImpl#validateAssignment always requests referenceType

On Fri, 11 Sep 2020 16:27:57 GMT, Daniil Titov wrote: >> The changes look good to me. Can I ask how you noticed the extra unnecessary >> JDWP packet? > > Thanks @plummercj for the review. The issue with an extra JDWP packet was > reported in serviceability-dev mail list : >

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

> 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()) { >>>

Re: RFR: 8252537: Updated @exception with @throws

Hi Vipin, I've only looked at the management files. They look good in general. src/java.management/share/classes/java/lang/management/ClassLoadingMXBean.java 108 * @throws java.lang.SecurityException if a security manager 109 *

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

On Mon, 14 Sep 2020 18:51:31 GMT, Daniel D. Daugherty wrote: >> @kimbarrett >> >>> src/hotspot/share/runtime/synchronizer.cpp >>> >>> The old code in chk_in_use_entry seems wrong. It checked for a null >>> object() and recorded that as an error. But then it went on and attempted >>> to use it

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

> 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 collateral > changes so this is not a trivial review. Sorry for the

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

On Mon, 14 Sep 2020 18:44:30 GMT, Daniel D. Daugherty wrote: >> @kimbarrett >> >>> src/hotspot/share/runtime/synchronizer.cpp >>> 1548 guarantee(m->object_peek() == NULL, "invariant"); >>> >>> Later: But see previous comment. Some of this might be relevat later though. >>> >>> object_peek()

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

On Mon, 14 Sep 2020 18:39:45 GMT, Daniel D. Daugherty wrote: >> @kimbarrett >> >>> src/hotspot/share/runtime/objectMonitor.cpp >>> 249 guarantee(self->is_Java_thread() || self->is_VM_thread(), "must be"); >>> 250 if (self->is_Java_thread()) { >>> >>> Maybe instead >>> >>> if

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

On Mon, 14 Sep 2020 18:36:28 GMT, Daniel D. Daugherty wrote: >> @kimbarrett >> >>> src/hotspot/share/runtime/objectMonitor.cpp >>> 244 // Check that object() and set_object() are called from the right >>> context: >>> 245 static void check_object_context() { >>> >>> This seems like checking

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

On Mon, 14 Sep 2020 18:33:17 GMT, Daniel D. Daugherty wrote: >> @dholmes-ora and @fisk - I've taken a first pass at creating a CSR: >> JDK-8253121 migrate ObjectMonitor::_object to OopStorage >> https://bugs.openjdk.java.net/browse/JDK-8253121 >> >> Please look it over and feel free to edit as

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

On Mon, 14 Sep 2020 18:35:29 GMT, Daniel D. Daugherty wrote: >> @kimbarrett: >> >>> src/hotspot/share/oops/weakHandle.cpp >>> 36 WeakHandle::WeakHandle(OopStorage* storage, oop obj) : >>> 37 _obj(storage->allocate()) { >>> 38 assert(obj != NULL, "no need to create weak null oop"); >>> >>>

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

On Mon, 14 Sep 2020 16:47:27 GMT, Daniel D. Daugherty wrote: >> @rkennke - I've commited the changes in your webrev as >> https://github.com/openjdk/jdk/pull/135/commits/bbf8dbd09bdf5c1c77c67cc637fbc10fe72d4894. > > @dholmes-ora and @fisk - I've taken a first pass at creating a CSR: >

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

On Mon, 14 Sep 2020 02:00:05 GMT, David Holmes wrote: >> Thanks for confirmation. > > From the spec I'm not clear on exactly what JVMTI_HEAP_REFERENCE_MONITOR is > intended to be. Serviceability folk should > be giving some input here though. I've taken a first pass at creating a CSR:

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

On Mon, 14 Sep 2020 01:52:04 GMT, David Holmes wrote: >> Thanks for confirmation. > > I don't see anything in the HPROF format description that claims this is a > strong root. At a minimum this seems to be a > behavioural change that would warrant a CSR request. This also seems to be >

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

On Mon, 14 Sep 2020 16:16:53 GMT, Daniel D. Daugherty wrote: >>> > Shenandoah changes are not complete: >>> > /home/rkennke/src/openjdk/jdk/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp: >>> > In member function 'virtual >>> > void ShenandoahFinalMarkingTask::work(uint)': >>> >

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

On Mon, 14 Sep 2020 14:27:51 GMT, Roman Kennke wrote: >>> Shenandoah changes are not complete: >>> /home/rkennke/src/openjdk/jdk/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp: >>> In member function 'virtual >>> void ShenandoahFinalMarkingTask::work(uint)': >>>

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

> 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 collateral > changes so this is not a trivial review. Sorry for the

Re: RFR: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents

On Mon, 14 Sep 2020 04:58:47 GMT, David Holmes wrote: >> Hi, >> >> this is the continuation of the review of the implementation for: >> >> https://bugs.openjdk.java.net/browse/JDK-8227745 >> https://bugs.openjdk.java.net/browse/JDK-8233915 >> >> It allows for JIT optimizations based on escape

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

On Mon, 14 Sep 2020 14:24:02 GMT, Roman Kennke wrote: > > @rkennke - Thanks for the review. I believe @fisk is going to address > > your comments. > > Actually, if I understand it correctly, OopStorage already gives us full > barriers, so we should be covered, and we can > simply revert

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

On Mon, 14 Sep 2020 02:01:58 GMT, David Holmes wrote: > Not sure about the JVM TI changes! Here is a generic comment. You mention that the specification doesn't make it clear whether the roots reported are strong or weak. This is true. There is no mention about roots being strong or weak

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

On Mon, 14 Sep 2020 13:48:25 GMT, Daniel D. Daugherty wrote: > @rkennke - Thanks for the review. I believe @fisk is going to address > your comments. Actually, if I understand it correctly, OopStorage already gives us full barriers, so we should be covered, and we can simply revert

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

On Mon, 14 Sep 2020 14:15:41 GMT, Erik Österlund wrote: >> @rkennke - Thanks for the review. I believe @fisk is going to address >> your comments. > > Hi Kim, > > Here is a partial reply to your review. Thanks for reviewing! > Hmm seems like your email was only sent to shenandoah-dev. Not sure

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

On Mon, 14 Sep 2020 13:48:25 GMT, Daniel D. Daugherty wrote: >> @kimbarrett - Thanks for the review. I believe @fisk is going to >> address your comments. > > @rkennke - Thanks for the review. I believe @fisk is going to address > your comments. Hi Kim, Here is a partial reply to your review.

Re: RFR: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents

On Mon, 14 Sep 2020 04:57:21 GMT, David Holmes wrote: >> Hi, >> >> this is the continuation of the review of the implementation for: >> >> https://bugs.openjdk.java.net/browse/JDK-8227745 >> https://bugs.openjdk.java.net/browse/JDK-8233915 >> >> It allows for JIT optimizations based on escape

Re: RFR: JDK-8247589: Implementation of Alpine Linux/x64 Port [v2]

On Mon, 14 Sep 2020 04:18:39 GMT, David Holmes wrote: >> Aleksei Voitylov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> JDK-8247589: Implementation of Alpine Linux/x64 Port > > Marked as reviewed by dholmes (Reviewer). thank you

Re: RFR: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents

On Thu, 10 Sep 2020 20:48:23 GMT, Richard Reingruber wrote: > Hi, > > this is the continuation of the review of the implementation for: > > https://bugs.openjdk.java.net/browse/JDK-8227745 > https://bugs.openjdk.java.net/browse/JDK-8233915 > > It allows for JIT optimizations based on escape