RFR: Value Array element size / value store fixes...
frederic.parain at oracle.com
Fri Mar 10 16:19:40 UTC 2017
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.
line 82: I’d like to have some asserts here to check that the return value
is a valid oop
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.
> On Mar 10, 2017, at 07:16, David Simms <david.simms at oracle.com> wrote:
> 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.
> 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:
> 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
> + 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
> /David SImms
More information about the valhalla-dev