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

[Vladimir Kozlov]

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