RFR 8208163 [lworld] performance regression on String(char[]) constructor

[Tobias Hartmann]

Hi Ioi,

this looks good to me.

Thanks,
Tobias

On 05.09.2018 08:36, Ioi Lam wrote:
> Hi Tobias,
> 
> I was finally able to get back on the Valhalla work. I've updated the webrev according to your
> suggestions. What do you think?
> 
> http://cr.openjdk.java.net/~iklam/valhalla/8208163_string_charary_ctor_slowdown.v02/
> 
> Thanks
> 
> - Ioi
> 
> 
> On 8/14/18 7:28 AM, Tobias Hartmann wrote:
>> Hi Ioi,
>>
>> On 10.08.2018 23:37, Ioi Lam wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8208163
>>> http://cr.openjdk.java.net/~iklam/valhalla/8208163_string_charary_ctor_slowdown.v01/
>> It seems that with your change, we now mark all non-array allocations as "unknown value", right?
>> This will cause PhaseMacroExpand::expand_allocate_common() to add additional runtime checks. It
>> might be better to check for primitive arrays (TypeArrayKlass) explicitly.
>>
>> Also, your comment is a bit misleading because it suggests that the set_unknown_value() code is only
>> used for reflective array creation. However, that code should be invoked whenever we create an array
>> and don't know if the element type is a value type.
>>
>> To improve readability, I think it would also be good to wrap the entire code starting from line
>> 3972 into 'if (EnableValhalla)' and not only the else branch.
>>
>> Thanks,
>> Tobias
>