RFR (S) 8223657: Remove unused THREAD argument from SymbolTable functions
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Mon May 13 21:39:23 UTC 2019
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
>>>>>
>>>>>
>>>
>>
More information about the hotspot-runtime-dev
mailing list