RFR (S): 8073632: Make auxiliary data structures know their own translation factor

[Stefan Karlsson]

On 2015-04-22 11:02, Thomas Schatzl wrote:
> Hi Stefan,
>
> On Wed, 2015-04-22 at 10:21 +0200, Stefan Karlsson wrote:
>> Hi Thomas,
>>
>> On 2015-04-22 09:56, Thomas Schatzl wrote:
>>>>     can I have reviews for this change that cleans up recent changes to
>>>> memory management in G1. In particular, for every class that represents
>>>> an auxiliary data structure in G1 it adds (and then uses) a specific
>>>> function that returns the "heap mapping factor" (the amount of bytes a
>>>> particular byte of that data structure corresponds in the java heap).
>>>> This method is then used instead of gathering it from various places
>>>> when allocating that data structure.
>>>>
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8073632
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~tschatzl/8073632/webrev/
>> http://cr.openjdk.java.net/~tschatzl/8073632/webrev/src/share/vm/gc_implementation/g1/g1SATBCardTableModRefBS.hpp.frames.html
>>
>> Why is this implemented as a call into G1BlockOffsetSharedArray?
>>
>>    157   // Returns how many bytes of the heap a single byte of the Card
>> Table corresponds to.
>>    158   static size_t heap_map_factor() {
>>    159     return G1BlockOffsetSharedArray::heap_map_factor();
>>    160   }
> You are correct, that is unnecessary. It is better to use
> CardTableModRefBS::card_size.
>
> Fixed in
> http://cr.openjdk.java.net/~tschatzl/8073632/webrev.1
> Diff webrev:
> http://cr.openjdk.java.net/~tschatzl/8073632/webrev.0_to_1

That looks better.

It still seems weird that the card_count is set up using 
G1BlockOffsetSharedArray:

    G1RegionToSpaceMapper* card_counts_storage =
      create_aux_memory_mapper("Card counts table",
G1BlockOffsetSharedArray::compute_size(g1_rs.size() / HeapWordSize),
-                             G1BlockOffsetSharedArray::N_bytes);
+ G1BlockOffsetSharedArray::heap_map_factor());

I think the "Card counts table" needs its own functions as well.

Thanks,
StefanK

>
> Thanks,
>    Thomas
>
>
>