RFR: 8199946: Move load/store and encode/decode out of oopDesc (original) (raw)

Stefan Karlsson stefan.karlsson at oracle.com
Fri Mar 23 15:54:46 UTC 2018

Hi Thomas,

On 2018-03-23 16:50, Thomas Schatzl wrote:

Hi,

looks good with minor caveats: There is another at psScavenge.inline.hpp 111 RawAccess<>::oopstore(p, newobj);

I'll fix.

and metaspaceShared.cpp 1940 RootAccess::oopstore(p, CompressedOops::decode(o)); has been a decodeheapoopnotnull()

I'll fix.

------ Depending on whether you think this fits this change, it would be nice to fix some code in the following locations. Otherwise I can do a separate change. g1OopClosures.inline.hpp: 244 RawAccess<>::oopstore(p, forwardee); This could be a RawAccess::oopstore() g1ParScanThreadState.inline.hpp: 49 RawAccess<>::oopstore(p, obj); Same here.

I had changes to do this, but I decided to revert those changes for this patch. A new RFE for this would be good.

Thanks, StefanK

Thanks, Thomas

On Fri, 2018-03-23 at 13:27 +0100, Stefan Karlsson wrote: On 2018-03-23 13:28, Erik Österlund wrote: Hi Stefan,

Looks good. There is one remaining OOPNOTNULL in psScavenge.inline.hpp, but I do not need another webrev. Fixed. Thanks for the review! StefanK

Thanks, /Erik On 2018-03-22 20:28, Stefan Karlsson wrote: Erik found two places where I didn't use OOPNOTNULL, where we previously used notnull functions. diff --git a/src/hotspot/share/gc/cms/parNewGeneration.cpp b/src/hotspot/share/gc/cms/parNewGeneration.cpp --- a/src/hotspot/share/gc/cms/parNewGeneration.cpp +++ b/src/hotspot/share/gc/cms/parNewGeneration.cpp @@ -734,7 +734,7 @@ oop newobj = obj->isforwarded() ? obj->forwardee() : g->DefNewGeneration::copytosurvivorspace(obj); - RawAccess<>::oopstore(p, newobj); + RawAccess::oopstore(p, newobj); } if (gcbarrier) { // If p points to a younger generation, mark the card. diff --git a/src/hotspot/share/gc/cms/parOopClosures.inline.hpp b/src/hotspot/share/gc/cms/parOopClosures.inline.hpp --- a/src/hotspot/share/gc/cms/parOopClosures.inline.hpp +++ b/src/hotspot/share/gc/cms/parOopClosures.inline.hpp @@ -52,7 +52,7 @@ newobj = ((ParNewGeneration*)g)->copytosurvivorspace(parscanstate, obj, objsz, m); } - RawAccess<>::oopstore(p, newobj); + RawAccess::oopstore(p, newobj); } } StefanK On 2018-03-22 17:01, Stefan Karlsson wrote: Hi,

This patch needs Erik's change to the LoadProxies: http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2018-Marc h/021504.html

to build on fastdebug. Here's a rebased patch: http://cr.openjdk.java.net/~stefank/8199946/webrev.02/ Thanks, StefanK On 2018-03-21 18:27, Stefan Karlsson wrote: Hi all, Please review this patch to get rid of the oopDesc::load/store functions and to move the oopDesc::encode/decode functions to a new CompressedOops subsystem. http://cr.openjdk.java.net/~stefank/8199946/webrev.01 https://bugs.openjdk.java.net/browse/JDK-8199946 When the Access API was introduced many of the usages of oopDesc::loaddecodeheapoop, and friends, were replaced by calls to the Access API. However, there are still some usages of these functions, most notably in the GC code. This patch is two-fold: 1) It replaces the oopDesc load and store calls with RawAccess equivalents. 2) It moves the oopDesc encode and decode functions to a new, separate, subsystem called CompressedOops. A future patch could even move all the Universe::narrowoop variables over to CompressedOops. The second part has the nice property that it breaks up a circular dependency between oop.inline.hpp and access.inline.hpp. After the change we have: oop.inline.hpp includes: access.inline.hpp compressedOops.inline.hpp access.inline.hpp includes: compressedOops.inline.hpp Thanks, StefanK

More information about the hotspot-dev mailing list