Re: RFR: JDK-8187498: Add a -Xmanagement flag as syntactic sugar for -Dcom.sun.management.jmxremote.* properties
You are going backwards. You need to have a CSR approved first. Kumar Ping. Could I please have reviews for below webrev. Thanks Harsha On Wednesday 14 February 2018 09:59 PM, Harsha Wardhana B wrote: Hi, Below is the updated webrev. http://cr.openjdk.java.net/~hb/8187498/webrev.03/ On Wednesday 14 February 2018 01:15 AM, mandy chung wrote: On 2/13/18 1:30 AM, Harsha Wardhana B wrote: Hi, Please find below the revised webrev. http://cr.openjdk.java.net/~hb/8187498/webrev.02/ On Friday 09 February 2018 05:07 AM, mandy chung wrote: On 2/7/18 1:19 AM, Harsha Wardhana B wrote: > > --start-management-agent will not be recognized in the current format and > hence will not default to --start-management-agent=local=true. `--start-local-management-server` is one option as Alan suggests. Another option is to make `--start-management-agent` to accept an optional argument. VM can accept `--start-management-agent` or `--start-management-agent=port=1234,ssl=on`. It may require more launcher change to support it. Yes. It requires lot more changes to launcher. Hence optional argument to --start-management-agent was not added in order to keep the launcher impact minimum. This is just one option for you to consider. So the current proposal of the new option to start a remote management server, right? Not necessarily. --start-management-agent=local=true starts local server and --start-management-agent=port=1234 starts remote management server. Agent.java If --start-management-agent is set, -Dcom.sun.management.* takes precedence. Do you really want to do that? The new VM option intends to simplify the command-line to type in. I think it's cleaner and reasonable if --start-management-agent is specified, -Dcom.sun.management.* are ignored (maybe worth emit a warning if set) We can probably document that -D options will be overridden instead of emitting a warning. What is the behavior when -Dcom.sun.management.jmxremote.port=1234 --start-management-agent port=2345 -Dcom.sun.management.jmxremote.port=3456? What is the value of the system property com.sun.management.jmxremote.port at runtime? What port number does the management server start with? As said earlier, values set via new flags override values set by -D flags. So 2345 will be the value of com.sun.management.jmxremote.port. Added a test case to validate that. The implementation uses VMManagement::getVmArguments and scan the VM options for -start-management-agent. The VM is the one invoking jdk.internal.agent.Agent::startAgent. A simpler option is to change Agent::startAgent to take an argument parameter that will be passed with the argument of --start-management-agent or null if not set. Similarly for startRemoteManagementAgent and startLocalAgent_name. Implementing this change requires lot of changes to argument parsing in native code and hence it is simpler to handle this at java layer. Can you describe how --start-management-agent option will be used for jcmd and any other tool that attaches to a running VM to start the agent? Example command-line will be useful. --start-management-agent cannot be used by jcmd (or dynamic attach) as it accepts flags only in -D format or -D flags with com.sun.management removed. That code to parse string passed via jcmd was a mistake and hence I have removed it. Mandy -Harsha
Re: RFR: JDK-8187498: Add a -Xmanagement flag as syntactic sugar for -Dcom.sun.management.jmxremote.* properties
Hi Harsha,
Changes look reasonable to me, couple of things that must be addressed:
1. Since this is a main-stream launcher change with a documented and
supported
option, a CSR is required, you have to add and document the option in
the help page
http://hg.openjdk.java.net/jdk/jdk/file/8cc67294ec56/src/java.base/share/classes/sun/launcher/resources/launcher.properties
2. You also have to create a doc bug so that the doc team will document
it in the Tools
Reference Guide, and link it to this bug. Does it need a Release note ?
probably does,
in which case you will have to create Release Note subtask and follow
the RN process.
3. Is XmanagementAgentTest.java part of tier1 test suite ? If not, then
I think it ought
to be in tier1 grouping, perhaps best to park this under
jdk/tools/launcher/management ?
Kumar
Hi All,
After internal discussions, below format for management flags was
agreed upon.
1. --start-management-agent port=1234,ssl=on(space seperator)
or
2. --start-management-agent=port=1234,ssl=on('=' seperator)
If option 1 is specified, it will be converted to option 2 by the java
launcher before it is passed onto VM.
With above GNU long format for management options, specifying
arguments is mandatory unlike before.
--start-management-agent will not be recognized in the current format
and hence will not default to --start-management-agent=local=true.
Below is the webrev with above changes and corresponding tests.
http://cr.openjdk.java.net/~hb/8187498/webrev.01/
Please review and comment.
Thanks
Harsha
On Monday 29 January 2018 03:14 PM, Harsha Wardhana B wrote:
Hi Alan,
I am not fully aware about Java launcher or how it passes options to
VM. Let me check with some other folks and get back to you.
Thanks
Harsha
On Monday 29 January 2018 01:55 PM, Alan Bateman wrote:
On 29/01/2018 05:20, Harsha Wardhana B wrote:
Hi Mandy,Alan,
Thanks for your inputs.
If I keep it as launcher option, it may need to know JMX agent
flags which may need to be extended in future.
I would prefer making it a VM option. I will make the required
changes and send out an updated webrev.
I think Mandy's suggestion is to just transform --management
so a form that the VM can read. The launcher will need to
replace the space anyway as the VM only accepts "=".
-Alan
Re: RFR: 8189102: All tools should support -?, -h and --help
Hi,
Besides my private comments to you, there are 2-3 internal tests
which fail.
Have you run all the langtools tests ? There are 4 Windows tests
that fail with langtools:
jdk/javadoc/doclet/testHelpOption/TestHelpOption.java
jdk/javadoc/tool/CheckResourceKeys.java
jdk/javadoc/tool/ToolProviderTest.java
tools/javap/InvalidOptions.java
tools/jdeps/MultiReleaseJar.java
This changeset needs to be vetted thoroughly using internal build and
test systems.
Any Oracle engineer willing to sponsor this ?
Kumar
On 11/28/2017 3:16 AM, Lindenmaier, Goetz wrote:
Hi,
I uploaded a new webrev:
http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.04/
This includes the changes
- to jshell requested by Robert
- to the test as requested by Kumar.
See also incremental patch and the test output including all the
help messages referenced in the webrev.
Best regards,
Goetz.
-Original Message-
From: Kumar Srinivasan [mailto:kumar.x.sriniva...@oracle.com]
Sent: Montag, 27. November 2017 17:43
To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>
Cc: core-libs-...@openjdk.java.net; 'compiler-...@openjdk.java.net'
<compiler-...@openjdk.java.net>; serviceability-dev (serviceability-
d...@openjdk.java.net) <serviceability-dev@openjdk.java.net>
Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi,
I looked at some of the components I maintain, and they look good.
Wrt. the test:
1. The local variables/fields don't comply with the coding conventions,
we have been
following in the JDK.
2. Hmm, someone parked a .ini file in bin directory, later removed,
perhaps on windows simply check for .exe ? Should a check exist
to verify if file has executable permissions ?"File.canExecute".
171 // Returns true if the file is not a tool.
172 static boolean notATool(String file) {
173 file = file.toLowerCase();
174 if (file.endsWith(".dll") ||
175 file.endsWith(".map") ||
176 file.endsWith(".pdb") ||
177 file.equals("server")) { // server subdir on windows.
178 return true;
179 }
180 return false;
181 }
3. Typo in comment
201 // Check whether 'flag' appears in 'line' as a word of itself. I must
not
s/I must/It must/
4. There is a method to check for isEnglishLocale in TestHelper, perhaps
use it
to have the test bale out in non english locales as early as possible ?
5. Has this been tested on all platforms ? do you need help testing it ?
Kumar
On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this:
http://cr.openjdk.java.net/~goetz/wr17/8189102-
helpMessage/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8189102
CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to
give a more complete overview of currently supported flags
for the CSR:
http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-
October/028615.html
Best regards,
Goetz.
Re: RFR: 8189102: All tools should support -?, -h and --help
Hi,
I looked at some of the components I maintain, and they look good.
Wrt. the test:
1. The local variables/fields don't comply with the coding conventions,
we have been
following in the JDK.
2. Hmm, someone parked a .ini file in bin directory, later removed,
perhaps on windows simply check for .exe ? Should a check exist
to verify if file has executable permissions ?"File.canExecute".
171 // Returns true if the file is not a tool.
172 static boolean notATool(String file) {
173 file = file.toLowerCase();
174 if (file.endsWith(".dll") ||
175 file.endsWith(".map") ||
176 file.endsWith(".pdb") ||
177 file.equals("server")) { // server subdir on windows.
178 return true;
179 }
180 return false;
181 }
3. Typo in comment
201 // Check whether 'flag' appears in 'line' as a word of itself. I must
not
s/I must/It must/
4. There is a method to check for isEnglishLocale in TestHelper, perhaps
use it
to have the test bale out in non english locales as early as possible ?
5. Has this been tested on all platforms ? do you need help testing it ?
Kumar
On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this:
http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8189102
CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to
give a more complete overview of currently supported flags
for the CSR:
http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-October/028615.html
Best regards,
Goetz.
Re: RFR: JDK-8179631: Fix Html5 errors in java.management, jdk.management, jdk.jdi and jdk.attach
Hello, Ping!. Lets wrap this up soon, as we have to move to the next steps in the docs related work. Thanks Kumar Hi All, Please review fixes to make the API doc comments HTML5 clean, there are no changes to the verbiage, and mostly fixes for the table styles defined here: http://hg.openjdk.java.net/jdk9/dev/langtools/rev/ee84b7d44339 For tables with many entries I have used "striped", there are few tables used for formatting purposes, for which "borderless" is used, and "plain" for everything else. Thanks Kumar Webrev: http://cr.openjdk.java.net/~ksrini/8179631/webrev.0/ JBS: https://bugs.openjdk.java.net/browse/JDK-8179631
RFR: JDK-8179631: Fix Html5 errors in java.management, jdk.management, jdk.jdi and jdk.attach
Hi All, Please review fixes to make the API doc comments HTML5 clean, there are no changes to the verbiage, and mostly fixes for the table styles defined here: http://hg.openjdk.java.net/jdk9/dev/langtools/rev/ee84b7d44339 For tables with many entries I have used "striped", there are few tables used for formatting purposes, for which "borderless" is used, and "plain" for everything else. Thanks Kumar Webrev: http://cr.openjdk.java.net/~ksrini/8179631/webrev.0/ JBS: https://bugs.openjdk.java.net/browse/JDK-8179631
RFR: JDK-8179538: Update jdk.jdi to be HTML-5 friendly
Hello,
Please review changes to make jdk.jdi HTML5 friendly,
table cell padding has not been addressed and will be fixed
separately, this takes care of others.
Note: Some of the errors were due to incorrect@throws but
in all cases there is the correct one further down, please use
sdiffs in these cases.
@throws {@link InvalidTypeException} if any
before:jdk.jdi {error=42, warning=1}
after: jdk.jdi {error=1}
http://cr.openjdk.java.net/~ksrini/8179538/webrev.00/index.html
https://bugs.openjdk.java.net/browse/JDK-8179538
Thanks
Kumar
RFR: 8179415: Update java.management and java.management.rmi to be HTML-5 friendly
Hello, Please review changes for java.management and java.management.rmi to be HTML5 ready, there are outliers like cellpadding, cellspacing that needs to be done separately, note this was *not* done mechanically by a script. http://cr.openjdk.java.net/~ksrini/8179415/ https://bugs.openjdk.java.net/browse/JDK-8179415 Thanks Kumar
Re: [9] RFR (S) 6762191: Setting stack size to 16K causes segmentation fault
Hi Chris, Approved with some minor nits, typos which needs correction. yes java.c follows the JDK indenting as Alan pointed out. TooSmallStackSize.java Copyright should be 2014, 1. 37 * stack size for the platform (as provided by the JVM error message when a very 38 * small is used), and then verify that the JVM can be launched with that stack s/small is/small stack is/ 2. 54 * Returns the minimum stack size this platform will allowed based on the s/allowed/allow/ 3. 58 * The TestResult argument must containthe result of having already run s/containthe/contain the/ 4. 92 if (verbose) System.out.println(*** Testing + stackSize); I know this is allowed in the HotSpot world but in JDK land we always use with a LF + Indent, also in other places. 5. 85 * Returns the mimumum allowed stack size gleaned from the error message, s/mimumum/minimum Finally I am concerned with the integration, since it spans both HotSpot and JDK, and it appears the test will fail if the HotSpot changes are not integrated first, or has it already ? Thanks Kumar On 12/1/2014 6:39 PM, Chris Plummer wrote: Sorry about the long delay in getting back to this. I ran into two separate JPRT issues that were preventing me from testing these changes, plus I was on vacation last week. Here's an updated webrev. I'm not sure where we left things, so I'll just say what's changed since the original version: 1. Rewrote the test to be in Java instead of a shell script. 2. Moved the test from hotspot/test/runtime/memory to jdk/test/tools/launcher 3. Added STACK_SIZE_MINIMUM to java.c, allowing a makefile to override the default 32k minimum value. https://bugs.openjdk.java.net/browse/JDK-6762191 http://cr.openjdk.java.net/~cjplummer/6762191/webrev.02/ thanks, Chris On 11/19/14 7:52 AM, Chris Plummer wrote: On 11/19/14 2:12 AM, David Holmes wrote: On 19/11/2014 6:49 PM, Chris Plummer wrote: I've update the webrev to add STACK_SIZE_MINIMUM in place of the 32k references, and also moved the test from hotspot/test/runtime to jdk/test/tools/launcher as David requested. That required some adjustments to the test script, since test_env.sh does not exist in jdk/test, so I had to pull in the bits I needed into the script. Is there a reason this needs a shell script instead of using the testlibrary tools to launch the VM and check the output? Not that I'm aware of. I guess I just really didn't look at what it would take to make it all in java. I'll have a look at java examples and convert it. Chris Sorry that should have been mentioned much earlier. David http://cr.openjdk.java.net/~cjplummer/6762191/webrev.01/ I still need to rerun through JPRT. I'll do so once there are no more suggested changes. thanks, Chris On 11/18/14 2:08 PM, Chris Plummer wrote: Adding core-libs-...@openjdk.java.net, since one of the changes is in java.c. Chris On 11/12/14 6:43 PM, David Holmes wrote: Hi Chris, Sorry for the delay. On 13/11/2014 5:44 AM, Chris Plummer wrote: Hi, I'm still looking for reviewers. As the change is to the launcher it needs to be reviewed by the launcher owner - which I think is serviceability (though also cc'd Kumar :) ). Launcher change, and your rationale, seems okay to me. I'd probably put the test in to jdk/test/tools/launcher/ though. Thanks, David thanks, Chris On 11/7/14 7:53 PM, Chris Plummer wrote: This is an initial review for 6762191. I'm guessing there will be recommendations to fix in a different way, but thought this would be a good time to start the discussion. https://bugs.openjdk.java.net/browse/JDK-6762191 http://cr.openjdk.java.net/~cjplummer/6762191/webrev.00.jdk/ http://cr.openjdk.java.net/~cjplummer/6762191/webrev.00.hotspot/ The bug is that if the -Xss size is set to something very small (like 16k), on linux there will be a crash due to overwriting the end of the stack. This happens before hotspot can compute its stack needs and verify that the stack is big enough. It didn't seem viable to move the hotspot stack size check earlier. It depends on too much other work done before that point, and the changes would have been disruptive. The stack size check is currently done in os::init_2(). What is needed is a check before the thread is created. That way we can create a thread with a big enough stack to handle all needs up to the point of the check in os::init_2(). This initial check does not need to be the final check. It just needs to confirm that we have enough stack to get us to the check in os::init_2(). I decided to check in java.c if the -Xss size is too small, and set it to a larger size if it is. I hard coded this size to 32k (I'll explain why 32k later). I suspect this is the part that will result in some debate. If you have better suggestions let me know. If it does stay here, then probably the 32k needs to be a #define, and maybe even an OS porting interface, but I'm not sure where to put it. The
Re: [9] RFR (S) 6762191: Setting stack size to 16K causes segmentation fault
On 12/3/2014 11:26 AM, Chris Plummer wrote: Hi Kumar, On 12/3/14 10:58 AM, Kumar Srinivasan wrote: Hi Chris, Approved with some minor nits, typos which needs correction. yes java.c follows the JDK indenting as Alan pointed out. TooSmallStackSize.java Copyright should be 2014, Copy/paste error from example test I was referred to. I will fix, and also remove the import if not needed. 1. 37 * stack size for the platform (as provided by the JVM error message when a very 38 * small is used), and then verify that the JVM can be launched with that stack s/small is/small stack is/ ok 2. 54 * Returns the minimum stack size this platform will allowed based on the s/allowed/allow/ ok 3. 58 * The TestResult argument must containthe result of having already run s/containthe/contain the/ ok 4. 92 if (verbose) System.out.println(*** Testing + stackSize); I know this is allowed in the HotSpot world but in JDK land we always use with a LF + Indent, also in other places. Are curly braces needed then? I know some coding conventions require them. No not necessary for one liners. 5. 85 * Returns the mimumum allowed stack size gleaned from the error message, s/mimumum/minimum ok. Finally I am concerned with the integration, since it spans both HotSpot and JDK, and it appears the test will fail if the HotSpot changes are not integrated first, or has it already ? There are no hotspot changes. java.c is where the fix is. Great!. Thanks Kumar thanks, Chris Thanks Kumar On 12/1/2014 6:39 PM, Chris Plummer wrote: Sorry about the long delay in getting back to this. I ran into two separate JPRT issues that were preventing me from testing these changes, plus I was on vacation last week. Here's an updated webrev. I'm not sure where we left things, so I'll just say what's changed since the original version: 1. Rewrote the test to be in Java instead of a shell script. 2. Moved the test from hotspot/test/runtime/memory to jdk/test/tools/launcher 3. Added STACK_SIZE_MINIMUM to java.c, allowing a makefile to override the default 32k minimum value. https://bugs.openjdk.java.net/browse/JDK-6762191 http://cr.openjdk.java.net/~cjplummer/6762191/webrev.02/ thanks, Chris On 11/19/14 7:52 AM, Chris Plummer wrote: On 11/19/14 2:12 AM, David Holmes wrote: On 19/11/2014 6:49 PM, Chris Plummer wrote: I've update the webrev to add STACK_SIZE_MINIMUM in place of the 32k references, and also moved the test from hotspot/test/runtime to jdk/test/tools/launcher as David requested. That required some adjustments to the test script, since test_env.sh does not exist in jdk/test, so I had to pull in the bits I needed into the script. Is there a reason this needs a shell script instead of using the testlibrary tools to launch the VM and check the output? Not that I'm aware of. I guess I just really didn't look at what it would take to make it all in java. I'll have a look at java examples and convert it. Chris Sorry that should have been mentioned much earlier. David http://cr.openjdk.java.net/~cjplummer/6762191/webrev.01/ I still need to rerun through JPRT. I'll do so once there are no more suggested changes. thanks, Chris On 11/18/14 2:08 PM, Chris Plummer wrote: Adding core-libs-...@openjdk.java.net, since one of the changes is in java.c. Chris On 11/12/14 6:43 PM, David Holmes wrote: Hi Chris, Sorry for the delay. On 13/11/2014 5:44 AM, Chris Plummer wrote: Hi, I'm still looking for reviewers. As the change is to the launcher it needs to be reviewed by the launcher owner - which I think is serviceability (though also cc'd Kumar :) ). Launcher change, and your rationale, seems okay to me. I'd probably put the test in to jdk/test/tools/launcher/ though. Thanks, David thanks, Chris On 11/7/14 7:53 PM, Chris Plummer wrote: This is an initial review for 6762191. I'm guessing there will be recommendations to fix in a different way, but thought this would be a good time to start the discussion. https://bugs.openjdk.java.net/browse/JDK-6762191 http://cr.openjdk.java.net/~cjplummer/6762191/webrev.00.jdk/ http://cr.openjdk.java.net/~cjplummer/6762191/webrev.00.hotspot/ The bug is that if the -Xss size is set to something very small (like 16k), on linux there will be a crash due to overwriting the end of the stack. This happens before hotspot can compute its stack needs and verify that the stack is big enough. It didn't seem viable to move the hotspot stack size check earlier. It depends on too much other work done before that point, and the changes would have been disruptive. The stack size check is currently done in os::init_2(). What is needed is a check before the thread is created. That way we can create a thread with a big enough stack to handle all needs up to the point of the check in os::init_2(). This initial check does not need to be the final check. It just needs to confirm that we have enough
Re: RFR: Changes to disable and/or remove Solaris 32-bit from JDK8
Here are the updated changes:
The build changes are here:
http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk8.1/
the delta changes since last reviewed:
http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk8.1/webrev.delta/index.html
The jdk changes are here:
http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.2/
The delta changes since last reviewed:
http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.2/webrev.delta/index.html
Thanks
Kumar
In SunCommandLineLauncher.java:
198 if (home.length() 0) {
199 String os_arch = System.getProperty(os.arch);
200 if (SunOS.equals(System.getProperty(os.name))) {
201 exePath = home + File.separator + bin +
File.separator + exe;
202 }
203 } else {
204 exePath = exe;
205 }
I think this should be:
198 if (home.length() 0) {
201 exePath = home + File.separator + bin + File.separator
+ exe;
203 } else {
204 exePath = exe;
205 }
Thanks,
/Staffan
On 9 sep 2013, at 18:12, Kumar Srinivasan kumar.x.sriniva...@oracle.com wrote:
Hi David,
Hi Kumar,
This is still dead code in
src/share/classes/com/sun/tools/jdi/SunCommandLineLauncher.java
String os_arch = System.getProperty(os.arch);
Ah, I will take care of it. Thanks for spotting this.
Also:
test/java/nio/channels/spi/SelectorProvider/inheritedChannel/lib/solaris-amd64/libLauncher.so
I know this already exist but I thought binaries were disallowed in the open
repo?
Alan, are the nio changes acceptable? Let me know if you need more time to go
over all
the changes.
Kumar
Davud
On 9/09/2013 1:09 PM, Kumar Srinivasan wrote:
Hi David, Staffan, Alan,
I have addressed all the issues pointed and some more I found while jprt
testing.
The updated webrev for jdk is here:
http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.1/
and the delta webrev since the last review webrev is here:
http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.1/webrev.delta/index.html
Thanks
Kumar
Hi Kumar,
A few minor comments ...
src/share/classes/com/sun/tools/jdi/SunCommandLineLauncher.java
Seems to me this is all dead now:
199 /*
200 * A wrinkle in the environment:
201 * 64-bit executables are stored under
$JAVA_HOME/bin/os_arch
202 * 32-bit executables are stored under
$JAVA_HOME/bin
203 */
204 String os_arch = System.getProperty(os.arch);
os_arch is no longer used and the comment no longer applicable.
---
src/solaris/bin/java_md_solinux.c
This seems to force DUAL_MODE off regardless of what the user may set
it to:
#ifdef __solaris__
! # ifdef DUAL_MODE
! #undef DUAL_MODE
! # endif
why doesn't it just not define DUAL_MODE?
---
test/demo/jvmti/DemoRun.java
test/sun/tools/jhat/HatRun.java
It isn't clear to me why you need to retain the d64 variable at all.
---
test/tools/launcher/ExecutionEnvironment.java
typo: appopriate
Thanks,
David
On 7/09/2013 2:47 AM, Kumar Srinivasan wrote:
Hello,
Please review the changes to remove Solaris 32-bit binaries from JDK8
distros,
at this time the dual mode support in the launcher is being disabled.
Message regarding this:
http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-September/003159.html
The jdk changes are here:
http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.0/
The top forest changes are here:
http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk8.0/
Thanks
Kumar
Re: RFR: Changes to disable and/or remove Solaris 32-bit from JDK8
Hi David, Hi Kumar, This is still dead code in src/share/classes/com/sun/tools/jdi/SunCommandLineLauncher.java String os_arch = System.getProperty(os.arch); Ah, I will take care of it. Thanks for spotting this. Also: test/java/nio/channels/spi/SelectorProvider/inheritedChannel/lib/solaris-amd64/libLauncher.so I know this already exist but I thought binaries were disallowed in the open repo? Alan, are the nio changes acceptable? Let me know if you need more time to go over all the changes. Kumar Davud On 9/09/2013 1:09 PM, Kumar Srinivasan wrote: Hi David, Staffan, Alan, I have addressed all the issues pointed and some more I found while jprt testing. The updated webrev for jdk is here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.1/ and the delta webrev since the last review webrev is here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.1/webrev.delta/index.html Thanks Kumar Hi Kumar, A few minor comments ... src/share/classes/com/sun/tools/jdi/SunCommandLineLauncher.java Seems to me this is all dead now: 199 /* 200 * A wrinkle in the environment: 201 * 64-bit executables are stored under $JAVA_HOME/bin/os_arch 202 * 32-bit executables are stored under $JAVA_HOME/bin 203 */ 204 String os_arch = System.getProperty(os.arch); os_arch is no longer used and the comment no longer applicable. --- src/solaris/bin/java_md_solinux.c This seems to force DUAL_MODE off regardless of what the user may set it to: #ifdef __solaris__ ! # ifdef DUAL_MODE ! #undef DUAL_MODE ! # endif why doesn't it just not define DUAL_MODE? --- test/demo/jvmti/DemoRun.java test/sun/tools/jhat/HatRun.java It isn't clear to me why you need to retain the d64 variable at all. --- test/tools/launcher/ExecutionEnvironment.java typo: appopriate Thanks, David On 7/09/2013 2:47 AM, Kumar Srinivasan wrote: Hello, Please review the changes to remove Solaris 32-bit binaries from JDK8 distros, at this time the dual mode support in the launcher is being disabled. Message regarding this: http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-September/003159.html The jdk changes are here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.0/ The top forest changes are here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk8.0/ Thanks Kumar
Re: RFR: Changes to disable and/or remove Solaris 32-bit from JDK8
Hi David, Staffan, Alan, I have addressed all the issues pointed and some more I found while jprt testing. The updated webrev for jdk is here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.1/ and the delta webrev since the last review webrev is here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.1/webrev.delta/index.html Thanks Kumar Hi Kumar, A few minor comments ... src/share/classes/com/sun/tools/jdi/SunCommandLineLauncher.java Seems to me this is all dead now: 199 /* 200 * A wrinkle in the environment: 201 * 64-bit executables are stored under $JAVA_HOME/bin/os_arch 202 * 32-bit executables are stored under $JAVA_HOME/bin 203 */ 204 String os_arch = System.getProperty(os.arch); os_arch is no longer used and the comment no longer applicable. --- src/solaris/bin/java_md_solinux.c This seems to force DUAL_MODE off regardless of what the user may set it to: #ifdef __solaris__ ! # ifdef DUAL_MODE ! #undef DUAL_MODE ! # endif why doesn't it just not define DUAL_MODE? --- test/demo/jvmti/DemoRun.java test/sun/tools/jhat/HatRun.java It isn't clear to me why you need to retain the d64 variable at all. --- test/tools/launcher/ExecutionEnvironment.java typo: appopriate Thanks, David On 7/09/2013 2:47 AM, Kumar Srinivasan wrote: Hello, Please review the changes to remove Solaris 32-bit binaries from JDK8 distros, at this time the dual mode support in the launcher is being disabled. Message regarding this: http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-September/003159.html The jdk changes are here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.0/ The top forest changes are here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk8.0/ Thanks Kumar
Re: RFR: Changes to disable and/or remove Solaris 32-bit from JDK8
On 9/6/2013 12:21 PM, Alan Bateman wrote: On 06/09/2013 17:47, Kumar Srinivasan wrote: Hello, Please review the changes to remove Solaris 32-bit binaries from JDK8 distros, at this time the dual mode support in the launcher is being disabled. Message regarding this: http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-September/003159.html The jdk changes are here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.0/ The top forest changes are here: http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk8.0/ I haven't studied the changes yet but I see you've updated test/java/nio/channels/spi/SelectorProvider/inheritedChannel/run_tests.sh. I don't think you need the changes at L42-48, instead you can just hg rm the 32-bit libraries that are in test/java/nio/channels/spi/SelectorProvider/inheritedChannel/lib. Will do, I was wondering about those libraries. You might want to bring the changes to serviceability-dev because of the change to the JDI launching connector and the JDI tests. cc'ed. Thanks Kumar -Alan.
Re: Fwd: large jar file failed
IMHO this looks like a valid use case that should be solved in some upcoming release, on all relevant target platforms (Windows/non-Windows, 32/64-bit). Is there a bug created yet? We need a bug/CR on this, I will look into fixing this for the next available update release. Kumar We can look to the JVM/JDK for info on how native large file support is implemented, e.g. we might need separate implementations for Windows and Linux/Solaris since O_LARGEFILE is not supported on Windows. /Robert -Original Message- From: David Holmes Sent: den 10 oktober 2011 08:27 To: wynne.wang Cc: serviceability-dev@openjdk.java.net Subject: Re: Fwd: large jar file failed FYI my initial response to the openjdk lists have been held for moderator approval due to my use of bcc :( On 10/10/2011 4:17 PM, wynne.wang wrote: Thanks for your kind reply of the issue. Yes, it seems more like a launcher issue. And if we use 32bit jvm with -d64 option (it will auto launch the 64bit jvm) it appears an Open() error of EOVERFLOW. And if we use 64bit jvm directly , it appears to be an Lseek() error of EINVAL. I doubt if there is any workaround such as a jvm option. If the issue is with the launcher then I think the only option would be to try and use a custom launcher that uses 64-bit file operations. David Thanks Wynne Wang δΊ 2011/10/10 11:22, David Holmes ει: I've bcc'ed the hotspot lists and redirected this to serviceability as this is a launcher issue not a JVM issue. On 9/10/2011 8:30 PM, wynne.wang wrote: Hi Experts I need help to solve a JDK issue. It is easy to repeat, one created a jar file 2GB , and it failed to run. By the deeper investigation, I found the error was thrown from the SelectVersion() method in JVM . What was the error? I suspect it may have been EOVERFLOW as the jar file was not opened with O_LARGEFILE set. David Holmes Inside the openjdk source code(/share/bin/java.c), I found some interesting notes as: * A NOTE TO DEVELOPERS: For performance reasons it is important that * the program image remain relatively small until after SelectVersion * CreateExecutionEnvironment have finished their possibly recursive * processing. Watch everything, but resist all temptations to use Java * interfaces. OK, the fact is , now there is a jar file2GB , and if it could pass the SelectVersion() check in JVM? Or any advice ? Thanks Wynne
hg: jdk7/tl/jdk: 6367077: Purge LD_LIBRARY_PATH usage from the launcher; ...
Changeset: de45eac5670e Author:ksrini Date: 2009-11-20 11:01 -0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/de45eac5670e 6367077: Purge LD_LIBRARY_PATH usage from the launcher 6899834: (launcher) remove the solaris libjvm.so symlink Summary: Fixes other related issues as well. Reviewed-by: darcy, ohair, xlu, martin ! make/java/jli/Makefile ! make/java/main/java/Makefile ! make/java/redist/Makefile ! src/share/bin/java.c ! src/solaris/bin/java_md.c ! test/tools/launcher/Arrrghs.java + test/tools/launcher/ExecutionEnvironment.java - test/tools/launcher/SolarisDataModel.sh - test/tools/launcher/SolarisRunpath.sh ! test/tools/launcher/TestHelper.java - test/tools/launcher/libraryCaller.c - test/tools/launcher/libraryCaller.h - test/tools/launcher/libraryCaller.java
hg: jdk7/tl/jdk: 6685121: (launcher) make ReportErrorMessages accessible by other launcher subsystems
Changeset: 2c65a59dd48d Author:ksrini Date: 2008-08-26 10:21 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/2c65a59dd48d 6685121: (launcher) make ReportErrorMessages accessible by other launcher subsystems Summary: provided error reporting interfaces to other java subsystems that the launcher uses. Reviewed-by: darcy ! make/java/jli/Makefile ! make/java/jli/mapfile-vers ! src/share/bin/emessages.h ! src/share/bin/java.c ! src/share/bin/java.h ! src/solaris/bin/java_md.c ! src/windows/bin/java_md.c
hg: jdk7/tl/langtools: 6618930: (javac) fix test after whitespace normalization
Changeset: 058bdd3ca02e Author:ksrini Date: 2008-03-20 08:44 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/langtools/rev/058bdd3ca02e 6618930: (javac) fix test after whitespace normalization Summary: whitespace normalization left the test unusable, back to service Reviewed-by: jjg ! test/tools/javac/6304921/T6304921.java ! test/tools/javac/6304921/T6304921.out
