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