RFR (S): 8016556: G1: Use ArrayAllocator for BitMaps

[Thomas Schatzl]

[added the CR number in the subject line]

Hi,

On Thu, 2013-06-13 at 16:53 +0200, Bengt Rutisson wrote:
> Hi everyone,
> 
> Could I have a couple of review for this small change?
> http://cr.openjdk.java.net/~brutisso/8016556/webrev.00/
> 

Looks good.

A few minor comments which are free to ignore:

- the code in the ArrayAllocator constructor/destructor is now more than
a line. Some additional spacing between them may help readability.

> One complication with the fix is that the BitMap data structures get 
> copied around quite a bit. The copies are shallow copies, so we don't 
> risk re-doing the allocation. But since I am now embedding an 
> ArrayAllocator in the BitMap structure the destructor for copies of the 
> ArrayAllocator gets called every now and then. The BitMap explicitly 
> frees up the allocated memory when it thinks it is necessary. So, rather 
> than trying to refactor the code to avoid copying I made it optional to 
> free the allocated memory in the ArrayAllocator desctructor.

- instead of the "free_in_destructor" name I would prefer something like
"owner"; but this is really just my personal opinion.

> I do think it would be good to review the BitMap code. It seems a bit 
> fishy that we pass around shallow copies. But I think my current change 
> keeps the same behavior as before.

I agree about both things :)

Thanks,
Thomas