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

[Erik Österlund]

Hi Per,

Thanks for the review.

/Erik

On 2017-10-18 20:57, Per Liden wrote:
> 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
>>>
>>>
>>