RFR: JDK-8205549 JDK-8205698 Support of flattened values in Unsafe

[Frederic Parain]

John,

This is an emergency fix (Unsafe was not part of the plan for LW1, until
we discovered VarHandles needed it), it focuses on robustness, not on
performance.

Looking  longer term:

  1 - retrieving all information for flattenable/flattened fields or value
       containers is expensive. I think there’s a clear consensus that
       new methods are required to efficiently support access to
       flattened/flattenable fields.  But the LW1 date is fast approaching,
       so I’m not convinced that rushing these new APIs for LW1 is a good
       choice.

  2 - deprecating U.getObject() and U.putObject(), for more precise
       methods makes sense, however deprecating is often a slow process
       in the Java world. An alternate solution would be to keep getObject()
       and putObject() while proposing the new alternative. It seems possible
       to restore almost all performance of getObject()/putObject() for the
       non value types cases (not part of the current patch, I wanted to
       push the fix ASAP to fix the current regressions). The trick is that
       objectFieldOffset() is not guaranteed to return a true offset, it could
       be some kind of a handle. It is possible to use three bits out of the
       64bits of the offset to encode the information the JVM needs
      (flattened field/flattenable field/value container). Internally, Hotspot
      is already using 3 bits of the field offset metadata to encode some
      information. With a simple mask and test against zero, it would be
      very simple to detect that no value type is involved and the code
      could directly perform the legacy behavior.
      This would preserve the performance of GetObject()/putObject()
      for legacy code, the time for the code base to migrate to the
      new APIs, and make the support of value almost transparent
      for this code (except for performance).
      Once all codes have been migrated, getObject()/putObject() would
      be removed.
      This trick could also be applied to arrayBaseOffset() for the support
      of arrays.

Fred




> On Jun 27, 2018, at 18:22, John Rose <john.r.rose at oracle.com> wrote:
> 
> On Jun 27, 2018, at 2:17 PM, Frederic Parain <frederic.parain at oracle.com> wrote:
>> 
>> Please review this changeset which adds support for flattened values to the Unsafe API.
>> 
>> http://cr.openjdk.java.net/~fparain/Unsafe/webrev.02/index.html
>> 
>> This changeset addresses the following issues:
>> - reading from /writing to a flattenable/flattened field with Unsafe
>> - null-check when writing to a flattenable field
>> - initialization barrier when reading an uninitialized static value field with Unsafe
>> - enforcing values immutability even when using Unsafe
>> - reading from /writing to a value array with Unsafe
>> - null-check when writing to a value array with Unsafe
>> 
>> Thanks,
>> 
>> Fred
> 
> Impressive work Fred; you have a nice way to doing the impossible.
> 
> The existing version of U.putObject doesn't look at metadata, but clearly
> the new version has to depend on metadata to see how the container is shaped.
> 
> I see how this can work the way you did it, and why it's useful to unblock
> prototyping.  But it's not a design that I want to see made permanent.
> 
> Container analysis is a delicate and potentially buggy operation which I am
> surprised can be hidden safely under U.putObject.  The basic contract of
> Unsafe is to force the user to take responsibility for querying the container,
> and issue the U.putX that matches the container.  The benefit of this contract
> is not only avoiding delicate and buggy operations inside Unsafe, but also
> a simpler optimization model.
> 
> So, I view this as a work-in-progress, which can unblock existing
> code that uses Unsafe without requiring that code to be revamped.
> 
> Later, the get/putObject APIs need to be split into versions which explicitly
> manage references vs. flat values.  Unsafe-using code that wants full
> optimization will want to avoid get/putObject if it has this complicated
> stuff going on under the hood.
> 
> New API points are needed to let the user work explicitly with the
> container shape:
> 
>    public Object getValue(Object o, long offset, Class<?> valueType);
>    public void putValue(Object o, long offset, Class<?> valueType, Object value);
>    public Object getReference(Object o, long offset);
>    public void putReference(Object o, long offset, Object reference);
> 
> The get/putValue API points would load and store values of the given type
> in their native format (flattened).  If you knew a container wasn't flattened
> although the type could be, you'd call putReference.
> 
> We could also deprecate get/putObject so that users will be forced to
> do the container analysis.  An non-deprecated version of your container
> analysis should have a more complex-sounding name, such as
> getReferenceOrValue.  Or we should just force users to write their
> own; it's simple if the hooks are provided (see below).
> 
> The valueType operand is there even when the value is also present,
> 
for robustness.  If we derive valueType from value automatically, the
> user loses a channel for asserting the container analysis.  You could
> get heap-scribbling bugs when the value is of the wrong type.  Oddly
> enough, the Unsafe API has a concern with safety:  You have to give
> the user a reasonable chance to write safe code using Unsafe.
> 
> I think we also want queries which can execute the logic you have written,
> directly, and not in the course of reading or writing the heap.  Something
> like this would give unsafe users control over factoring the metadata
> queries from the access itself:
> 
>    public Class<?> objectFieldContainerType(Field f);
>    public Field findObjectField(Class<?> objClass, long offset);
>    public Object getReferenceOrValue(Object o, long offset) {
>      Class<?> ctype = objectFieldContainerType(findObjectField(o.getClass(), offset));
>      if (ctype.isValue())  return getValue(o, offset, ctype);
>      else return getReference(o, offset);
>    }
> 
> All that said, for the present review, it looks good as a work in progress.
> 
> (If the benefit of overloading get/putObject is small, consider naming
> this new logic with a new name like getReferenceOrValue.  But I'm guessing
> the overloading trick is a short-term requirement, to avoid cracking open
> lots of platform code.)
> 
> If we don't have another bug to track Unsafe support for container analysis
> of flat fields, let's file one as a followup to this.
> 
> — John