RFR: Uses keep_alive_barrier to keep deduped char array alive

[Roman Kennke]

Am 11.10.2017 um 21:01 schrieb Aleksey Shipilev:
> On 10/11/2017 08:58 PM, Zhengyu Gu wrote:
>> On 10/11/2017 02:53 PM, Aleksey Shipilev wrote:
>>> On 10/11/2017 08:41 PM, Zhengyu Gu wrote:
>>>> We need to use newly introduced keep_alive_barrier to ensure deduped string's char array alive, as
>>>> required by new concurrent partial GC.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~zgu/shenandoah/strdedup_keep_alive/webrev.00/e
>>> Shouldn't the read barrier be on some common path, and not here, at least not under UseShenandoahGC?
>>> Why do we need it here?
>> Because keep_alive_barrier can move the oop, while SATB barrier does not.
> Ah, damn, OK then. Good to go.
>
> Roman, this is something to fix generically for keep_alive_barrier, right? There are dangerous uses
> like that in e.g JNIHandles::resolve_jweak and friends.
OK, I need to check this. Might need to extend the keep_alive_barrier() 
interface to return an oop if it is a problem.

However, I don't think it is a problem here. The existing_value is only 
used as storeval later, which should do a RB/WB as needed. More 
generally, any access to an object needs to do the correct barrier, 
preceding keep_alive_barrier() or not.

Also, the whole block:

384 if (UseShenandoahGC) {
385 oopDesc::bs()->keep_alive_barrier(existing_value);
386 existing_value = 
(typeArrayOop)oopDesc::bs()->read_barrier(existing_value);
387 } else {
  388       // Enqueue the reference to make sure it is kept alive. Concurrent mark might
  389       // otherwise declare it dead if there are no other strong references to this object.
  390       G1SATBCardTableModRefBS::enqueue(existing_value);
391 } Should be just: oopDesc::bs()->keep_alive_barrier(existing_value);

For G1 and Shenandoah under conc-mark, this enqueues the object in SATB, 
for Shenandoah under conc-partial it does the WB. As I said above, I 
don't think the RB is needed. I would be very surprised.

Roman