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

Aleksey Shipilev aleksey.shipilev at oracle.com
Wed Oct 1 10:00:34 UTC 2014


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

-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.
> 




More information about the hotspot-dev mailing list