RFR: 8241825: Make compressed oops and compressed class pointers independent on x86_64

Mon Apr 27 15:28:30 UTC 2020

Hi Frederic,

I hear your concern about hardcoded offsets making the JVM more fragile.

First of all, let me assure you that you would not be able to run hello 
world if the offsets ever were to be
wrong. They are verified during bootstrapping in 
JavaClasses::check_offsets(), so you have to be quite skilled
to manage to break anything without immediately noticing it. You 
literally can't have run a single test or java -version
without immediately crashing. This makes this significantly less 
fragile. So I don't think I believe that this is fragile.

Having said that, I do agree it could be annoying and a pain to maintain 
and update the layout code if such changes
are occasionally incompatible with the random selection of hardcoded 
offsets, forcing new awkward updates of said offsets,
and that hardcoding things in general is undesirable, compared to a more 
solid solution.
I had a look at an approach for removing hardcoded offsets completely. 
It looks something like this:

http://cr.openjdk.java.net/~eosterlund/8241825/webrev.00..not_01/

The change is a bit involved and complicated though. Because of that, I 
would prefer to push it as a follow-up RFE
focused on removing hardcoded offsets, if you are okay with that. This 
change is already quite tricky, and I think
these things ought to be separate.

The fix involves splitting allocation and code generation of the 
interpreter (due to dependencies with verification code in i2i adapters
generated during method loading, requiring the interpreter to be 
allocated early), computing offsets of jlr.Reference early during
resolution of well known classes (as is done for java_lang_String and 
java_lang_Class), because it needs offsets to remove the
discovered and referent fields from oop maps before its subclasses are 
initialized, and moving initialization (and generation)
of the interpreter until after offsets have been computed (no longer 
having the dependency to method loading and linking).
This way, all offsets have always been computed (or loaded by CDS) 
before they are needed, without any need for hardcoding.

What do you think?

Thanks,
/Erik

