RFR: 8197999: Accessors in typeArrayOopDesc should use new Access API

Erik Osterlund erik.osterlund at oracle.com
Mon Feb 19 17:05:56 UTC 2018


And I replied to the wrong email as usual.
Disregard this please.

/Erik

> On 19 Feb 2018, at 17:58, Erik Österlund <erik.osterlund at oracle.com> wrote:
> 
> Hi Roman,
> 
> I see there is a need to resolve a stable address for some objects to bulk access primitives. The code base is full of assumptions that no barriers are needed for such address resolution. It looks like the proposed approach is to one by one hunt down all such callsites. I could find some places where such barriers are missing.
> 
> To make the code as maintainable as possible, I would like to propose a slightly different take on this, and would love to hear if this works for Shenandoah or not. The main idea is to annotate places where we do *not* want GC address resolution for internal pointers to objects, instead of where we want it, as it seems to be the common case that we do want to resolve the address.
> 
> In some more detail:
> 
> 1) Rip out the *_addr fascilities not used (a whole bunch on oopDesc).
> 2) Ignore the difference between read/write resolution (write resolution handles both reads and writes). Instead introduce an oop resolve_stable_addr(oop) function in Access. This makes it easier to use.
> 3) Identify as few callsites as possible for this function. I'm thinking arrayOop::base() and a few strange exceptions.
> 4) Identify the few places where we explicitly do *not* want address resolution, like calls from GC, and replace them with *_addr_raw variants.
> 5) Have a switch in barrierSetConfig.hpp that determines whether the build needs to support not to-space invariant GCs or not.
> 
> With these changes, the number of callsites have been kept down to what I believe to be a minimum. And yet it covers some callsites that you accidentally missed (e.g. jvmciCodeInstaller.cpp). Existing uses of the various *_addr fascilities can in most cases continue to do what they have done in the past. And new uses will not be surprised that they accidentally missed some barriers. It will be solved automagically.
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/typearray_resolve/webrev.00/
> 
> Please let me know what you think about this style and whether that works for you or not. I have not done proper testing yet, but presented this patch for quicker turn-around so we can synchronize the direction first.
> 
> Thanks,
> /Erik
> 
> 
>> On 2018-02-15 23:59, Roman Kennke wrote:
>> I forgot to mention, I tested this successfully using hotspot_tier1
>> test group with and without precompiled headers.
>> 
>> Thanks,
>> Roman
>> 
>>> On Thu, Feb 15, 2018 at 11:54 PM, Roman Kennke <rkennke at redhat.com> wrote:
>>> The accessors in typeArrayOop.hpp still access the heap directly, but
>>> they should go through the new Access API instead (so that GCs can
>>> intercept it, if they wish). This changeset fixes this.
>>> 
>>> In order to avoid excessive inclusion of stuff (esp. into
>>> precompiled.hpp), the actual code has been moved into
>>> typeArrayOop.inline.hpp, and corresponding includes added whereever
>>> needed.
>>> 
>>> Webrev:
>>> http://cr.openjdk.java.net/~rkennke/8197999/webrev.00/
>>> 
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8197999
>>> 
>>> Please review!
>>> 
>>> Thanks, Roman
> 



More information about the hotspot-runtime-dev mailing list