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