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

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Fri Mar 2 12:50:09 UTC 2018

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.

> 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}

More information about the hotspot-dev mailing list