RFR: 8196876: OopStorage::assert_at_safepoint clashes with assert_at_safepoint macros in g1CollectedHeap.hpp

[Kim Barrett]

> On Mar 2, 2018, at 7:50 AM, coleen.phillimore at oracle.com wrote:
> 
> 
> 
> On 3/1/18 10:23 PM, Kim Barrett wrote:
>> Please remove this fix for a macro name collision.
>> 
>> g1CollectedHeap.hpp contains a collection of assertion macros,
>> embedded in the middle of the class definition.  A couple of them
>> could be quite widely useful: assert_at_safepoint and
>> assert_not_at_safepoint.  The specific implementations here aren't
>> really what we'd want elsewhere, and assert_at_safepoint has some
>> extra stuff about whether the current thread is the VM thread, which
>> is similarly not what we'd want elsewhere.  And it collides with a
>> helper function in OopStorage.
>> 
>> Moved assert_at_safepoint() and assert_not_at_safepoint() macros to
>> runtime/safepoint.hpp, along with a pair of associated macros that let
>> the caller provide the failure message.  These have the "obvious"
>> implementations using SafepointSynchronize::is_at_safepoint() and
>> assert.
>> 
>> The assert_at_safepoint macro that was in g1CollectedHeap.hpp is now
>> called assert_at_safepoint_on_vm_thread, since that's the test that
>> all but one of the (G1) callers wanted.  The only outlier was
>> DirtyCardQueueSet::apply_closure_during_gc, and it doesn't really care
>> whether it's called from the VM thread.
>> 
>> The colliding OopStorage function has been removed; OopStorage now
>> just uses the new shared macro.
>> 
>> There are a large number of places that could use the new safepoint
>> assertion macros; that's a cleanup I'm going to leave for later.
> 
> Yes, please leave for later.  The rest of the code can migrate to the new macro eventually now that it's there.
> 
> Change looks good.
> 
> Thanks,
> Coleen

Thanks.

>> 
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8196876
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~kbarrett/8196876/open.00/
>> 
>> Testing:
>> mach5 {hs,jdk}-tier{1,2,3}