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

David Holmes david.holmes at oracle.com
Mon May 13 12:24:47 UTC 2019


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


More information about the hotspot-runtime-dev mailing list