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
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
> 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()) {
>>>
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
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()
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
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
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
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");
>>>
>>>
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:
>
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)':
>>> >
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)':
>>>
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
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
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
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
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.
17 matches
Mail list logo
