RFR(S): 8189425: Minor updates in support of closed changes

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR(S): 8189425: Minor updates in support of closed changes

Erik Gahlin
Hi,

Could I have a review of this change that will adjust an assertion and
remove a lock associated with JFR.

Webrev:
http://cr.openjdk.java.net/~egahlin/8189425_0

Bug:
https://bugs.openjdk.java.net/browse/JDK-8189425

Thanks
Erik


Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8189425: Minor updates in support of closed changes

David Holmes
Hi Erik,

On 18/10/2017 12:23 PM, Erik Gahlin wrote:
> Hi,
>
> Could I have a review of this change that will adjust an assertion and

Can you explain the adjustment please.

> remove a lock associated with JFR.

That bit is fine :)

Thanks,
David
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8189425: Minor updates in support of closed changes

Erik Gahlin
Hi David,

> Hi Erik,
>
> On 18/10/2017 12:23 PM, Erik Gahlin wrote:
>> Hi,
>>
>> Could I have a review of this change that will adjust an assertion and
>
> Can you explain the adjustment please.
We have closed code that modifies the mark word and then changes it back
during a safepoint. When the mark word is modified, we reuse GC
infrastructure that run into the assert. If we change the assert to
ignore checking that the mark word is NULL, we don't run into the problem.

>
>> remove a lock associated with JFR.
>

I forgot to modify the header file, see updated webrev.

http://cr.openjdk.java.net/~egahlin/8189425_1/

I  also made a change to GrowableArray, the insert_sorted method now
takes a const.

Thanks
Erik

> That bit is fine :)
>
> Thanks,
> David
>
>> Webrev:
>> http://cr.openjdk.java.net/~egahlin/8189425_0
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8189425
>>
>> Thanks
>> Erik
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8189425: Minor updates in support of closed changes

David Holmes
Hi Erik,

On 19/10/2017 6:04 AM, Erik Gahlin wrote:

> Hi David,
>
>> Hi Erik,
>>
>> On 18/10/2017 12:23 PM, Erik Gahlin wrote:
>>> Hi,
>>>
>>> Could I have a review of this change that will adjust an assertion and
>>
>> Can you explain the adjustment please.
> We have closed code that modifies the mark word and then changes it back
> during a safepoint. When the mark word is modified, we reuse GC
> infrastructure that run into the assert. If we change the assert to
> ignore checking that the mark word is NULL, we don't run into the problem.

Ok. This weakens the assert somewhat but then I don't know what it is
really trying to catch.

>>
>>> remove a lock associated with JFR.
>>
>
> I forgot to modify the header file, see updated webrev.
>
> http://cr.openjdk.java.net/~egahlin/8189425_1/

Okay.

> I  also made a change to GrowableArray, the insert_sorted method now
> takes a const.

Presumably ok.

Thanks,
David

> Thanks
> Erik
>
>> That bit is fine :)
>>
>> Thanks,
>> David
>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~egahlin/8189425_0
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8189425
>>>
>>> Thanks
>>> Erik
>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

RE: RFR(S): 8189425: Minor updates in support of closed changes

Markus Gronlund
In reply to this post by Erik Gahlin
Hi Erik,

Looks good.

Thanks
Markus

-----Original Message-----
From: Erik Gahlin
Sent: den 18 oktober 2017 22:05
To: David Holmes; [hidden email]
Subject: Re: RFR(S): 8189425: Minor updates in support of closed changes

Hi David,

> Hi Erik,
>
> On 18/10/2017 12:23 PM, Erik Gahlin wrote:
>> Hi,
>>
>> Could I have a review of this change that will adjust an assertion
>> and
>
> Can you explain the adjustment please.
We have closed code that modifies the mark word and then changes it back during a safepoint. When the mark word is modified, we reuse GC infrastructure that run into the assert. If we change the assert to ignore checking that the mark word is NULL, we don't run into the problem.

>
>> remove a lock associated with JFR.
>

I forgot to modify the header file, see updated webrev.

http://cr.openjdk.java.net/~egahlin/8189425_1/

I  also made a change to GrowableArray, the insert_sorted method now takes a const.

Thanks
Erik

> That bit is fine :)
>
> Thanks,
> David
>
>> Webrev:
>> http://cr.openjdk.java.net/~egahlin/8189425_0
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8189425
>>
>> Thanks
>> Erik
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8189425: Minor updates in support of closed changes

Erik Gahlin
Thanks for the review,  David and Markus!

Erik

> Hi Erik,
>
> Looks good.
>
> Thanks
> Markus
>
> -----Original Message-----
> From: Erik Gahlin
> Sent: den 18 oktober 2017 22:05
> To: David Holmes; [hidden email]
> Subject: Re: RFR(S): 8189425: Minor updates in support of closed changes
>
> Hi David,
>
>> Hi Erik,
>>
>> On 18/10/2017 12:23 PM, Erik Gahlin wrote:
>>> Hi,
>>>
>>> Could I have a review of this change that will adjust an assertion
>>> and
>> Can you explain the adjustment please.
> We have closed code that modifies the mark word and then changes it back during a safepoint. When the mark word is modified, we reuse GC infrastructure that run into the assert. If we change the assert to ignore checking that the mark word is NULL, we don't run into the problem.
>
>>> remove a lock associated with JFR.
> I forgot to modify the header file, see updated webrev.
>
> http://cr.openjdk.java.net/~egahlin/8189425_1/
>
> I  also made a change to GrowableArray, the insert_sorted method now takes a const.
>
> Thanks
> Erik
>
>> That bit is fine :)
>>
>> Thanks,
>> David
>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~egahlin/8189425_0
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8189425
>>>
>>> Thanks
>>> Erik
>>>
>>>