RFR: 8234748: Clean up atomic and orderAccess includes

[David Holmes]

On 27/11/2019 8:34 pm, Stefan Karlsson wrote:
> Hi Thomas,
> 
> On 2019-11-27 11:20, Thomas Schatzl wrote:
>> Hi Stefan,
>>
>>    thanks for tackling this.
>>
>> On 26.11.19 12:11, Stefan Karlsson wrote:
>>> Hi all,
>>>
>>> Please review this trivial, but large, patch to cleanup the includes 
>>> of atomic.hpp and orderAccess.hpp.
>>>
>>> https://cr.openjdk.java.net/~stefank/8234748/webrev.01/
>>> https://bugs.openjdk.java.net/browse/JDK-8234748
>>>
>>> Thanks,
>>> StefanK
>>
>> I did the following bash-fu to find missing/superfluous includes:
>>
>>   grep "Atomic::" `find . -name '*.?pp'` | sed 's/\(.*pp\):.*/\1/' | 
>> uniq | sort > users.txt
>> $ grep "atomic.hpp" `find . -name '*.?pp'` | sed 's/\(.*\):.*/\1/' | 
>> uniq | sort > includers.txt
>> $ diff users.txt includers.txt > diff.txt
>>
>> diff.txt then contained the differences, with some false positives, 
>> all about containing the keywords in comments. Otherwise I think this 
>> should be complete.
>>
>> The same has been done with "OrderAccess::" and "orderAccess.hpp".
>>
>> Here are the results:
>>
>> Improvements to orderAccess.hpp includes:
>>
>> - gc/shenandoah/shenandoahVerifier.cpp misses it
>>
>> - the #include from gc/z/zLiveMap.inline.hpp should maybe be moved to 
>> zLiveMap.cpp
>>
>> Improvements to atomic.hpp includes:
>>
>> In the following files the include of atomic.hpp should be removed as 
>> it seems unnecessary:
>> - share/utilities/bitMap.hpp
>> - share/oops/oop.hpp
> 
> These needs atomic.hpp since they use atomic_memory_order.
> 
>> - share/gc/z/zNMethodTable.cpp
>> - share/gc/shenandoah/shenandoahForwarding.inline.hpp
>> - share/gc/g1/g1CardTable.cpp
>> - os_cpu/solaris_x86/os_solaris_x86.cpp
>>
>> The following need a #include atomic.hpp:
>> - share/oops/methodData.cpp
>> - share/oops/constantPool.cpp
>> - share/memory/metaspace/virtualSpaceNdoe.cpp
>> - os/posix/os_posix.cpp
>>
>> Looks good otherwise.
> 
> Since, I already pushed the original bug, I created a new with your 
> changes:
> https://cr.openjdk.java.net/~stefank/8234897/webrev.01/

I've verified each of those as well.

Thanks,
David

> 
> I intend to take this through our build steps, and push it if it's 
> succeeds.
> 
> Thanks,
> StefanK
> 
>>
>> Thanks,
>>    Thomas