RFR: 8196876: OopStorage::assert_at_safepoint clashes with assert_at_safepoint macros in g1CollectedHeap.hpp (original) (raw)

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: assertatsafepoint and assertnotatsafepoint. The specific implementations here aren't really what we'd want elsewhere, and assertatsafepoint 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 assertatsafepoint() and assertnotatsafepoint() 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::isatsafepoint() and assert. The assertatsafepoint macro that was in g1CollectedHeap.hpp is now called assertatsafepointonvmthread, since that's the test that all but one of the (G1) callers wanted. The only outlier was DirtyCardQueueSet::applyclosureduringgc, 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

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