review(S): 7058510: multinewarray with 6 dimensions uncommon traps in server compiler

[Igor Veresov]

John, Vladimir, Peter,

Thank you for your comments.
I've made the changes and tested with my microbenchmark and DeoptimizeALot.

Webrev: http://cr.openjdk.java.net/~iveresov/7058510/webrev.03/

Thanks!
igor

On 7/1/11 2:00 PM, Vladimir Kozlov wrote:
> John Rose wrote:
>> On Jul 1, 2011, at 1:26 PM, Vladimir Kozlov wrote:
>>
>>> Wrap new java dimension allocation into PreserveReexecuteState scope
>>> and restore stack.
>>
>> Is this in case the initial allocation deoptimizes (due to some
>> external event)? I suppose that should be tested with DeoptimizeALot.
>
> The allocation of dimension array may go slow path (no space left in
> TLAB) into runtime and on return the method could be deoptimized so we
> need restore stack so that interpreter can reexecute multinewarray
> allocation. There are several places where we do this already (for
> example, around the call to expand_multianewarray()).
>
> And yes, it should be tested with DeoptimizeALot.
>
> Vladimir
>
>>
>> Igor, the extraction from the dope array bothers me:
>> + jint *j_dims = (jint*)dims->base(T_INT);
>> + jint *c_dims = NEW_RESOURCE_ARRAY(jint, len);
>> + memcpy(c_dims, j_dims, sizeof(jint)*len);
>>
>> It is correct, but I'd rather see more use of Hotspot machinery, with
>> stronger typing:
>> + jint *j_dims = typeArrayOop(dims)->int_at_addr(0);
>> + jint *c_dims = NEW_RESOURCE_ARRAY(jint, len);
>> + Copy::conjoint_jints_atomic(c_dims, j_dims, sizeof(jint)*len);
>>
>> Note that Copy::* strongly outnumbers memcpy in the opto directory.
>> The distinction is mostly stylistic, but Copy:: is more strongly typed
>> in this case.
>>
>> (In other uses, Copy:: is more explicit and careful about some of the
>> things we need to care about, such as overlaps and word-tearing.)
>>
>> Also, use is_typeArray instead of is_array in the previous assert.
>> That will make the cast to typeArrayOop safe.
>>
>> -- John