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.
CR:
https://bugs.openjdk.java.net/browse/JDK-8067336
Webrev:
http://cr.openjdk.java.net/~tschatzl/8067336/webrev.1 (full)
http://cr.openjdk.java.net/~tschatzl/8067336/webrev.0_to_1 (diff)
Testing:
jprt, vm.gc testlist, perf testing benchmarks, locally checked
allocation tracing (enabled by define only)
Thanks,
Thomas
More information about the hotspot-gc-dev
mailing list