RFR (S): 8189355: Cleanup of BarrierSet barrier functions

Per Liden per.liden at oracle.com
Wed Oct 18 18:57:38 UTC 2017


Looks good. Just one minor nit.

share/gc/shared/cardTableModRefBS.hpp:

  182  protected:
  183   CardTableModRefBS(MemRegion whole_heap, const 
BarrierSet::FakeRtti& fake_rtti);
  184   ~CardTableModRefBS();
  185
  186   // Record a reference update. Note that these versions are precise!
  187   // The scanning code has to handle the fact that the write 
barrier may be
  188   // either precise or imprecise. We make non-virtual inline 
variants of
  189   // these functions here for performance.
  190
  191   void write_ref_field_work(oop obj, size_t offset, oop newVal);
  192   virtual void write_ref_field_work(void* field, oop newVal, bool 
release);
  193
  194  protected:
  195   void write_region_work(MemRegion mr) {
  196     dirty_MemRegion(mr);
  197   }
  198
  199  protected:
  200   void write_ref_array_work(MemRegion mr) {
  201     dirty_MemRegion(mr);
  202   }

Looks like we now have three protected declarations in a row, so the 
last two could go. I don't need to see a new webrev for that.

cheers,
Per

On 2017-10-18 09:27, Erik Österlund wrote:
> Hi Aleksey,
> 
> New full webrev:
> http://cr.openjdk.java.net/~eosterlund/8189355/webrev.01/
> 
> New incremental webrev:
> http://cr.openjdk.java.net/~eosterlund/8189355/webrev.00_01/
> 
> 
> On 2017-10-17 21:37, Aleksey Shipilev wrote:
>> Hi Erik,
>>
>> On 10/17/2017 12:27 PM, Erik Österlund wrote:
>>> Webrev:
>>> http://cr.openjdk.java.net/~eosterlund/8189355/webrev.00/
>> Good patch, I scratched my head about the meaning about many of those 
>> methods when building the most
>> trivial BS for Epsilon:
>>
>> http://hg.openjdk.java.net/jdk10/sandbox/hotspot/file/2e4b568252ea/src/share/vm/gc/epsilon/epsilonBarrierSet.hpp 
>>
> 
> :)
> 
>> Which also raises one observations:
>>
>>   *) gen_write_ref_array_pre_barrier in stubGenerator_arm.cpp now has 
>> unconditional call to
>> BarrierSet::static_write_ref_array_pre, right? Epsilon returns "false" 
>> for
>> has_write_ref_pre_barrier. Is this something to be addressed by GC 
>> interface later?
> 
> Yes this will indeed be addressed later on and will be solved in a much 
> better way (if I say so myself). But since it was mentioned by both you 
> and Kim in this round, I have a new version of the patch that fixes that 
> so that the pre barrier runtime calls for arraycopy stubs are only 
> generated if G1 was selected.
> 
>> Otherwise a very welcome cleanup!
> 
> Thank you for the review.
> 
> /Erik
> 
>> Thanks,
>> -Aleksey
>>
>>
> 



More information about the hotspot-gc-dev mailing list