RFR (S) 8223657: Remove unused THREAD argument from SymbolTable functions

[coleen.phillimore at oracle.com]

Thanks Gerard!
Coleen

On 5/14/19 11:49 AM, gerard ziemski wrote:
> Thank you for posting incremental webrev, looks good.
>
>
> cheers
>
> On 5/13/19 4:39 PM, coleen.phillimore at oracle.com wrote:
>>
>> The functions SignatureStream::as_symbol and as_symbol_or_null() 
>> don't need the TRAPS argument for the same reason, neither do 
>> java_lang_Class::as_symbol, as_symbol_or_null or as_signature. This 
>> propagated not needing TRAPS to a couple verifier functions as well.  
>> These parameters and arguments to the calls are all removed. This 
>> makes (S) into (M) I guess, but it's simple.
>>
>> http://cr.openjdk.java.net/~coleenp/2019/8223657.02.incr/webrev/index.html 
>>
>>
>> And full:
>>
>> open webrev at 
>> http://cr.openjdk.java.net/~coleenp/2019/8223657.02/webrev
>>
>> This was tested with hs-tier1-3 on Oracle platforms.  Non-oracle 
>> platform changes are trivial.
>>
>> Thanks,
>> Coleen
>>
>> On 5/13/19 8:24 AM, David Holmes wrote:
>>> On 13/05/2019 10:14 pm, coleen.phillimore at oracle.com wrote:
>>>>
>>>>
>>>> On 5/13/19 7:55 AM, coleen.phillimore at oracle.com wrote:
>>>>>
>>>>> Thanks David for reviewing.
>>>>>
>>>>> On 5/12/19 6:59 PM, David Holmes wrote:
>>>>>> Hi Coleen,
>>>>>>
>>>>>> Generally this all looks fine. A couple of follow ups:
>>>>>>
>>>>>> src/hotspot/share/classfile/classListParser.cpp
>>>>>>
>>>>>> 329   TempNewSymbol class_name_symbol = 
>>>>>> SymbolTable::new_symbol(_class_name);
>>>>>>  330   guarantee(!HAS_PENDING_EXCEPTION, "Exception creating a 
>>>>>> symbol.");
>>>>>
>>>>> Yup, I should have removed that.
>>>>>>
>>>>>> This guarantee seems wrong even before this change, but if no 
>>>>>> exceptions can come from new_symbol then it serves no purpose now.
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> src/hotspot/share/classfile/javaClasses.cpp
>>>>>>
>>>>>> 572 Symbol* java_lang_String::as_symbol(oop java_string, TRAPS) {
>>>>>>
>>>>>> TRAPS is unused now. Further there seems to be no reason to have 
>>>>>> both java_lang_String::as_symbol and 
>>>>>> java_lang_String::as_symbol_or_null as the only distinction 
>>>>>> appears to be that the former was expected to post an exception 
>>>>>> if it would return NULL. But apparently it is impossible to 
>>>>>> return NULL as we should have aborted the VM on allocation 
>>>>>> failure - right?
>>>>>
>>>>> Yes, exactly.  I can remove as_symbol_or_null as part of this 
>>>>> change. Let me see if it increases the change too much, if so, 
>>>>> I'll file a follow-up RFE.  Thanks for finding these!
>>>>
>>>> Actually looking again, as_symbol_or_null is a probe but as_symbol 
>>>> creates a new_symbol.  So there is a distinction. But the as_symbol 
>>>> function doesn't require the TRAPS argument anymore.  There's also 
>>>> the same in SignatureStream.
>>>
>>> Ah - sorry - yes read that too quick. I'm glad you've renamed the 
>>> lookup functions to new_symbol to make it clearer.
>>>
>>> Thanks,
>>> David
>>>
>>>>
>>>> Coleen
>>>>
>>>>>
>>>>> Coleen
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>> On 11/05/2019 1:15 am, coleen.phillimore at oracle.com wrote:
>>>>>>> Summary: also made lookup and lookup_only functions private to 
>>>>>>> SymbolTable. External callers use new_symbol or probe.
>>>>>>>
>>>>>>> Ran hs-tier1-3.
>>>>>>>
>>>>>>> open webrev at 
>>>>>>> http://cr.openjdk.java.net/~coleenp/2019/8223657.01/webrev
>>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8223657
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Coleen
>>>>>>>
>>>>>>>
>>>>>
>>>>
>>
>>
>