RFR(xs): 8149096: Remove unused code in methodComparator

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Tue Feb 9 23:56:32 UTC 2016


Coleen,

I've already suggested my sponsorship to Thomas.
Thank you for identifying more dead code!

I'll wait for comments from David before push.

Thanks,
Serguei


On 2/9/16 15:04, Coleen Phillimore wrote:
>
> I agree.  This looks great!  Serguei, do you want to sponsor it.
>
> Thank you!
> Coleen
>
> On 2/9/16 5:32 PM, serguei.spitsyn at oracle.com wrote:
>> Hi Thomas,
>>
>> This looks great.
>> Thank you a lot!
>>
>> Thanks,
>> Serguei
>>
>>
>> On 2/9/16 08:26, Thomas Stüfe wrote:
>>> Hi Coleen,
>>>
>>> You were right, there was more dead code. See my new webrev here:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8149096-remove-unused-code-methodComparator/webrev.02/webrev/ 
>>>
>>>
>>> Kind Regards, Thomas
>>>
>>> On Tue, Feb 9, 2016 at 1:34 PM, Coleen Phillimore <
>>> coleen.phillimore at oracle.com> wrote:
>>>
>>>> Hi, So this was really good of Serguei to find that the code under
>>>> _switchable_test is dead.   It seems that old_and_new_locations_same()
>>>> appears dead also now.  Do you mind continuing to unravel this?  Also
>>>> new_bci_for_old() is dead, and _old_bci, _new_st_bci and _new_end_bci.
>>>> Unless I'm reading this wrong.  Simplifying anything in 
>>>> redefinition is
>>>> greatly appreciated, especially this.
>>>>
>>>> thanks,
>>>> Coleen
>>>>
>>>>
>>>> On 2/9/16 4:36 AM, Thomas Stüfe wrote:
>>>>
>>>>> Hi all,
>>>>>
>>>>> thanks to all for your feedback. Here is the new webrev:
>>>>>
>>>>>
>>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8149096-remove-unused-code-methodComparator/webrev.01/webrev/index.html 
>>>>>
>>>>>
>>>>> I removed _switchable_test and all code dependending from it.
>>>>>
>>>>> Thomas
>>>>>
>>>>> On Tue, Feb 9, 2016 at 7:17 AM, serguei.spitsyn at oracle.com <
>>>>> serguei.spitsyn at oracle.com> wrote:
>>>>>
>>>>> On 2/8/16 20:20, David Holmes wrote:
>>>>>> On 9/02/2016 5:22 AM, serguei.spitsyn at oracle.com wrote:
>>>>>>> Hi Thomas,
>>>>>>>> The fix looks good.
>>>>>>>> The following variable and all its uses are dead too as it 
>>>>>>>> never gets
>>>>>>>> 'true' value.
>>>>>>>>
>>>>>>>> I think _switchable_test is the variable Serguei was referring to.
>>>>>>> Yes.
>>>>>> I forgot to paste the variable name. :)
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>>
>>>>>> The whole notion of switchability is "dead" now so all code that 
>>>>>> relates
>>>>>>
>>>>>>> to _switchable_test being true is also dead. Also any comment 
>>>>>>> use of
>>>>>>> switchable.
>>>>>>>
>>>>>>> But it is up to you to fix it or not.
>>>>>>> I'd like to see it all cleaned up please. :)
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>
>>>>>>> I can sponsor and push the fix.
>>>>>>>> Thank you for taking care about this.
>>>>>>>> Serguei
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2/8/16 06:24, Thomas Stüfe wrote:
>>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>> please review and sponsor this little change. It gets rid of two
>>>>>>>>> unused methods in methodComparator(cpp/hpp).
>>>>>>>>>
>>>>>>>>> We stumbled over this because of warings about unchecked 
>>>>>>>>> realloc()
>>>>>>>>> return values; but instead of fixing the code, we may just 
>>>>>>>>> remove the
>>>>>>>>> offending methods, because they are not used anywhere.
>>>>>>>>>
>>>>>>>>> See also mail thread here:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://openjdk.5641.n7.nabble.com/Unused-code-in-prims-methodComparator-td254425.html 
>>>>>>>>>
>>>>>>>>> .
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8149096
>>>>>>>>>
>>>>>>>>> webrev:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8149096-remove-unused-code-methodComparator/webrev.00/webrev/ 
>>>>>>>>>
>>>>>>>>> <
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/%7Estuefe/webrevs/8149096-remove-unused-code-methodComparator/webrev.00/webrev/ 
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Kind Regards, Thomas
>>>>>>>>>
>>>>>>>>>
>>
>



More information about the hotspot-runtime-dev mailing list