RFR 8029567: Clean up linkResolver code

[Coleen Phillimore]

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
>>>
>>
>