RFR (XS) 8059474: Clean up vm/utilities/Bitmap type uses

[Mikael Gerdin]

Aleksey,

On Wednesday 01 October 2014 14.00.34 Aleksey Shipilev wrote:
> Hi Mikael,
> 
> Thanks for a review. Truly, I missed that spot, done here:
>  http://cr.openjdk.java.net/~shade/8059474/webrev.01/
> 
> Testing: JPRT, Nashorn/Octane with Linux x86_64/fastdebug,
> -XX:+ExecuteInternalVMTests
> 
> If you are happy with this change, please sponsor:
>  http://cr.openjdk.java.net/~shade/8059474/8059474.changeset

Looks good. I'll sponsor it.

/Mikael

> 
> -Aleksey.
> 
> On 10/01/2014 11:36 AM, Mikael Gerdin wrote:
> > Hi Aleksey,
> > 
> > On Wednesday 01 October 2014 11.23.59 Aleksey Shipilev wrote:
> >> Thanks for review, Coleen!
> >> 
> >> Any GC guys around? Please take a look.
> > 
> > In
> > 
> >  338 bool BitMap::set_union_with_result(BitMap other) {
> >  339   assert(size() == other.size(), "must have same size");
> >  340   bool changed = false;
> >  341   bm_word_t* dest_map = map();
> >  342   bm_word_t* other_map = other.map();
> >  343   idx_t size = size_in_words();
> >  344   for (idx_t index = 0; index < size; index++) {
> >  345     idx_t temp = map(index) | other_map[index];
> >  346     changed = changed || (temp != map(index));
> >  347     dest_map[index] = temp;
> >  348   }
> >  349   return changed;
> >  350 }
> > 
> > You changed one call to map() to use the local variable dest_map, can you
> > make it consistently use the locals instead of calling map(index)?
> > 
> > The rest of the change looks good, thanks for cleaning this up a bit.
> > 
> > There are some tests in TestBitMap_test(), you can run them with
> > -XX:+ExecuteInternalVMTests on a fastdebug VM.
> > 
> > /Mikael
> > 
> >> -Aleksey.
> >> 
> >> On 10/01/2014 02:10 AM, Coleen Phillimore wrote:
> >>> Aleksey,
> >>> 
> >>> This looks good to me.  GC code uses BitMap a lot and JPRT tests the
> >>> different collectors so I think this is fine.  Someone from the GC team
> >>> should look at this also.
> >>> 
> >>> Thanks!
> >>> Coleen
> >>> 
> >>> On 9/30/14, 11:03 AM, Aleksey Shipilev wrote:
> >>>> Hi,
> >>>> 
> >>>> Not sure which group this belongs to, using the generic hotspot-dev at .
> >>>> 
> >>>> vm/utilities/BitMap inconsistencies bugged me for quite some time: the
> >>>> mention of naked uintptr_t instead of properly typedef-ed bm_word_t
> >>>> alias; casting AllBits/NoBits constants of (luckily) the same type;
> >>>> other things like a benign inconsistency in init_pop_count_table
> >>>> new/free, etc.
> >>>> 
> >>>> Here is a cleanup, please review:
> >>>>    http://cr.openjdk.java.net/~shade/8059474/webrev.00/
> >>>>    https://bugs.openjdk.java.net/browse/JDK-8059474
> >>>> 
> >>>> Testing:
> >>>>    JPRT, Nashorn/Octane on Linux/x86_64/fastdebug.
> >>>> 
> >>>> Thanks,
> >>>> -Aleksey.