[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-gc-dev
Subject:    Re: RFR (M): 8201492: Properly implement non-contiguous generations for reference discovery
From:       Kim Barrett <kim.barrett () oracle ! com>
Date:       2018-04-30 14:12:14
Message-ID: 024C3D6A-C0B3-4B01-862F-59D391380EAC () oracle ! com
[Download RAW message or body]

> On Apr 29, 2018, at 2:45 PM, Thomas Schatzl <thomas.schatzl@oracle.com> wrote:
> Webrevs
> http://cr.openjdk.java.net/~tschatzl/8201492/webrev.1 (full)
> http://cr.openjdk.java.net/~tschatzl/8201492/webrev.0_to_1 (diff)
> 
> I pushed the changes through hs-tier1-3 again.
> 
> Note that the diff webrev is not very helpful in this case.
> 
> Thanks,
>  Thomas

------------------------------------------------------------------------------
The member name _span_discoverer, used by several of the collectors,
might be confusing; it's not discovering spans.  I'm not sure what a
better name might be though.  _span_based_discoverer?

------------------------------------------------------------------------------ 
src/hotspot/share/gc/g1/g1FullCollector.hpp
  47 class G1FullGCSubjectToDiscoveryClosure: public BoolObjectClosure {
  48 public:
  49   bool do_object_b(oop p) {
  50     return (p != NULL);
  51   }
  52 };

I think no null check is needed here, and that p could instead be
asserted to be non-NULL, because Full-GC is discovery_is_atomic.  This
is similar to the young/mixed gc case
(G!STWSubjectToDiscoveryClosure), and different from the cm case.
That would better match the _always_subject_to_discovery name too.

------------------------------------------------------------------------------ 
src/hotspot/share/gc/shared/referenceProcessor.hpp
 489   ReferenceProcessorSpanMutator(ReferenceProcessor* rp,
 490                                 MemRegion span):
 491     _rp(rp), _discoverer(span) {
 492     _old_discoverer = rp->is_subject_to_discovery_closure();
 493     rp->set_is_subject_to_discovery_closure(&_discoverer);
 494   }

Why is _old_discoverer initialized in the body rather than the
initializer list.

Also, formatting makes it hard to spot where the initializer list ends
and the body begins.

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

[prev in list] [next in list] [prev in thread] [next in thread]