RFR: Eliminating string dedup dependency on the second bitmap

[Roman Kennke]

Am 27.10.2017 um 19:15 schrieb Zhengyu Gu:
>
>>
>>     typeArrayOop obj() {
>> - return _obj;
>> + return typeArrayOop(oopDesc::bs()->read_barrier(_obj));
>>     }
>>       typeArrayOop* obj_addr() {
>>       return &_obj;
>>     }
>>       void set_obj(typeArrayOop obj) {
>> - _obj = obj;
>> + _obj = typeArrayOop(oopDesc::bs()->write_barrier(obj));
>>     }
>>
>>
>> You're not reading/writing anything *to* the object. Therefore it 
>> looks wrong to do those barriers there.
>>
>> +class ShenandoahEvacuateUpdateRefsClosure: the sounds like it may 
>> clash with something that already exists. Can you check that? Romn
>>
>
> String Dedup table holds byte array oops, somewhat likes roots, but 
> not be scanned during marking, their reachability depend on the 
> strings referring them.
>
> When a new byte array oop is added to the table, you have to make sure 
> it is to-space oop, otherwise, it can be trashed.
>
> I agree, that we may not need read barrier, since we applied 
> keep_alive barrier if string is dedup.
>
> I will test without read barrier, but I do think write barrier is 
> necessary here.
>
It is possible that I get it wrong. But let me outline what I think:
The StringDedupTable is a sort of GC root. In other words, it is a 
pointers to heap oops from outside the heap. As such, it should be 
treated and work like all other GC roots. Which does not involved 
write-barriers on setting the pointer.

It is possible and perfectly legal to write a from-space ref to a GC 
root reference. What happens in this case is that later update-roots 
pass after evacuation updates them to point to to-space. (that would be 
either before marking or before the separate update_refs phase.) Until 
that happens, all *access* to those oops, e.g. actual writes to the char 
arrays in this case, need to go through a barrier.

Whenever you insert a new barrier that doesn't involve any actual 
directly subsequent heap access, it's a strong indication that you 
should stop, step back, and check if it's already covered by some other 
mechanism. It's almost always wrong to do a barrier after which there is 
no actual access.

Are the StringDedupTable roots covered by ShenandoahRootProcessor? Is it 
covered by the update-roots passes before marking or before update refs?

Are you getting any actual failures if you leave out those barriers?

Roman