RFR: 8194406: Use Atomic::replace_if_null

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

RFR: 8194406: Use Atomic::replace_if_null

Kim Barrett
Please review this change to use Atomic::replace_if_null where
applicable.

CR:
https://bugs.openjdk.java.net/browse/JDK-8194406

Webrev:
http://cr.openjdk.java.net/~kbarrett/8194406/open.00/

Testing:
Mach5 hs-tier1,2,3

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194406: Use Atomic::replace_if_null

David Holmes
Hi Kim,

On 3/01/2018 2:02 PM, Kim Barrett wrote:
> Please review this change to use Atomic::replace_if_null where
> applicable.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8194406
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8194406/open.00/

Looks good, but I also see:

./share/utilities/bitMap.cpp:    if (Atomic::cmpxchg(table,
&_pop_count_table, (BitMap::idx_t*)NULL) != NULL) {

Thanks,
David

> Testing:
> Mach5 hs-tier1,2,3
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194406: Use Atomic::replace_if_null

Kim Barrett
> On Jan 2, 2018, at 11:41 PM, David Holmes <[hidden email]> wrote:
>
> Hi Kim,
>
> On 3/01/2018 2:02 PM, Kim Barrett wrote:
>> Please review this change to use Atomic::replace_if_null where
>> applicable.
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8194406
>> Webrev:
>> http://cr.openjdk.java.net/~kbarrett/8194406/open.00/
>
> Looks good, but I also see:

Thanks.

> ./share/utilities/bitMap.cpp:    if (Atomic::cmpxchg(table, &_pop_count_table, (BitMap::idx_t*)NULL) != NULL) {

Thanks for spotting that one.

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194406: Use Atomic::replace_if_null

coleen.phillimore
In reply to this post by Kim Barrett
Looks good.
Coleen

On 1/2/18 11:02 PM, Kim Barrett wrote:

> Please review this change to use Atomic::replace_if_null where
> applicable.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8194406
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8194406/open.00/
>
> Testing:
> Mach5 hs-tier1,2,3
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194406: Use Atomic::replace_if_null

Kim Barrett
> On Jan 3, 2018, at 9:16 AM, [hidden email] wrote:
>
> Looks good.
> Coleen

Thanks.

>
> On 1/2/18 11:02 PM, Kim Barrett wrote:
>> Please review this change to use Atomic::replace_if_null where
>> applicable.
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8194406
>>
>> Webrev:
>> http://cr.openjdk.java.net/~kbarrett/8194406/open.00/
>>
>> Testing:
>> Mach5 hs-tier1,2,3


Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194406: Use Atomic::replace_if_null

Kim Barrett
In reply to this post by Kim Barrett
> On Jan 3, 2018, at 3:15 AM, Kim Barrett <[hidden email]> wrote:
>
>> On Jan 2, 2018, at 11:41 PM, David Holmes <[hidden email]> wrote:
>
>> ./share/utilities/bitMap.cpp:    if (Atomic::cmpxchg(table, &_pop_count_table, (BitMap::idx_t*)NULL) != NULL) {
>
> Thanks for spotting that one.

I missed that one because my grep pattern didn't include qualified
type names in the cast.  Fixing my search, I didn't find any other
calls to update.

Here's the updated webrev:
http://cr.openjdk.java.net/~kbarrett/8194406/open.01/

Only change is to bitMap.cpp.

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194406: Use Atomic::replace_if_null

David Holmes
Looks good.

Thanks,
David

On 4/01/2018 5:57 PM, Kim Barrett wrote:

>> On Jan 3, 2018, at 3:15 AM, Kim Barrett <[hidden email]> wrote:
>>
>>> On Jan 2, 2018, at 11:41 PM, David Holmes <[hidden email]> wrote:
>>
>>> ./share/utilities/bitMap.cpp:    if (Atomic::cmpxchg(table, &_pop_count_table, (BitMap::idx_t*)NULL) != NULL) {
>>
>> Thanks for spotting that one.
>
> I missed that one because my grep pattern didn't include qualified
> type names in the cast.  Fixing my search, I didn't find any other
> calls to update.
>
> Here's the updated webrev:
> http://cr.openjdk.java.net/~kbarrett/8194406/open.01/
>
> Only change is to bitMap.cpp.
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194406: Use Atomic::replace_if_null

Kim Barrett
> On Jan 4, 2018, at 6:54 AM, David Holmes <[hidden email]> wrote:
>
> Looks good.

Thanks.

>
> Thanks,
> David
>
> On 4/01/2018 5:57 PM, Kim Barrett wrote:
>>> On Jan 3, 2018, at 3:15 AM, Kim Barrett <[hidden email]> wrote:
>>>
>>>> On Jan 2, 2018, at 11:41 PM, David Holmes <[hidden email]> wrote:
>>>
>>>> ./share/utilities/bitMap.cpp:    if (Atomic::cmpxchg(table, &_pop_count_table, (BitMap::idx_t*)NULL) != NULL) {
>>>
>>> Thanks for spotting that one.
>> I missed that one because my grep pattern didn't include qualified
>> type names in the cast.  Fixing my search, I didn't find any other
>> calls to update.
>> Here's the updated webrev:
>> http://cr.openjdk.java.net/~kbarrett/8194406/open.01/
>> Only change is to bitMap.cpp.