Unsafe.{get,put}-X-Unaligned; Efficient array comparison intrinsics

[Paul Sandoz]

On Mar 9, 2015, at 6:09 PM, Andrew Haley <aph at redhat.com> wrote:

> On 03/09/2015 04:59 PM, Paul Sandoz wrote:
>> 
>> On Mar 9, 2015, at 5:04 PM, Andrew Haley <aph at redhat.com> wrote:
>> 
>>> On 03/09/2015 03:10 PM, Paul Sandoz wrote:
>>>> Do you want to tackle the single-address access methods as a follow up issue?
>>> 
>>> It's not clear to me that we need any single-address access methods
>>> because we can pass in a null object.  Personally I would prefer that.
>>> Thoughts?
>>> 
>> 
>> I agree that would be desirable. 
>> 
>> I know that people rely on that behaviour for off-heap CAS etc, but
>> i was wondering about such behaviour given that the base and offset
>> should, as documented, be derived from an object/staticFieldBase and
>> objectFieldOffset/staticFieldOffset respectfully.
> 
> The doc for Unsafe.getInt() says:
> 
>  public int getInt(java.lang.Object o,
>                    long offset)
> 
>  Fetches a value from a given Java variable. More specifically, fetches
>  a field or array element within the given object o at the given
>  offset, or (if o is null) from the memory address whose numerical
>  value is the given offset.
> 
> Therefore it seems to me that passing in a null object to the
> unaligned methods would be entirely legitimate and in keeping with the
> rest of Unsafe.
> 

But after that there is also:

* The results are undefined unless one of the following cases is true:
...
* If one of the above cases is true, the call references a specific Java
* variable (field or array element).  However, the results are undefined
* if that variable is not in fact of the type returned by this method.

Perhaps we just need to clear up some inconsistencies in the docs, it's hard to see how this would be undefined for the primitive values.


>> Are there subtle differences? I looked at the C2 code and it seems
>> the results might be equivalent but i cannot say the same for C1
>> from quickly eyeballing the code.
>> 
>> e.g. LibraryCallKit::inline_unsafe_access:
>> 
>> if (!is_native_ptr) {
>>  // The base is either a Java object or a value produced by Unsafe.staticFieldBase
>>  Node* base = argument(1);  // type: oop
>>  // The offset is a value produced by Unsafe.staticFieldOffset or Unsafe.objectFieldOffset
>>  offset = argument(2);  // type: long
>>  // We currently rely on the cookies produced by Unsafe.xxxFieldOffset
>>  // to be plain byte offsets, which are also the same as those accepted
>>  // by oopDesc::field_base.
>>  assert(Unsafe_field_offset_to_byte_offset(11) == 11,
>>         "fieldOffset must be byte-scaled");       <----- weird assertion
> 
> Indeed it is.  There was in the past some weirdness to do with offset
> being a kind of opaque handle rather than guaranteed to be a byte
> offset; I guess that's what this assert is thinking of.
> 

So perhaps i can remove that assert and Unsafe_field_offset_to_byte_offset (i conservatively left them in there in the previous clean up because there is usage in the shark vm code i could not work out was important or not)

Paul.