On 11/1/13 9:18 AM, Tristan Yan wrote:
Hi Everyone
http://cr.openjdk.java.net/~pzhang/Tristan/7190106/webrev/
Description:
1. Convert shell script test to Java program test.
2. Using random server port by reusing Darryl Mocek's work to
replace fixed server port.
3. Using java Process class to start client process.
4. Also convert other shell script test runSerialBench.sh to java
program test also
Hi Tristan,
Several comments on this webrev.
** The original arrangement within the
test/java/rmi/reliability/benchmark
directory had the main benchmark files (scripts) at the top, some
benchmark framework files in the "bench" subdirectory, and the
actual RMI and serialization benchmarks in bench/rmi and bench/serial
subdirectories.
The webrev moves all the RMI benchmarks to the top benchmark
directory but leaves the serial benchmarks in bench/serial. The
RMI benchmarks are now all cluttering the top directory, but the
main serial benchmark test is now buried in the bench/serial
directory. The nice organization that was there before is now
spoiled. Is this rearrangement necessary in order to convert the scripts
to Java? I would prefer the original arrangement be left in place.
** The RMI benchmark Main.java file has a @run tag of the form,
@run main/othervm/policy=policy.all/timeout=1800 -server Main
-c config
There is a subtle but serious problem here: the -server option is
passed to the >>JVM<< and not as an argument to the main() method.
The main() method gets neither a -server nor a -client argument,
so its default "run mode" as defined by the benchmark itself is
SAMEVM. This runs the client and server in the same JVM, which is
different from the shell script, which ran separate client and server
JVMs.
** The logic to process the -server argument still expects to take
a port, even though the port is assigned automatically. So the
obvious fix to the above,
@run main/othervm/policy=policy.all/timeout=1800 Main -server
-c config
doesn't work, since a port is missing. The logic to increment the
argument index to collect the port argument should be removed.
Also, the -server line should be restored to the usage message, but
without the port argument.
** After this is done, the client's command line is constructed
improperly.
The command line ends up looking something like this:
java client -cp <classpath> Main client localhost:58583 -c
config
The word "client" appears twice, but what's really required is
"-client" to appear as an argument after Main.
** The client is run using ProcessBuilder, which by default sends
stdout and stderr to pipes to be read by the parent. But the
parent never reads them, thus any messages from the client are
never seen. The client is the one that emits the benchmark report,
so its output needs to be seen. It might be sufficient to have the
client inherit the parent's stdout and stderr. This might intermix
the client's and server's output, but it's better than nothing.
** The config file is checked with the following code:
try {
confFile = args[i];
confstr = new FileInputStream(System.getProperty("test.src")
+ System.getProperty("file.separator") + confFile);
} catch (IOException e) {
die("Error: unable to open \"" + args[i] + "\"");
}
This is potentially misleading, as the message doesn't print the
actual filename that was attempted to be opened.
This is important, as the test.src property doesn't exist in the client
JVM.
Note that the original shell script passed full pathnames for the
config file to both the client and the server. The new @run tag
merely says "-c config" which redefines the config filename to be
relative to the test.src directory. You could pass -Dtest.src=...
to the client, but it seems like there should be something better than
can be done.
** The client needs to have its security policy set up. This is
missing from the construction of the client's command line.
** ProcessBuilder takes a List<String> for its command; there is
no need to turn the list into an array.
** In the benchmark main methods, code of the form,
while (true) {
runBenchmarks();
if (exitRequested) {
System.exit();
}
}
was replaced with
while (!exitRequested) {
runBenchmarks();
}
This is a subtle logic change, in that the former code always
executed the loop at least once. It seems unlikely, but it's
possible that a timer could set exitRequested before loop entry,
resulting in the benchmark running zero times. I guess, if you
really want to clean this up (we do need to avoid System.exit in jtreg
tests), use a do-while loop instead.
** Don't forget to remove the 7190106/runRmiBench.sh entry from
ProblemList.txt.
** Remove the references to RMISecurityManager and just use
SecurityManager. This is just general cleanup. (I deprecated
RMISecurityManager last week.) :-)
It would be good if you could fix up these issues and post another webrev.
Thanks.
s'marks
Thank you
Tristan
On 01/11/2013 23:58, Stuart Marks wrote:
On 10/31/13 10:22 PM, Tristan Yan wrote:
I am working on bug
https://bugs.openjdk.java.net/browse/JDK-7190106. Based on my
research, it looks like the issue of fixed port was already
addressed by Stuart Marks in other RMI tests which are Java based. I
would like to reuse his solution, however it does not work for shell
based tests.
(Darryl Mocek did the unique port work for the RMI tests.)
Was the patch attached to your message? If so, it didn't get
through. Most OpenJDK mailing lists strip off attachments before
forwarding Hi Stuart Also there is one more solution, which is
there is one
jdk.testlibrary.Utils.getFreePort() method under test/lib. It's
same function as TestLibrary.getUnusedRandomPort() and it has named
package.
Do you mind I use this one?
Since these two function provide same functionality. Maybe we
should think about to merge them.
Thank you
Tristan
On 11/10/2013 11:19 AM, Tristan Yan wrote:
Hi Stuart
I tried your suggestion but unfortunately all the benchmarks
have dependencies to Main class because they need get stub from
server. I suggest we move the benchmark tests to unnamed
package unless we do want to put TestLibrary into a named package
right now.
Please let me know if you have objection on this.
Thank you
Tristan
On 11/09/2013 02:28 AM, Stuart Marks wrote:
Hi Tristan,
Yes, it's kind of a problem that the RMI TestLibrary is in the
unnamed package. Classes in a named package cannot import
classes from the unnamed package. We've run into problems with
this before. Eventually, we should move TestLibrary a named package.
I think it's possible to work around this without too much difficulty.
Note that classes in the unnamed package can import classes
from named packages. So, perhaps you can put the RmiBench main
class in the unnamed package so it has access to TestLibrary.
Then have the benchmarks themselves in the bench.rmi package.
The config file already references the benchmarks by fully
qualified class name (e.g.,
"bench.rmi.NullCalls") so with a bit of tinkering you ought to
be able to get this to work.
s'marks
On 11/8/13 3:00 AM, Tristan Yan wrote:
Thank you, Stuart
There is one review point I want to ask you opinion. Which is
the reason that I moved from
test/java/rmi/reliability/benchmark/bench/rmi
to test/java/rmi/reliability/benchmark is Main.java need
access class TestLibrary for supporting random port.
TestLibrary is a unpackage class, I couldn't find a way to
let a class which has Package to access the class without package.
Do you have suggestion on that?
Thank you so much.
Tristan
On 11/06/2013 09:50 AM, Stuart Marks wrote:
On 11/1/13 9:18 AM, Tristan Yan wrote:
Hi Everyone
http://cr.openjdk.java.net/~pzhang/Tristan/7190106/webrev/
Description:
1. Convert shell script test to Java program test.
2. Using random server port by reusing Darryl Mocek's work
to replace fixed server port.
3. Using java Process class to start client process.
4. Also convert other shell script test runSerialBench.sh
to java program test also
Hi Tristan,
Several comments on this webrev.
** The original arrangement within the
test/java/rmi/reliability/benchmark directory had the main
benchmark files (scripts) at the top, some benchmark
framework files in the "bench" subdirectory, and the actual
RMI and serialization benchmarks in bench/rmi and bench/serial
subdirectories.
The webrev moves all the RMI benchmarks to the top benchmark
directory but leaves the serial benchmarks in bench/serial.
The RMI benchmarks are now all cluttering the top directory,
but the main serial benchmark test is now buried in the
bench/serial directory.
The nice organization that was there before is now spoiled.
Is this rearrangement necessary in order to convert the
scripts to Java? I would prefer the original arrangement be left in
place.
** The RMI benchmark Main.java file has a @run tag of the
form,
@run main/othervm/policy=policy.all/timeout=1800 -server
Main -c config
There is a subtle but serious problem here: the -server
option is passed to the >>JVM<< and not as an argument to the
main() method.
The main() method gets neither a -server nor a -client
argument, so its default "run mode" as defined by the benchmark
itself is SAMEVM.
This runs the client and server in the same JVM, which is
different from the shell script, which ran separate client and
server JVMs.
** The logic to process the -server argument still expects
to take a port, even though the port is assigned
automatically. So the obvious fix to the above,
@run main/othervm/policy=policy.all/timeout=1800 Main
-server -c config
doesn't work, since a port is missing. The logic to
increment the argument index to collect the port argument
should be removed. Also, the -server line should be restored
to the usage message, but without the port argument.
** After this is done, the client's command line is
constructed improperly. The command line ends up looking something
like this:
java client -cp <classpath> Main client localhost:58583
-c config
The word "client" appears twice, but what's really required
is "-client" to appear as an argument after Main.
** The client is run using ProcessBuilder, which by default
sends stdout and stderr to pipes to be read by the parent.
But the parent never reads them, thus any messages from the client
are never seen.
The client is the one that emits the benchmark report, so
its output needs to be seen. It might be sufficient to have
the client inherit the parent's stdout and stderr. This
might intermix the client's and server's output, but it's better
than nothing.
** The config file is checked with the following code:
try {
confFile = args[i];
confstr = new
FileInputStream(System.getProperty("test.src")
+ System.getProperty("file.separator") + confFile);
} catch (IOException e) {
die("Error: unable to open \"" + args[i] + "\"");
}
This is potentially misleading, as the message doesn't print
the actual filename that was attempted to be opened.
This is important, as the test.src property doesn't exist in
the client JVM.
Note that the original shell script passed full pathnames
for the config file to both the client and the server. The
new @run tag merely says "-c config" which redefines the
config filename to be relative to the test.src directory.
You could pass -Dtest.src=... to the client, but it seems
like there should be something better than can be done.
** The client needs to have its security policy set up. This
is missing from the construction of the client's command line.
** ProcessBuilder takes a List<String> for its command;
there is no need to turn the list into an array.
** In the benchmark main methods, code of the form,
while (true) {
runBenchmarks();
if (exitRequested) {
System.exit();
}
}
was replaced with
while (!exitRequested) {
runBenchmarks();
}
This is a subtle logic change, in that the former code
always executed the loop at least once. It seems unlikely,
but it's possible that a timer could set exitRequested
before loop entry, resulting in the benchmark running zero
times. I guess, if you really want to clean this up (we do
need to avoid System.exit in jtreg tests), use a do-while loop
instead.
** Don't forget to remove the 7190106/runRmiBench.sh entry
from ProblemList.txt.
** Remove the references to RMISecurityManager and just use
SecurityManager. This is just general cleanup. (I deprecated
RMISecurityManager last week.) :-)
It would be good if you could fix up these issues and post
another webrev.
Thanks.
s'marks
Thank you
Tristan
On 01/11/2013 23:58, Stuart Marks wrote:
On 10/31/13 10:22 PM, Tristan Yan wrote:
I am working on bug
https://bugs.openjdk.java.net/browse/JDK-7190106. Based
on my research, it looks like the issue of fixed port was
already addressed by Stuart Marks in other RMI tests
which are Java based. I would like to reuse his solution,
however it does not work for shell based tests.
(Darryl Mocek did the unique port work for the RMI tests.)
Was the patch attached to your message? If so, it didn't
get through. Most OpenJDK mailing lists strip off
attachments before forwarding the message to the
recipients.
2. My recommendation would be to convert this shell
script test into Java based test and re-use the dynamic
port allocation solution by Stuart Marks to address the
issue
3. Also this test was written with server/client mode in
shell script. In the past there have been sync issues
between server/client which caused the test to fail. If
we convert the shell script into Java based test, it
would avoid using "sleep 10" mechanism to allow for
server and client to start up and also give us better
control in synchronizing server and client.
(Background for interested readers.) In general, yes, it's
quite difficult to make reliable shell tests, especially
for multi-process tests like this one.
There is the unique port issue, and there is also the
issue of how long for the client to wait until the server
is ready. Error handling is also a problem, for example,
if one of the JVMs gets an unexpected exception, it's easy
for shell tests to mishandle this case. They might hang or
erroneously report success.
--
If this is a rewrite, it's probably fairly large, so you
need to upload it somewhere (e.g., cr.openjdk.java.net)
and then post a link to it.
Thanks.
s'marks
the message to
the recipients.
2. My recommendation would be to convert this shell script test
into Java based test and re-use the dynamic port allocation
solution by Stuart Marks to address the issue
3. Also this test was written with server/client mode in shell script.
In the
past there have been sync issues between server/client which
caused the test to fail. If we convert the shell script into
Java based test, it would avoid using "sleep 10" mechanism to
allow for server and client to start up and also give us better
control in synchronizing server and client.
(Background for interested readers.) In general, yes, it's quite
difficult to make reliable shell tests, especially for
multi-process tests like this one.
There is the unique port issue, and there is also the issue of
how long for the client to wait until the server is ready. Error
handling is also a problem, for example, if one of the JVMs gets
an unexpected exception, it's easy for shell tests to mishandle
this case. They might hang or erroneously report success.
--
If this is a rewrite, it's probably fairly large, so you need to
upload it somewhere (e.g., cr.openjdk.java.net) and then post a link to
it.
Thanks.
s'marks