RFR (M): 8034842: Parallelize the Free CSet phase in G1

Derek White derek.white at oracle.com
Tue May 10 23:00:26 UTC 2016


Hi Thomas,

This is an initial pass of mostly minor comments. I'm still getting a 
handle on the bigger picture.

----------------------------------
src/share/vm/gc/g1/heapRegionSet.hpp
  - Copyrights.

----------------------------------
src/share/vm/gc/g1/g1CollectedHeap.cpp:

Lines 4733, 4735:
- Add comment about being array?

Line 4744:
- Is "_pre_used" the same as "bytes (or words?) successfully evacuated"?

Line 4752:
  - Consider making do_serial_work directly update G1GCPhaseTimes, like 
do_parallel_work() does. Then cleanup up 
G1FreeCollectionSetTask::work(). Otherwise fix up spacing in 
G1FreeCollectionSetTask::work()

Line 4755:
  _g1h is already a field, we don't need a local.

Line 4760:
  - age_bound not used?

Line 4775:
  - blank line.

Line 4781:
  - Assertion is partially redundant with lines 4771 & 4778.

Line 4827:
  _g1h is already a field, we don't need a local.

Line 4937:
  G1CollectedHeap::heap() is already a field.

Line 4947:
  _g1h is already a field, we don't need a local.

----------------------------------
Other files looked good.

  - Derek

On 5/10/16 11:00 AM, Thomas Schatzl wrote:
> Hi again,
>
> On Tue, 2016-05-03 at 12:39 +0200, Thomas Schatzl wrote:
>> Hi all,
>>
>>    due to recent pushes the change did not apply cleanly any more.
>>
>> http://cr.openjdk.java.net/~tschatzl/8034842/webrev.0_to_1/ (diff)
>> http://cr.openjdk.java.net/~tschatzl/8034842/webrev.1/ (full)
>>
>> contain changesets that do.
>    another set of updates after recent changes:
>
> http://cr.openjdk.java.net/~tschatzl/8034842/webrev.2/ (full)
>
> Thanks,
>    Thomas
>




More information about the hotspot-gc-dev mailing list