RFR 8203381 Replace InstanceKlass::allocate_instance_handle with JavaCalls::construct_new_instance

[David Holmes]

On 22/05/2018 2:24 PM, Ioi Lam wrote:
> On 5/20/18 7:05 PM, David Holmes wrote:
>> Hi Ioi,
>>
>> On 19/05/2018 2:34 AM, Ioi Lam wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8203381
>>> http://cr.openjdk.java.net/~iklam/jdk11/8203381-construct-new-instance.v01/ 
>>>
>>>
>>> This patch removes a lot of the boilerplate code for allocating a
>>> Java object and calling its constructor. Hopefully the code is cleaner
>>> and easier to read.
>>
>> That's a great cleanup! Why didn't we do this years ago! :)
>>
>> Only nit is that now call_special is replaced with 
>> construct_new_instance the argument line indentation is no longer 
>> correct.
>>
> Hi David,
> 
> I noticed that there isn't a uniform style of indentation with passing 
> function parameters. Usually people indent to the open parenthesis, but 
> I am hesitant to do that when the open parenthesis is more than 40 chars 
> from the beginning of a line. Otherwise I'll end up with lots of skinny 
> lines. They would also run past 80-char width when you have "wide" 
> parameters.

Yes it is certainly not a uniform rule and often conflicts with other 
line length guidelines. But in this case the change from aligned to 
not-aligned was quite noticeable.

>    return 
> JavaCalls::construct_new_instance(SystemDictionary::Module_klass(),
> vmSymbols::java_lang_module_init_signature(),
>                            loader, module_name, CHECK_NH);
> 
> So if you don't have a big objection, I'll just keep the indentation as is.

It was only a nit so that is fine.

Though I was almost tempted to suggest that you rename 
construct_new_instance to do_construct, as it is the same length as 
call_special. ;-)

Cheers,
David


> Thanks
> - Ioi
>> Great comments on the Thread construction process too! Hope it didn't 

>> take you too long to figure out why you couldn't make the replacement 
>> in those cases!
>>
>> Thanks,
>> David
>>
>>> Also:
>>>
>>> Added assert(InstanceKlass::is_initialized(), ...) in case where
>>> we cannot call JavaCalls::construct_new_instance for thread objects.
>>>
>>> Replaced unnecessary SystemDictionary::resolve_or_null() calls with
>>> SystemDictionary::XXX_klass().
>>>
>>> Tested with hs-tier[1,2,3,4,5]
>>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>>
>>>
>>>
>