Great!
Thanks,
David
On 5/04/2019 2:00 pm, Daniil Titov wrote:
Hi David,
I figured out what went wrong. I used webrev.ksh with the list of files to diff. Using
"webrev.ksh -m -N" solved the problem. The new webrev is uploaded as
webrev.04.
Webrev: http://cr.openjdk.java.net/~dtitov/8221730/webrev.04
Bug: https://bugs.openjdk.java.net/browse/JDK-8221730
Thanks,
Daniil
On 4/4/19, 7:15 PM, "David Holmes" <[email protected]> wrote:
Hi Daniil,
On 5/04/2019 12:01 pm, Daniil Titov wrote:
> Hi David,
>
> I didn't use "hg rename". Now I recreated the patch using "hg rename"
for moving test/hotspot/jtreg/serviceability/dcmd/framework/TestJavaProcess.java to
test/hotspot/jtreg/serviceability/dcmd/framework/process/TestJavaProcess.java. I also used the latest
version of webrev.ksh (25.17) to re-create a webrev (I uploaded it as webrev.03). However, I still
don't see that renaming is somehow reflected in the webrev. Not sure what might be wrong here. The hg
version is 4.5.3, the OS is Ubuntu 18.04.2 LTS.
Strange. Here's an example of where the rename shows up in the webrev:
http://cr.openjdk.java.net/~dholmes/8213233/webrev/
Anyway as long as it was done with hg rename that is fine.
Thanks,
David
----
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8221730/webrev.03/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8221730
>
> Thanks!
>
> Best regards,
> Daniil
>
> On 4/4/19, 5:45 PM, "David Holmes" <[email protected]> wrote:
>
> Hi Daniil,
>
> On 5/04/2019 8:17 am, Daniil Titov wrote:
> > Hi David and JC,
> >
> > Thank you for reviewing this change. Please review a new version
of
> > the fix that adds additional tests as David suggested. The tests
are
> > added to
> >
test/hotspot/jtreg/serviceability/dcmd/framework/VMVersionTest.java and
> > test the cases when Java process is started with -jar or -m
(--module)
> > options.
>
> Looks good - thanks for doing that.
>
> > Since unnamed packages are not allowed in the modules, file
> >
test/hotspot/jtreg/serviceability/dcmd/framework/TestJavaProcess.java
> > was moved to
> >
test/hotspot/jtreg/serviceability/dcmd/framework/process/TestJavaProcess.java.
>
> Did you use "hg rename" for that? The webrev doesn't show that you
did.
>
> Thanks,
> David
>
> >
> > Webrev: http://cr.openjdk.java.net/~dtitov/8221730/webrev.02/
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8221730
> >
> > Thanks!
> > --Daniil
> >
> >
> > On 4/3/19, 5:23 PM, "David Holmes" <[email protected]>
wrote:
> >
> > Hi Daniil,
> >
> > This seems reasonable, but can we add a suitable regression
test please
> > to verify behaviour before JDK-8205654 and with this fix.
> >
> > Thanks,
> > David
> >
> > On 4/04/2019 5:02 am, Daniil Titov wrote:
> > > Please review the change that makes jcmd process name
matching on Linux platform consistent with pre-existing behavior.
> > >
> > > On other platforms (and on Linux platform before changes [3]) the jcmd uses
system property "sun.rt.javaCommand" (see sun.jvmstat.monitor.
MonitoredVmUtil.mainClass(MonitoredVm, boolean) that is called from sun.tools.common. ProcessArgumentMatcher
at line 96) and treats the part before the first space as a main class when matching the process name.
However, if the application is started with -jar option this part contains the path to the jar file. If -m
or --module option is used this part contains the module name and the main class (if the main class was
specified in the command line) in the format <modulename>/<mainclass>. After changes [3] , on
Linux platform the proc filesystem is used to find a Java process, however, it always matches the process name
against the main class regardless what options were used to launch the application. This created discrepancies
between old and new behavior on Linux platform as well as between behavior on Linux and other platforms. Th!
> > > e fix changes sun.tool.ProcessHelper (that was
introduced in [3]) to correct these discrepancies.
> > >
> > >
> > > Reference:
> > > [1] Webrev:
http://cr.openjdk.java.net/~dtitov/8221730/webrev.01
> > > [2] Bug: https://bugs.openjdk.java.net/browse/JDK-8221730
> > > [3] https://bugs.openjdk.java.net/browse/JDK-8205654
> > >
> > >
> > > Thanks!
> > > --Daniil
> > >
> > >
> >
> >
> >
>
>
>
