RFR: JDK-8222281: GC interface for load-klass: runtime part

[Roman Kennke]

Am 15.04.19 um 08:17 schrieb Per Liden:
> Hi Roman,
>
> On 04/12/2019 05:13 PM, Roman Kennke wrote:
>> Alright, I am experimenting with this. It turns out that there are a
>> *lot* of paths that call into oopDesc::klass() which need extra
>> handling, all of which come out of the GC. Example is oopDesc::size()
>> and oopDesc::oop_iterate() and various others.
>
> It's till not quite clear to me why you need this. Put another way, I 
> think you should really try to design this such that you don't need 
> this. Doing things like size() or oop_iterate() on from-space objects 
> sounds like there's an abstraction/load-barrier missing somewhere.

It's GC code: e.g. when I want to evacuate an object, I need to know its 
size, and we're racing with other threads to do the same, possibly.

Said that, I've come up with a way to handle it nicely, and actually 
improves some code paths. Stay tuned!

Roman

> I can understand if the GC sometimes needs to fiddle with from-space 
> objects, but that should be a Shenandoah internal thing and shouldn't 
> have to leak out into BarrierSet, etc.
>
> Btw, once your conversion to to-space invariant is complete (is it?), 
> I'm hoping we can start to cleaning out all the stuff we no longer 
> need from BarrierSet and the Access API.
>
> cheers,
> Per
>
>>
>> I am wondering if a simple BarrierSet::obj_klass() might be
>> useful/feasible for such cases? I can work around some of those
>> occurances by calling variants that take a Klass* parameter instead,
>> and resolve that internally in the GC. But not always. For example, I
>> can call oopDesc::size_given_klass() instead. However, that blows up
>> later because objArrayKlass::obj_size() is asserting klass()-
>>> is_objArrayKlass() or something like that later...
>>
>> Ideas?
>>
>> Roman
>>
>>> On 4/12/19 8:23 AM, Roman Kennke wrote:
>>>>
>>>> Am 12. April 2019 07:18:41 MESZ schrieb Per Liden <
>>>> per.liden at oracle.com>:
>>>>> Hi Roman,
>>>>>
>>>>> On 04/11/2019 10:58 PM, Roman Kennke wrote:
>>>>>> An upcoming feature in Shenandoah requires that GC can
>>>>>> intercept
>>>>> loading
>>>>>> the Klass* of an object. I'd like to introduce a GC interface
>>>>>> for
>>>>> that.
>>>>>
>>>>> Could you please give some more insight into what this feature is
>>>>> and
>>>>> why it needs to intercept these loads?
>>>>
>>>> Oh yeah, apparently it was late last night. ;-) We want to
>>>> eliminate Shenandoah's extra word for the forwarding pointer and
>>>> stick it in the Klass* word for forwarded objects, with a little
>>>> bit of encoding to distinguish a forward pointer from a Klass*.
>>>> Therefore we need to see Klass* loads to check that encoding and
>>>> possibly load the Klass* from the forwardee instead.
>>>
>>> Maybe I'm missing something, but didn't you just switch to having a
>>> to-space invariant? What runtime code can get hold of a an oop
>>> pointing
>>> into from-space and then try load its klass?
>>>
>>> cheers,
>>> Per
>>>
>>>> Roman
>>>>
>>>>> thanks,
>>>>> Per
>>>>>
>>>>>> This change covers the runtime part.
>>>>>>
>>>>>> Bug:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8222281
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~rkennke/JDK-8222281/webrev.00/
>>>>>>
>>>>>> The change takes the 3 variants of oopDesc::klass() and funnels
>>>>>> them
>>>>>> through the Access API to be intercepted by the GC in any way
>>>>>> it
>>>>> wants.
>>>>>> Behaviour- and performance-wise it should be identical
>>>>>> (assuming the
>>>>>> compiler can make sense of the Access API).
>>>>>>
>>>>>> I see that there might be an opportunity here to make the if
>>>>>> (UseCompressedClassPointers) check pre-resolved, but I don't
>>>>>> know how
>>>>> to
>>>>>> do that. I also don't really see how that is supposed to work
>>>>>> for
>>>>> loads
>>>>>> and stores wrt UseCompressedOops either: in order to select the
>>>>> proper
>>>>>> functions on first call, and subsequently go through that
>>>>>> selected
>>>>>> function, it would have to be done in the *_init() functions.
>>>>>> But I
>>>>>> don't see any selection code there. Instead, it seems to be in
>>>>>> PreRuntimeDispatch? I must be missing something. I left the
>>>>>> selection
>>>>> in
>>>>>> the raw implementation. Maybe we want to sort this out?
>>>>>>
>>>>>> <rant>
>>>>>> I must say that I fought with myself whether or not I should
>>>>>> add to
>>>>> the
>>>>>> madness that is the Access API. What should have been a
>>>>> 1-line-addition
>>>>>> to an API (e.g. BarrierSet) turned out to become:
>>>>>>
>>>>>>     7 files changed, 86 insertions(+), 20 deletions(-)
>>>>>>
>>>>>> and two days of work. And god forbid we ever have to change or
>>>>>> even
>>>>> fix
>>>>>> anything there.
>>>>>>
>>>>>> I was about to just not do it in Access API at all, but
>>>>>> somewhere
>>>>> else
>>>>>> instead, but then I did not want to introduce a schism there,
>>>>>> so I
>>>>> bit
>>>>>> the bullet. Maybe we should consider to turn this into a proper
>>>>>> C++
>>>>>> interface instead? This is just unmaintainable madness.
>>>>>> </rant>
>>>>>>
>>>>>> Testing: tier1 fine. Will submit into jdk/submit shortly, works
>>>>>> with
>>>>> the
>>>>>> Shenandoah prototype that I have here (iow, the API is good)
>>>>>>
>>>>>> Can I please get a review?
>>>>>>
>>>>>> Thanks,
>>>>>> Roman
>>>>>>