RFR (M): 8067336: Allow that PLAB allocations at the end of regions are flexible

Thomas Schatzl thomas.schatzl at oracle.com
Wed Aug 19 21:18:29 UTC 2015

Hi Tom,

On Tue, 2015-08-18 at 11:03 -0400, Tom Benson wrote:
> Hi Thomas,
> It looks like the actual plab allocation changes are missing from 
> g1Allocator.cpp - only an include was added...   ?
> Aside from that, just a couple of minor comments.  I think the comment 
> at line 193-194 in g1AllocRegion.hpp should say "...maximum word size of 
> the allocation in desired_word_size" rather than "... in word_size".
> In g1AllocRegion.inline.hpp, attempt_allocation and 
> attempt_allocation_locked both have an error path that includes:
> trace("alloc failed", *actual_word_size);
> In the old version, the value handed to trace was the requested size, so 
> here it should probably be min_word_size, since *actual_word_size is 
> likely to be uninitialized.

  thanks for the review, even if it was for an unintentionally broken
change. I seem to have removed the actual change, the use of the new
methods in G1PLABAllocator::allocate_direct_or_new_plab(), while merging
these series of PLAB changes and the evac failure changes.

I also fixed the G1AllocRegion::trace() method, adding the new sizes
(desired/actual size) as needed.

Then I decided to move making G1OffsetTableContigSpace::_top volatile
(this is a _potential_ pre-existing bug - I am sure there is some weird
compiler that notices that it could hoist this access out of the loop in
G1OffsetTableContigSpace::par_allocate_impl()) into this CR. I am sure
you would have noticed anyway.

I think I also covered the other problems you mentioned. Thanks again.

http://cr.openjdk.java.net/~tschatzl/8067336/webrev.1 (full)
http://cr.openjdk.java.net/~tschatzl/8067336/webrev.0_to_1 (diff)
jprt, vm.gc testlist, perf testing benchmarks, locally checked
allocation tracing (enabled by define only)


More information about the hotspot-gc-dev mailing list