RFR 8029567: Clean up linkResolver code

[Yumin Qi]

Thanks. I would think if JPRT integrated with your commit script so we 
don't care about the file header updating.

On 5/29/2015 11:57 AM, Coleen Phillimore wrote:
>
> Hi Yumin,
> My 'commit' script automatically updates copyright years.  I should 
> have mentioned that.
> Thanks,
> Coleen
>
> On 5/29/15 2:25 PM, Yumin Qi wrote:
>> Coleen,
>>
>> *src/share/vm/ci/ciMethod.cpp*
>> *src/share/vm/classfile/loaderConstraints.cpp
>> **src/share/vm/classfile/loaderConstraints.hpp
>> **src/share/vm/interpreter/bytecode.cpp*
>> *src/share/vm/interpreter/linkResolver.hpp*
>> *src/share/vm/runtime/javaCalls.cpp
>>
>> *Those files need copyright 'year' to be updated.
>>
>> Thanks
>> Yumkin
>> On 5/28/2015 3:34 PM, Coleen Phillimore wrote:
>>>
>>> The change (jck testing in progress) with the loader constraint 
>>> changes backed out.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8029567.03/
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 5/28/15 5:59 PM, Coleen Phillimore wrote:
>>>>
>>>> Hi Karen,
>>>>
>>>> Yes, I'll back out the loader constraint changes.  I was trying to 
>>>> merge the error message construction code for failing loader 
>>>> constraint checks since it's similar code in 5 places but I gave 
>>>> up, and made the CLD change as a side effect.
>>>> We store ClassLoaderData in the loader constraint table now but 
>>>> this is a good find, because I think there is a behavior change 
>>>> here in the comparison.
>>>>
>>>> Thanks!
>>>> Coleen
>>>>
>>>> On 5/28/15 5:43 PM, Karen Kinnear wrote:
>>>>> Coleen,
>>>>>
>>>>> 1. Thank you for highlighting where we have check_access as false 
>>>>> - so we can make sure we
>>>>> really really meant to do that going forward.
>>>>>
>>>>> 2. I am concerned about the loader constraint changes to use class 
>>>>> loader data rather than class loaders.
>>>>> Since these are not 1:1 - what about use cases in which the class 
>>>>> loaders are the same but the loader datas are not?
>>>>>
>>>>> In the Valhalla exploration of ClassDynamic, we are exploring 
>>>>> having dynamic classes, which would be defined
>>>>> via DefineAnonymousClass - which can be used in signatures, so we 
>>>>> would need to be able to compare types which may
>>>>> have different loader datas but might need to compare as 
>>>>> equivalent since they have the same class loader.
>>>>>
>>>>> I believe the intention of this set of changes is cleanup - not 
>>>>> meant to change behavior.
>>>>>
>>>>> But that change actually is a meaningful behavior change - and I 
>>>>> think it is premature to make it.
>>>>>
>>>>> Could you possibly back out the loader constraint related changes?
>>>>>
>>>>> Those will take more thought, more future designs, and detailed 
>>>>> targeted tests.
>>>>>
>>>>> thanks,
>>>>> Karen
>>>>>
>>>>> On May 14, 2015, at 7:16 PM, Coleen Phillimore wrote:
>>>>>
>>>>>> Summary: Moved non-const reference return values to actual return 
>>>>>> values, refactored error handling code, pass CLD rather than 
>>>>>> class_loader oop, remove oop from Method* variable names.
>>>>>>
>>>>>> Also modified the long parameter lists into a class LinkInfo to 
>>>>>> hold information from the constant pool that is always passed 
>>>>>> through several layers of functions together.  Also reformatted 
>>>>>> to split parameter list lines to some reasonable width.
>>>>>>
>>>>>> The type methodHandle should be passed as const reference types 
>>>>>> to avoid copy construction, because it has a non-trivial 
>>>>>> destructor. This sort of change could be made in more places in 
>>>>>> the JVM, but I stopped with linkResolver.
>>>>>>
>>>>>> Ran all hotspot jtreg tests, jck lang/vm/api/java_lang tests, 
>>>>>> internal testbase tests: vm.quick.testlist, vm.defmeth.testlist, 
>>>>>> proposed new selection-resolution tests, jdk/java/lang/invoke 
>>>>>> tests (see RFR).
>>>>>>
>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8029567/
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8029567
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>
>>>
>>
>