CRR (M): 7157073: G1: type change size_t -> uint for region counts / indexes

[Tony Printezis]

PS Peter Kessler suggested the use of specific types for region counts 
and indexes, like:

typedef uint region_count_t;
typedef uint region_index_t;

to make a little more explicit what variables / fields represent. I 
think it's a good idea. If we decide to go with this change, let me know 
if this is something I should also add.

Tony

On 04/02/2012 09:27 AM, Tony Printezis wrote:
> Mikael and Igor,
>
> Members of the GC group objected to the use of size_t for counts and 
> indexes. Personally, I don't mind it given that it's less error-prone 
> (IMHO) and I think a bit more consistent (less casting during heap 
> size -> index calculations). But, I'm OK with the change too (if we 
> have lots of counts / indexes in some data structures, they will be a 
> little more cache friendly).
>
> I'm happy if folks "fight it out" and tell me what to do - I'm OK 
> either way.
>
> Tony
>
> On 03/30/2012 06:41 PM, Mikael Vidstedt wrote:
>>
>> I'm also interested in the background to the change. For structures 
>> that are very common in memory it makes sense to streamline the size 
>> of the fields and structs. Is that the case here or is something else 
>> driving the change?
>>
>> Thanks,
>> Mikael
>>
>> On 2012-03-30 15:06, Igor Veresov wrote:
>>> On a second thought, what is the rationale for doing this? This 
>>> seems to be quite error prone I have to say. In case where you have 
>>> to compute a size of something that is in units that are less than a 
>>> region you have to take special care and do the cast. Like if you 
>>> say: size_t region_offset_bytes = region_index << 
>>> HeapRegion::LogOfHRGrainBytes; that would already be an error that 
>>> is not easy to find - you'll just get a bogus answer and the 
>>> compiler will be fine with it.
>>>
>>> Just my $0.02..
>>>
>>> igor
>>>
>>> On Tuesday, March 27, 2012 at 9:34 AM, Tony Printezis wrote:
>>>
>>>> Hi all,
>>>>
>>>> I'd like a couple of reviews for this cleanup. We have been using
>>>> size_t's to represent region counts / indexes and I have been 
>>>> encouraged
>>>> to change those to uint's. This unfortunately turned out to be 
>>>> quite an
>>>> extensive change that was hard to localize. I've split the change into
>>>> two webrevs.
>>>>
>>>> First, this breaks down a long line in g1CollectedHeap.cpp. I I 
>>>> have it
>>>> on a separate webrev as it messes up the webrev pages for the rest of
>>>> the changes (i.e., the rest of the changes are easier to read with 
>>>> this
>>>> excluded):
>>>>
>>>> http://cr.openjdk.java.net/~tonyp/7157073/webrev.0/webrev.0.G1LongLineFix/ 
>>>> <http://cr.openjdk.java.net/%7Etonyp/7157073/webrev.0/webrev.0.G1LongLineFix/> 
>>>>
>>>>
>>>> The main body of the changes is here:
>>>>
>>>> http://cr.openjdk.java.net/~tonyp/7157073/webrev.0/webrev.1.G1Uints/ <http://cr.openjdk.java.net/%7Etonyp/7157073/webrev.0/webrev.1.G1Uints/> 
>>>>
>>>>
>>>> Some important comments:
>>>>
>>>> * I tried to change as many of the size_t occurrences as possible 
>>>> but I
>>>> didn't touch the following:
>>>>
>>>> - HeapRegionRemSet - we're in the process of reworking this code so I
>>>> decided to leave it mostly as is (I only added a few necessary casts)
>>>>
>>>> - CollectionSetChooser / HeapRegion::sort_index() /
>>>> HeapRegion::_sort_index - I have a CR open for a CSetChooser 
>>>> cleanup and
>>>> I'll do the conversion as part of that cleanup (I did change a few 
>>>> types
>>>> on the CSetChooser external API to avoid casts from methods that 
>>>> access it).
>>>>
>>>> - HeapRegion::_young_index_in_cset / 
>>>> HeapRegion::young_index_in_cset() -
>>>> The code looks as if it relies on the fact that this is an int. It 
>>>> seems
>>>> well localized so I left it as is and we can look at it on a 
>>>> separate CR.
>>>>
>>>> * I had to make some changes to the vmStructs_g1.hpp and I need to
>>>> ensure that the SA code works with those changes. Unfortunately, I'm
>>>> going on vacation for a few days tonight and I haven't had a chance to
>>>> do that yet. I'll look at the SA more closely when I get back and, 
>>>> if I
>>>> need to make any changes to the SA, I'll publish those separately 
>>>> (they
>>>> will be self-contained anyway). I just wanted to get the main body of
>>>> the changes out before I leave so any potential reviewers can get
>>>> started on it.
>>>>
>>>> * Testing: the source builds on all platforms and passes JPRT. And 
>>>> I did
>>>> a lot of extra 64-bit testing (as any issues should mainly show up in
>>>> 64-bit builds).
>>>>
>>>> Tony
>>>
>>