RFR: 8217877: Dead code in jdk.jlink's TaskHelper

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8217877: Dead code in jdk.jlink's TaskHelper

Severin Gehwolf
Hi,

There seems to be dead code in class TaskHelper. Plugins are loaded via
ServiceLoader from the module boot loader. I don't see how this code
could ever be reached. The proposal is to remove it for clarity.

Bug: https://bugs.openjdk.java.net/browse/JDK-8217877
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8217877/webrev.01/

Testing: jdk/submit + test/jdk/tools/jlink and test/jdk/jdk/modules
tests on Linux x86_64

Thoughts?

Thanks,
Severin

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8217877: Dead code in jdk.jlink's TaskHelper

Alan Bateman
On 28/01/2019 15:00, Severin Gehwolf wrote:
> Hi,
>
> There seems to be dead code in class TaskHelper. Plugins are loaded via
> ServiceLoader from the module boot loader. I don't see how this code
> could ever be reached. The proposal is to remove it for clarity.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8217877
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8217877/webrev.01/
>
This code was used in JDK 9 for the experimental plugin interface, it
was disabled near the end of the release. So I think it's okay to remove
this code, meaning your patch looks okay, but it might of course come
back when that plugin interface is looked at again.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8217877: Dead code in jdk.jlink's TaskHelper

Mandy Chung


On 1/28/19 11:01 AM, Alan Bateman wrote:

> On 28/01/2019 15:00, Severin Gehwolf wrote:
>> Hi,
>>
>> There seems to be dead code in class TaskHelper. Plugins are loaded via
>> ServiceLoader from the module boot loader. I don't see how this code
>> could ever be reached. The proposal is to remove it for clarity.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8217877
>> webrev:
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8217877/webrev.01/
>>
> This code was used in JDK 9 for the experimental plugin interface, it
> was disabled near the end of the release. So I think it's okay to
> remove this code, meaning your patch looks okay, but it might of
> course come back when that plugin interface is looked at again.

Removing this unused code is okay to me.

Mandy
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8217877: Dead code in jdk.jlink's TaskHelper

Severin Gehwolf
On Mon, 2019-01-28 at 11:16 -0800, Mandy Chung wrote:

>
>
> On 1/28/19 11:01 AM, Alan Bateman wrote:
> > On 28/01/2019 15:00, Severin Gehwolf wrote:
> > > Hi,
> > >
> > > There seems to be dead code in class TaskHelper. Plugins are loaded via
> > > ServiceLoader from the module boot loader. I don't see how this code
> > > could ever be reached. The proposal is to remove it for clarity.
> > >
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8217877 
> > > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8217877/webrev.01/ 
> > >
> >  This code was used in JDK 9 for the experimental plugin interface,
> > it was disabled near the end of the release. So I think it's okay
> > to remove this code, meaning your patch looks okay, but it might of
> > course come back when that plugin interface is looked at again.
>  
> Removing this unused code is okay to me.

Thanks for the reviews, Alan, Mandy!

Cheers,
Severin