RFR: 8141501: Problems with BitMap buffer management

[Kim Barrett]

> On Apr 28, 2016, at 9:23 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> Hi all,
> 
> I decided to restructure the allocation code a bit to get better separation between the bitmap allocation logic and the actual bitmap memory allocators.
> 
> http://cr.openjdk.java.net/~stefank/8141501/webrev.02.delta
> http://cr.openjdk.java.net/~stefank/8141501/webrev.02

Generally looks good.

Just a few mostly minor comments or suggestions.  The only really
important one is about ~G1RegionToSpaceMapper.

------------------------------------------------------------------------------ 
src/share/vm/c1/c1_GraphBuilder.cpp
 355 void BlockListBuilder::mark_loops() {
 356   ResourceMark rm;
 357 
 358   _active.initialize(BlockBegin::number_of_blocks());
 359   _visited.initialize(BlockBegin::number_of_blocks());

Consider (possibly debug-only) resizing _active and _visited to zero
at the end of the function, to avoid leaving dangling pointers when
the ResourceMark is exited.

------------------------------------------------------------------------------
src/share/vm/c1/c1_Instruction.hpp
1721   void set_live_in       (ResourceBitMap map)    { _live_in = map;        }
1722   void set_live_out      (ResourceBitMap map)    { _live_out = map;       }
1723   void set_live_gen      (ResourceBitMap map)    { _live_gen = map;       }
1724   void set_live_kill     (ResourceBitMap map)    { _live_kill = map;      }
1725   void set_fpu_register_usage(ResourceBitMap map) { _fpu_register_usage = map; }

By-value parameters seem like unnecessary (shallow) copies.  Could
these be passed by const-ref?

------------------------------------------------------------------------------ 
src/share/vm/c1/c1_LinearScan.cpp
 718     block->set_live_in  (ResourceBitMap(live_size)); block->live_in().clear();
 719     block->set_live_out (ResourceBitMap(live_size)); block->live_out().clear();

Consider instead

  block->live_in().reinitialize(live_size);
  block->live_out().reinitialize(live_size);

------------------------------------------------------------------------------ 
src/share/vm/gc/g1/g1CardLiveData.cpp
 400   G1ClearCardLiveDataTask(BitMapView bitmap, size_t num_tasks) :

Pass bitmap by const-ref?

------------------------------------------------------------------------------ 
src/share/vm/gc/g1/g1RegionToSpaceMapper.hpp
[Removed]
  65   virtual ~G1RegionToSpaceMapper() {
  66     _commit_map.resize(0, /* in_resource_area */ false);
  67   }

This changes this base class's destructor from public virtual to
public non-virtual, introducing risk of unintended slicing.

------------------------------------------------------------------------------ 
src/share/vm/utilities/bitMap.hpp
 182   idx_t size_in_bytes() const { return size_in_words() * BytesPerWord; }

Perhaps instead use

  return calc_size_in_bytes(size());

------------------------------------------------------------------------------ 
src/share/vm/utilities/bitMap.hpp
 163   // Protected constructor.

Mention destructor too?

------------------------------------------------------------------------------