On 2020-04-24 16:45, Frederic Parain wrote:
>
>> On Apr 24, 2020, at 10:36, coleen.phillimore at oracle.com wrote:
>>
>>
>>
>> On 4/24/20 10:16 AM, Erik Österlund wrote:
>>> Hi Frederic,
>>>
>>> Thanks for reviewing this.
>>>
>>> On 2020-04-24 15:28, Frederic Parain wrote:
>>>> Hi Erik,
>>>>
>>>> Why did you removed the code handling classes with hard coded layouts?
>>>> This code was there to provide the freedom to play with layout algorithms
>>>> without having to deal with these special cases. Without this code, any
>>>> modification is now constrained by these special classes.
>>> Good question. Since I started using the klass gap for fields, even when compressed oops is off, dealing with the
>>> hardcoded offsets and layouts really became quite awkward, to the extent that I started wondering why do we even have
>>> special layouts for these things anyway. I don't like special things. Treating it like any other object seemed far
>>> easier, than matching the special special hardcoded layouts with the hardcoded offsets.
>>>
>>> I wouldn't say that the whole layout algorithm is constrained by the fact that there are hardcoded offsets.
>>> I would say that it's just a matter of whoever changes the layout algorithms such that the hardcoded offsets
>>> are no longer the same, is just gonna have to update the offset to whatever the correct offsets are with the
>>> new algorithm. And if you look at the classes where we actually do hardcode the offsets, they are rather vanilla
>>> with relatively few ways you could change the layout other than changing it around for the sake of it. Especially
>>> primitive box objects. My fantasy is quite limited how we expect a new layout algorithm is gonna do anything else
>>> there really.
>>>
>>> Philosophically, I really think it is up to the person hardcoding an offset to know what the offset is going to
>>> be, ratherthan having completely different special (and worse) layouts produced for certain objects, just to
>>> satisfy the expectations of the hardcoded offsets.Especially if the result is pretty bad memory waste in for
>>> example Integer box objects, that can be quite plentyful in an application.
>> We used to have a lot more special hardcoded layouts but most have been removed. The reason these few classes are hardcoded is because the interpreter generates code using these offsets before JavaClasses::compute_offsets is run.  We tried to get rid of all of them and maybe there's a way to indirectly refer to them in the generated code so we can do so in the future.
> The proposed changes make the whole system brittle. Restoring the special handling of these classes during
> layout computation or making the generated code able to deal with dynamically computed offsets would be a
> more robust solution.
>
>
>>> The code to figure out the right offsets if a very small amount of code, compared to the rather large amount of
>>> code to create special layouts and syncing the special hardcoded layout code with the hardcoded offsets. That
>>> is why my patch has a net negative amount of code.
>>>
>>> Philosophically I also don't like having "special" layouts because we don't trust our ability to hardcode some
>>> offset to the right value. Almost all "special" things are bad and create unnecessary complications, and hence
>>> should have very good motivation for being allowed to be special. In this case, I really could not see any reason
>>> why these layouts would be special, unless it was somehow super hard to figure out what the hardcoded offsets would be.
>>> But for the classes we currently need hardcoded offsets, the calculations are trivial. So I don't think there is
>>> enough motivation to have special (bad) layouts for these objects for that reason. It only makes things harder than
>>> it needs to be, and makes the layout worse.
>>>
>>>> And I’d prefer to keep the dispatch in FieldLayoutBuilder::build_layout()
>>>> because we have more cases to handle there for the Valhalla project (the
>>>> layout algorithm for inline types is different than the one in
>>>> compute_regular_layout()).
>>> Okay. There was no need for the dispatching any more since there was only the normal layout and nothing
>>> else. But I can put that code back into the compute_regular_layout() function if that makes it easier for you
>>> in Valhalla, and have FieldLayoutBuilder::build_layout() just call that function. Will update in the next webrev.
> Thank you,
>
> Fred
>
>>>
>>>> Regards,
>>>>
>>>> Fred
>>>>
>>>>
>>>>> On Apr 24, 2020, at 04:17, Erik Österlund <erik.osterlund at oracle.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Today, the use of compressed class pointers assumes the use of compressed oops.
>>>>> This patch loosens up the relationship between compressed oops and compressed
>>>>> class pointers, so that compressed class pointers can be used without compressed
>>>>> oops. This benefits for example ZGC that wants compressed class pointers, but
>>>>> not compressed oops, effectively giving ZGC 4 bytes smaller objects.
>>>>>
>>>>> Much of the complexity of the change is that r12 used to be used by compressed
>>>>> class pointers as some kind of semi-fast temp register that one would restore
>>>>> to the compressed oops heap base (or zero). That made it effectively a slightly
>>>>> optimized spilling mechanism used to find a temp register. I replaced that
>>>>> mechanism with a plain old normal temp register that you pass in as a parameter.
>>>>> The bad news is that I had to find available temp registers in a lot of places.
>>>>> The good news is that almost always were there temp registers available for free,
>>>>> and hence the new temp register is even faster than the old optimized spilling
>>>>> mechanism. Because we almost never need any spilling at all in the contexts where
>>>>> this is used.
>>>>>
>>>>> Since I want the 4 new bytes to actually make objects smaller, I poked the new
>>>>> layout code a bit so that the klass gap is made available for fields. That used
>>>>> to be made available only with compressed oops enabled, due to restrictions in
>>>>> the layout code. Our new layout code does not have such restrictions, and so
>>>>> I will make the 4 bytes available for fields when the new layout code is used
>>>>> and compressed class pointers are used.
>>>>>
>>>>> Now I'm only fixing this for HotSpot x86_64. Ideally the use of compressed oops
>>>>> and compressed class pointers should not be entangled in other platforms and
>>>>> Graal. But that is beyond the scope of this change, and I will let them behave
>>>>> the way that they used to, to be potentially fixed later.
>>>>>
>>>>> Bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8241825
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~eosterlund/8241825/webrev.00/
>>>>>
>>>>> Testing:
>>>>> hs-tier1-7
>>>>>
>>>>> Thanks,
>>>>> /Erik