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

[bdelsart work]

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
>
>