RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

Konstantin Shefov konstantin.shefov at oracle.com
Fri Dec 11 11:54:00 UTC 2015


Hello

Please review the new version on the patch.

New webrev:
Webrev hotspot: 
http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.02
Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.02

What has been changed:
1. Added tests for the new added methods.
2. Removed CP tag codes 100 - 105 from being passed to java and left 
only codes from the open JVM spec 
(https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4-140).

Thanks
-Konstantin

On 11/27/2015 07:48 PM, Konstantin Shefov wrote:
> Coleen,
> Thanks for review
>
> On 11/24/2015 07:33 PM, Coleen Phillimore wrote:
>>
>> I have a couple preliminary comments.  First, there are no tests 
>> added with all this new functionality.  Tests should be added with 
>> the functionality changeset, not promised afterward. 
> I will add tests.
>> Secondly, I do not like that JDK code knows the implementation 
>> details of the JVM's constant pool tags.  These should be only for 
>> internal use.
> The package "sun.reflect" is for internal use only, so it shouldn’t be 
> a problem.
>> My third comment is that I haven't looked carefully at this constant 
>> pool implementation but it seems very unsafe if the class is 
>> redefined and relies on an implementation detail in the JVM that can 
>> change.   I will have more comments once I look more at the jvmti 
>> specification.
>>
>> thanks,
>> Coleen
>>
>> On 11/24/15 9:48 AM, Konstantin Shefov wrote:
>>> Hello
>>>
>>> Please, review modified webrev.
>>>
>>> I have added methods
>>> getNameAndTypeRefIndexAt(int index) - to get name and type entry 
>>> index from a method, field or indy entry index;
>>> getClassRefIndexAt(int index) - to get class entry index from a 
>>> method or field entry index;
>>>
>>> I removed previously added method
>>> getInvokedynamicRefInfoAt(int index) - as it is no more needed now.
>>>
>>> New webrev:
>>> Webrev hotspot: 
>>> http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.01
>>> Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.01
>>>
>>> Thanks
>>> -Konstantin
>>>
>>> On 11/18/2015 02:11 PM, Konstantin Shefov wrote:
>>>> Remi,
>>>>
>>>> Thanks for reviewing. Your suggestion sounds sensibly.
>>>> May be it also has sense to make a method 
>>>> "getMethodRefNameAndTypeIndex(int index)" to acquire name-and-type 
>>>> entry index for methods also.
>>>>
>>>> -Konstantin
>>>>
>>>> On 11/18/2015 12:04 AM, Remi Forax wrote:
>>>>> Hi Konstantin,
>>>>> Technically, getInvokedynamicRefInfoAt should be 
>>>>> getNameAndTypeOfInvokedynamicRefInfoAt because it only extract the 
>>>>> nameAndType value of the InvokeDynamicRef.
>>>>>
>>>>> In term of API, I think it's better to decompose the API, i.e. to 
>>>>> have a method
>>>>>    int getInvokedynamicRefNameAndTypeIndex(int index)
>>>>> that returns the nameAndType index and to reuse 
>>>>> getNameAndTypeRefInfoAt(index) to get the corresponding array of 
>>>>> Strings.
>>>>>
>>>>> cheers,
>>>>> Rémi
>>>>>
>>>>> ----- Mail original -----
>>>>>> De: "Christian Thalinger" <christian.thalinger at oracle.com>
>>>>>> À: "Konstantin Shefov" <konstantin.shefov at oracle.com>
>>>>>> Cc: "hotspot-dev developers" <hotspot-dev at openjdk.java.net>, 
>>>>>> core-libs-dev at openjdk.java.net
>>>>>> Envoyé: Lundi 16 Novembre 2015 23:41:45
>>>>>> Objet: Re: RFR [9] 8141615: Add new public methods to 
>>>>>> sun.reflect.ConstantPool
>>>>>>
>>>>>> [CC'ing core-libs-dev]
>>>>>>
>>>>>>> On Nov 13, 2015, at 4:55 AM, Konstantin Shefov
>>>>>>> <konstantin.shefov at oracle.com> wrote:
>>>>>>>
>>>>>>> Hello
>>>>>>>
>>>>>>> Please review an enhancement to add three new methods to
>>>>>>> sun.reflect.ConstantPool class.
>>>>>>> The following methods are suggested:
>>>>>>>
>>>>>>> public String[] getNameAndTypeRefInfoAt(int index) - returns string
>>>>>>> representation of name and type from a NameAndType constant pool 
>>>>>>> entry
>>>>>>> with the specified index
>>>>>>>
>>>>>>> public String[] getInvokedynamicRefInfoAt(int index) - returns 
>>>>>>> string
>>>>>>> representation of name and type from an InvokeDynamic constant 
>>>>>>> pool entry
>>>>>>> with the specified index
>>>>>>>
>>>>>>> public Tag getTagAt(int index) - returns a Tag enum instance of 
>>>>>>> a constant
>>>>>>> pool entry with the specified index
>>>>>>>
>>>>>>> These three methods could be used for testing API working with 
>>>>>>> constant
>>>>>>> pool, e.g. JVMCI.
>>>>>>>
>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8141615
>>>>>>> Webrev hotspot:
>>>>>>> http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.00
>>>>>>> Webrev jdk: 
>>>>>>> http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.00
>>>>>>>
>>>>>>> Thanks
>>>>>>> -Konstantin
>>>>>>
>>>>
>>>
>>
>




More information about the core-libs-dev mailing list