RFR(M): 8190430 [MVT] Fixing GC support for Thread-local value buffers

[Frederic Parain]

Bertrand,

We’re glad to see you back!

The simple ‘OR’ (‘|’) operator is sufficient in this case because it is always applied
on a oop that has been sanitized for the GC (lines 566-568).

But I’ve followed your recommendation and added an assert on the mark bits
at the end of the oops_do() method.

Thank you,

Fred

> On Nov 17, 2017, at 11:43, bdelsart work <bdelsart.work at gmail.com> wrote:
> 
> Hi Fred,
> 
> I started to look at the webrev and will try to investigate more deeply but I did notice something strange in dealiser marking. Here is a quick feedback in case this is currently leading to issues in more complex tests.
> 
> It looks like you set set the new mark with just an '|' on the markbits in BufferedValuesDealiaser::oops_do. If I understand correctly, the expected marks are switched regularly and hence the initial state of the VTBuffer mask part may not always be 0. If this is the case, the proposed code may not always set the mark to what you expected.
> 
> You should probably clear the VTBuffer::mark_mask part from value->mark() before the | in
> +  intptr_t new_mark_word = ((intptr_t) (oopDesc*)(value->mark()))
> +              | (intptr_t)_current_mark;
> Alternatively, the marks could be chosen so that a simple XOR works (for instance by using only one bit for this marking)
> 
> To double check, you can easily add an assert at the end of BufferedValuesDealiaser::oops_do to check that indeed your filtering condition is now true, e.g. that (new_mark_word & VTBuffer::mark_mask) == _current_mark
> 
> Regards
> 
> Bertrand
> 
> --
> Freelance - Bertrand Delsart Software Solutions 
> Remote Research, Development and Troubleshooting 
> JVM, Real-Time and Concurrency expert 
> http://www.bdelsart.com
> 
> 
> 2017-11-08 20:39 GMT+01:00 Frederic Parain <frederic.parain at oracle.com>:
> Updated webrev where the memory allocation for the TLVB is changed from
> on-demand mmap() calls to a reserve/lazy commits mechanism.
> 
> http://cr.openjdk.java.net/~fparain/8190430/webrev.01/
> 
> Fred
> 
> 
> > On Oct 31, 2017, at 13:21, Frederic Parain <frederic.parain at oracle.com> wrote:
> >
> > Please review this changeset fixing the GC support and other
> > issues with the Thread-Local Value Buffer:
> >
> > http://cr.openjdk.java.net/~fparain/8190430/webrev.00/
> >
> > This changeset re-activates the TLVB by default.
> > The VTBuffer test has been fixed to generate value types
> > with object references.
> >
> > All hotspot_valhalla tests pass.
> >
> > Thank you,
> >
> > Fred
> 
>