RFR (M): 8064458 OopMap class could be more compact

[Bertrand Delsart]

On 29/04/2015 16:53, Rickard Bäckman wrote:
> On 04/28, Bertrand Delsart wrote:
>> Hi,
>>
>> First, thanks for the change. The additional benefit is that an
>> ImmutableOopMapSet no longer contains any absolute references.
>>
>> A few comments.
>>
>> The ImmutableOopMapSet.java seems to be missing in the webrev.
>
> Added.

Thanks.

I suppose that ImmutableOopMap.java and ImmutableOopMapSet.java are 
renamings of the old non Immutable code. Did you use 'hg move' ? This 
may help get cleaner history and webrevs, showing what you modified in 
these files.

Note also the missing copyright header in ImmutableOopMapPair.java

Regards,

Bertrand.

>
>>
>> There also seem to be a few issues with ImmutableOopMap::print_on
>> and ImmutableOopMapSet::print_on:
>>
>> - I did not spot the closing "}" corresponding to
>>    "ImmutableOopMap{"
>
> Fixed.
>
>>
>> - the "map != last" part does not look complete. I assume that you
>> are trying to dump only once the OopMap when it is shared by
>> successive pcs but you are then missing a "last = map;" line
>> somewhere in the if statement.
>
> Thanks, missed that. Fixed.
>
>>
>> - as a minor point, I'd rather print "offs:" or "pc offsets:"
>> instead of "pcs:" because OopMapSet use pc offsets, not absolute
>> pcs. [ Your CR might also be the right time to replace "pc" by
>> "pc_offset" for a few field or variable names since this is a bit
>> confusing. ]
>
> Changed to "pc offsets:". Renamed some of the new fields as well. Didn't
> want to mess with the old ones.
>
> Updated webrev: http://cr.openjdk.java.net/~rbackman/8064458.2/
>
>
> Thanks
> /R
>
>>
>> Regards,
>>
>> Bertrand.
>>
>> On 28/04/2015 12:03, Rickard Bäckman wrote:
>>> Hi all,
>>>
>>> can I please have reviews for this change:
>>>
>>> RFR: http://cr.openjdk.java.net/~rbackman/8064458.1/
>>> RFE: http://bugs.openjdk.java.net/browse/JDK-8064458
>>>
>>> While looking at OopMaps a while ago I noticed that there were a couple
>>> of different fields that were unused after the OopMaps were finalised.
>>>
>>> I took some time to investigate and rearrange the OopMaps. Since I
>>> didn't want to change how the OopMaps are built I introduced new data
>>> structures. ImmutableOopMapSet and ImmutableOopMap. The original OopMap
>>> structures are used to build up the OopMaps and when finalised they are
>>> copied into the Immutable variants.
>>>
>>> The ImmutableOopMapSet contains a few fields [size, count] and then a
>>> list of [pc, offset]. The offset points to the offset after the list
>>> where the ImmutableOopMap is placed. By moving pc out from OopMap to be
>>> part of the list we can now have multiple pcs with identical OopMaps
>>> point to the same data.
>>>
>>> We only keep 1 empty OopMap, and the other compaction that is done in
>>> this change is to check if the OopMap is identical to the previous one
>>> and then reuse that one. So no complete uniqueness check.
>>>
>>> I ran a couple of small benchmarks and printed the size of the old
>>> OopMaps vs the new. The new layout uses about 20 - 25% of the space on
>>> the benchmarks I've run.
>>>
>>> Tested by running through JPRT, running BigApps and NSK.quick.testlist
>>>
>>> Thanks
>>> /R
>>>
>>
>>
>> --
>> Bertrand Delsart,                     Grenoble Engineering Center
>> Oracle,         180 av. de l'Europe,          ZIRST de Montbonnot
>> 38330 Montbonnot Saint Martin,                             FRANCE
>> bertrand.delsart at oracle.com             Phone : +33 4 76 18 81 23
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> NOTICE: This email message is for the sole use of the intended
>> recipient(s) and may contain confidential and privileged
>> information. Any unauthorized review, use, disclosure or
>> distribution is prohibited. If you are not the intended recipient,
>> please contact the sender by reply email and destroy all copies of
>> the original message.
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /R
>


-- 
Bertrand Delsart,                     Grenoble Engineering Center
Oracle,         180 av. de l'Europe,          ZIRST de Montbonnot
38330 Montbonnot Saint Martin,                             FRANCE
bertrand.delsart at oracle.com             Phone : +33 4 76 18 81 23

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE: This email message is for the sole use of the intended
recipient(s) and may contain confidential and privileged
information. Any unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient,
please contact the sender by reply email and destroy all copies of
the original message.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~