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

[Paul Sandoz]

Hi John,

Those new API points you propose match closely with what i had in mind, and i also agree that a set of query methods are important (I was just scratching the surface of that).

IMHO i think we can make a tentative split now and that would also increase prototyping velocity without potentially destabilizing  existing unsafe usages. Mandy’s patch indicates the amount of Java platform code changes are manageable for such a split and it can allow for quicker experimentation with VarHandles.  

Paul.

> On Jun 27, 2018, at 3:22 PM, 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