CRR (M): 7157073: G1: type change size_t -> uint for region counts / indexes
igor.veresov at oracle.com
Tue Mar 27 14:38:53 PDT 2012
Looks good to me, except for one pedantic bit:
671 memset(_in_cset_fast_test_base, false, 672 (size_t) (_in_cset_fast_test_length * sizeof(bool))); 673 }
In case sizeof(bool) > 1, may be it should be: (size_t)_in_cset_fast_test_length * sizeof(bool) ?
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
> The main body of the changes is here:
> 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).
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev