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

[Vladimir Kozlov]

You need closed SA changes.

Style:

Move fields to the beginning of ImmutableOopMapBuilder and classes.
Add comments describing each field in ALL new classes. Add comments to fields in old class. It will help next persone 
who will look on oop maps later.

Add ResourceMark into ImmutableOopMapSet::build_from() to free memory allocated in ImmutableOopMapBuilder().
Why you need ImmutableOopMapBuilder to be friend of class ImmutableOopMap?

I think a simple loop in ImmutableOopMapBuilder::verify() would be faster than calling memcmp.

Field _end is not used.

ImmutableOopMapBuilder() calls reset() and next called heap_size() calls reset() again. May be move reset() to the end 
of heap_size() so that you don't need to call it in fill().

Thanks,
Vladimir

On 4/28/15 3:03 AM, 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
>