RFR (S): 8176831: Dead code: function jmm_GetLoadedClasses is not used in jmm_interface

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR (S): 8176831: Dead code: function jmm_GetLoadedClasses is not used in jmm_interface

serguei.spitsyn@oracle.com
Please, review the jdk 10 fix for:
   https://bugs.openjdk.java.net/browse/JDK-8176831


Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8176831-jmm-dead.1/


Summary:

   It was found at the review of the 8155672 that the function
   jmm_GetLoadedClasses() defined in services/management.cpp
   is not really used in the jmm_interface. This function and
   dead code associated with its implementation is removed.

Testing:
   The nsk.monitoring and jtreg jdk_management tests are in progress.


Thanks,
Serguei




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (S): 8176831: Dead code: function jmm_GetLoadedClasses is not used in jmm_interface

David Holmes
Looks good!

Don't forget to update copyright years.

Thanks,
David

On 17/03/2017 10:59 AM, [hidden email] wrote:

> Please, review the jdk 10 fix for:
>   https://bugs.openjdk.java.net/browse/JDK-8176831
>
>
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8176831-jmm-dead.1/
>
>
>
> Summary:
>
>   It was found at the review of the 8155672 that the function
>   jmm_GetLoadedClasses() defined in services/management.cpp
>   is not really used in the jmm_interface. This function and
>   dead code associated with its implementation is removed.
>
> Testing:
>   The nsk.monitoring and jtreg jdk_management tests are in progress.
>
>
> Thanks,
> Serguei
>
>
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (S): 8176831: Dead code: function jmm_GetLoadedClasses is not used in jmm_interface

serguei.spitsyn@oracle.com
David,

Thank you for the review!


On 3/16/17 18:15, David Holmes wrote:
> Looks good!
>
> Don't forget to update copyright years.

Yes, of course.


Thanks,
Serguei

>
> Thanks,
> David
>
> On 17/03/2017 10:59 AM, [hidden email] wrote:
>> Please, review the jdk 10 fix for:
>>   https://bugs.openjdk.java.net/browse/JDK-8176831
>>
>>
>> Webrev:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8176831-jmm-dead.1/ 
>>
>>
>>
>>
>> Summary:
>>
>>   It was found at the review of the 8155672 that the function
>>   jmm_GetLoadedClasses() defined in services/management.cpp
>>   is not really used in the jmm_interface. This function and
>>   dead code associated with its implementation is removed.
>>
>> Testing:
>>   The nsk.monitoring and jtreg jdk_management tests are in progress.
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (S): 8176831: Dead code: function jmm_GetLoadedClasses is not used in jmm_interface

serguei.spitsyn@oracle.com
I've reloaded the webrev with pulled latest jdk10 changes.
It looks almost the same but has the recent update from Coleen.

Thanks,
Serguei


On 3/16/17 18:20, [hidden email] wrote:

> David,
>
> Thank you for the review!
>
>
> On 3/16/17 18:15, David Holmes wrote:
>> Looks good!
>>
>> Don't forget to update copyright years.
>
> Yes, of course.
>
>
> Thanks,
> Serguei
>>
>> Thanks,
>> David
>>
>> On 17/03/2017 10:59 AM, [hidden email] wrote:
>>> Please, review the jdk 10 fix for:
>>>   https://bugs.openjdk.java.net/browse/JDK-8176831
>>>
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8176831-jmm-dead.1/ 
>>>
>>>
>>>
>>>
>>> Summary:
>>>
>>>   It was found at the review of the 8155672 that the function
>>>   jmm_GetLoadedClasses() defined in services/management.cpp
>>>   is not really used in the jmm_interface. This function and
>>>   dead code associated with its implementation is removed.
>>>
>>> Testing:
>>>   The nsk.monitoring and jtreg jdk_management tests are in progress.
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>
>>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (S): 8176831: Dead code: function jmm_GetLoadedClasses is not used in jmm_interface

coleen.phillimore
Serguei,  This change looks great!  This was one of the
SystemDictionary::classes_do function calls that I could not resolve why
it didn't want the array classes or anonymous classes, loaded classes in
error or redefined scratch classes, or what the purpose of the function
was in general.

Are uncalled function removals "trivial fixes" only needing one code
review?   You have two now.
Coleen

On 3/16/17 9:38 PM, [hidden email] wrote:

> I've reloaded the webrev with pulled latest jdk10 changes.
> It looks almost the same but has the recent update from Coleen.
>
> Thanks,
> Serguei
>
>
> On 3/16/17 18:20, [hidden email] wrote:
>> David,
>>
>> Thank you for the review!
>>
>>
>> On 3/16/17 18:15, David Holmes wrote:
>>> Looks good!
>>>
>>> Don't forget to update copyright years.
>>
>> Yes, of course.
>>
>>
>> Thanks,
>> Serguei
>>>
>>> Thanks,
>>> David
>>>
>>> On 17/03/2017 10:59 AM, [hidden email] wrote:
>>>> Please, review the jdk 10 fix for:
>>>>   https://bugs.openjdk.java.net/browse/JDK-8176831
>>>>
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8176831-jmm-dead.1/ 
>>>>
>>>>
>>>>
>>>>
>>>> Summary:
>>>>
>>>>   It was found at the review of the 8155672 that the function
>>>>   jmm_GetLoadedClasses() defined in services/management.cpp
>>>>   is not really used in the jmm_interface. This function and
>>>>   dead code associated with its implementation is removed.
>>>>
>>>> Testing:
>>>>   The nsk.monitoring and jtreg jdk_management tests are in progress.
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>>
>>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (S): 8176831: Dead code: function jmm_GetLoadedClasses is not used in jmm_interface

serguei.spitsyn@oracle.com
Hi Coleen,


On 3/17/17 05:29, [hidden email] wrote:
> Serguei,  This change looks great!  This was one of the
> SystemDictionary::classes_do function calls that I could not resolve
> why it didn't want the array classes or anonymous classes, loaded
> classes in error or redefined scratch classes, or what the purpose of
> the function was in general.

Yes, it is nice one of the problematic spots is gone.

>
> Are uncalled function removals "trivial fixes" only needing one code
> review?   You have two now.
Thank you for the review!
It is better to have two anyway. :)

Thanks,
Serguei


> Coleen
>
> On 3/16/17 9:38 PM, [hidden email] wrote:
>> I've reloaded the webrev with pulled latest jdk10 changes.
>> It looks almost the same but has the recent update from Coleen.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 3/16/17 18:20, [hidden email] wrote:
>>> David,
>>>
>>> Thank you for the review!
>>>
>>>
>>> On 3/16/17 18:15, David Holmes wrote:
>>>> Looks good!
>>>>
>>>> Don't forget to update copyright years.
>>>
>>> Yes, of course.
>>>
>>>
>>> Thanks,
>>> Serguei
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 17/03/2017 10:59 AM, [hidden email] wrote:
>>>>> Please, review the jdk 10 fix for:
>>>>>   https://bugs.openjdk.java.net/browse/JDK-8176831
>>>>>
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8176831-jmm-dead.1/ 
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Summary:
>>>>>
>>>>>   It was found at the review of the 8155672 that the function
>>>>>   jmm_GetLoadedClasses() defined in services/management.cpp
>>>>>   is not really used in the jmm_interface. This function and
>>>>>   dead code associated with its implementation is removed.
>>>>>
>>>>> Testing:
>>>>>   The nsk.monitoring and jtreg jdk_management tests are in progress.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>
>

Loading...