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

Erik Österlund erik.osterlund at oracle.com
Mon Feb 19 16:58:47 UTC 2018


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