[9] RFR (S): 8153540: C2 intrinsic for Unsafe.allocateInstance doesn't properly filter out array classes

[Vladimir Kozlov]

You did not fix comment:

+ // public native Object Unsafe.allocateInstance(Class<?> cls);

should be:

+ // private native Object allocateInstance0(Class<?> cls) throws 
InstantiationException;

An other question: does it really throw InstantiationException?

Thanks,
Vladimir

On 4/12/16 4:07 AM, Vladimir Ivanov wrote:
>
>>> Additional flag in a mirror (j.l.Class) which marks instance klasses
>>> could help here, but I'm still not sure it's worth the effort.
>>>
>>> Ideally, something like [1] (which requires 2 new intrinsics):
>>
>> I would advise against that.  We are fixing a long-standing bug here
>> and although we see a regression the code we produced before was just
>> wrong.  Comparing against something that was wrong in the first place
>> is moot.
>
> It wasn't intended as a call for action, but more like a backup plan if
> there's a need to speed up the reflection case.
>
> I'd like to keep the fix simple and current version looks good enough:
>    http://cr.openjdk.java.net/~vlivanov/8153540/webrev.00
>
> Any Reviews, please?
>
> Best regards,
> Vladimir Ivanov
>
>>
>> Take the hit; I doubt it will show up at customer applications.
>>
>>>
>>>   * isFastAllocatable() performs all necessary checks: null checks on
>>> cls, not primitive, not array, not interface, not abstract, fully
>>> initialized, no finalizers;
>>>
>>>   * allocateInstanceSlow() handles all cases the intrisic doesn't
>>> handle: either throws IE or does necessary operations (e.g.,
>>> initialize the class or register a finalizer) when instantiating an
>>> object.
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> [1]
>>>     @ForceInline
>>>     public Object allocateInstance(Class<?> cls) throws
>>> InstantiationException {
>>>         // Interfaces and abstract classes are handled by the intrinsic.
>>>         if (isFastAllocatable(cls)) {
>>>             return allocateInstance0(cls);
>>>         } else {
>>>             return allocateInstanceSlow(cls);
>>>         }
>>>     }
>>>
>>>     @HotSpotIntrinsicCandidate
>>>     private native boolean isFastAllocatable(Class<?> cls);
>>>
>>>     @HotSpotIntrinsicCandidate
>>>     private native Object allocateInstance0(Class<?> cls) throws
>>> InstantiationException;
>>>
>>>     // Calls into modified OptoRuntime::new_instance_C
>>>     @HotSpotIntrinsicCandidate
>>>     private native Object allocateInstanceSlow(Class<?> cls) throws
>>> InstantiationException;
>>>
>>>
>>>>
>>>>> [1] http://cr.openjdk.java.net/~vlivanov/8153540/AllocInstance.java
>>>>
>>>> Suggestions to improve fidelity:
>>>>    * Run allocation benchmarks with -Xmx1g -Xms1g; this improves
>>>> variance
>>>>    * Add @CompilerControl(CompilerControl.Mode.DONT_INLINE) on
>>>> @Benchmarks if you want to use -prof perfasm
>>>>
>>>> Thanks,
>>>> -Aleksey
>>>>
>>>>
>>