RFR (M) 8135085: Change Method::_intrinsic_id from u1 to u2

[Ioi Lam]

On 9/9/15 11:36 AM, Aleksey Shipilev wrote:
> Hi Coleen,
>
> On 09/09/2015 08:37 PM, Coleen Phillimore wrote:
>> Increasing the size of this field unfortunately gives us a pretty large
>> alignment gap in Method* which will affect footprint.
> Where does this come from? As I mentioned in the original note, on my
> Linux x86_64/gcc, sizeof(Method) is 88 bytes for both baseline and
> patched version -- it would seem the alignment gaps are already present
> where we can fit the u1 -> u2 expansion. It only expands to 96 bytes
> when I add three additional u1 fields.
>
>
>> The good news is that the field _method_size is not actually needed.
>> In order to increase intrinsic_id, you should also remove this field at
>> the same time.   Then we'll only have a u1 alignment gap which we can
>> use for something in the future.
> Um. There is a Method::method_size() getter. All right, the only use of
> it is Method::size(). Method::size() is used at least in
> MetadataFactory::free_metadata()... I'm puzzled what you mean by
> "_method_size is not actually needed".
>
>
>> Also, because you changed the vmStructs, there's likely a Java
>> duplicated code change to make in the serviceability agent.   There
>> should be a basic test in the jtreg hotspot/test/serviceability/sa.
>> You should run more tests on a change like this.   I will send you
>> directions on how to run RBT internally.
> Of course, but can anyone from runtime team take over? This is a core
> runtime issue that affects everyone generally, and it is important for
> an ongoing JDK project. Please help?

I just checked, there's no serviceability agent code that uses 
intrinsic_id. So we're safe. No work to be done there :-)

./src/share/vm/runtime/vmStructs.cpp:  nonstatic_field(Method, 
_intrinsic_id, u1)

~/jdk/jdk9_rt/hotspot/agent$ find . -name \*.java | xargs grep 
intrinsic_id | wc
       0       0       0

Thanks
- Ioi


> Thanks,
> -Aleksey
>
>> thanks,
>> Coleen
>>
>> On 9/9/15 1:22 PM, Aleksey Shipilev wrote:
>>> Hi,
>>>
>>> In VarHandles development, we have reached the tipping point when
>>> Method::_intrinsic_id is not enough to be u1, as we have >255 intrinsics:
>>>     https://bugs.openjdk.java.net/browse/JDK-8135085
>>>
>>> This is a patch that changes u1 -> u2:
>>>     http://cr.openjdk.java.net/~shade/8135085/webrev.00/
>>>
>>> Quite a few places in VM assume the _instance_id size, I changed those
>>> as well, and also put more asserts where we assume the particular value
>>> width.
>>>
>>> Testing:
>>>     * JPRT on all platforms
>>>     * Method::Method instrumentation to figure out the number of Methods
>>> and their sizeof-s. There seems to be no regression in Method instance
>>> size at least on Linux x86_64, because the instance alignment seems to
>>> have enough gaps to fit the larger u2 field. (I played more, and it
>>> seems three more u1 fields can be fitted in).
>>>
>>> Please review! If there are any problems/concerns, could anyone from
>>> runtime team please take over this work? We will use this prototype
>>> version in VarHandles to unblock the development for the time being.
>>>
>>> Thanks,
>>> -Aleksey
>>>
>