RFR: Value Array element size / value store fixes...

[Frederic Parain]

Mr Simms,

Thank you cleaning up the code related to value type payload handling.

General comment about coding style: HotSpot coding style recommends
to put the ‘else’ keyword on same line as the closing bracket of the ‘then’
block, not on the line below.

src/share/vm/oops/valueKlass.hpp:
  line 82: I’d like to have some asserts here to check that the return value
               is a valid oop

src/share/vm/oops/valueKlass.cpp:
  line 59 int ValueKlass::raw_value_byte_size()
         I assume support for object references will be added as part of
         your next push, right?

Otherwise, looks good.

Fred


> On Mar 10, 2017, at 07:16, David Simms <david.simms at oracle.com> wrote:
> 
> Hi,
> 
> Whilst in the middle of making sure value reference fields were working I discovered number some bugs which can be fixed separately:
> 
> * valueArrayKlass incorrectly using the heapOopSize aligned size (i.e.
>   nonstatic_field_size()).
>     o This meant small values, like a single byte would use up to 8
>       bytes per element. This was not the intention, it should behave
>       more like typeArrayKlass.
> * ValueKlass::value_store() used "Copy::conjoint_jlongs_atomic()" with
>   an incorrect length.
>     o Caused overwrite on small values (e.g. single narrowOop)
> * ValueKlass::value_store() use of memcpy is implementation dependent
>     o Risk tearing individual primitive and reference fields
> 
> Here's a patch to address the problems:
> 
> http://cr.openjdk.java.net/~dsimms/valhalla/elemsize/webrev0/
> 
> 
> Summary of changes:
> 
> * valueKlass.hpp:
>     o Reworked "raw_value_byte_size()" to return the correct pow2
>       aligned size for small values, otherwise heapOopSize aligned.
>         + This enables the correct element size
>         + Is a little expensive to call given it needs to field iterate
>     o value_store()
>         + Added the ability to specify copy size (for array elements)
>         + Added "raw_field_copy()" to copy the correct size for small
>           values or units of long (much like "JVM_Clone()" does)
>     o Some clean up:
>         + Removed static "valueOopDescBase()", replace usage with
>           "first_field_offset()"
>         + Removed value_store_<x> helpers
> * valueArrayKlass.hpp
>     o Added "element_value_store_size()" to store
>       "ValueKlass::raw_value_byte_size()" since it is costly
>         + Also helps limit the amount bytes copy due to wasted space
>           with pow2 element addressing
>     o Add typeArray style memory copy to "copy_array()" when primitive
>       only.
> 
> Cheers
> 
> /David SImms
> 
>