[7ux-osx] Please review: 7124089: launcher refactoring (original) (raw)
Anthony Petrov anthony.petrov at oracle.com
Tue Jan 17 05:21:56 PST 2012
- Previous message: [7ux-osx] Please review: 7124089: launcher refactoring
- Next message: [7ux-osx] Please review: 7124089: launcher refactoring
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi Kumar,
src/share/bin/java.c
987 } else if (IsMacOSX() && JLIStrCmp(arg, "-XstartOnFirstThread") == 0) { 988 ProcessSpecialArg(arg); 989 } else if (IsMacOSX() && JLIStrCCmp(arg, "-Xdock:") == 0) { 990 ProcessSpecialArg(arg);
1595 if (IsMacOSX()) {
If the initial goal was to get rid of mentioning a platform name in the java.c, then "if (IsMacOSX())" instead of "#ifdef MACOSX" doesn't make sense as well. Also, what justifies replacing the compile time checks with run-time checks in these two occurrences? I realize that in both cases the code itself is completely correct, but I don't see why it should be even present/executed in the launcher for platforms other than the Mac.
Generally, the fix looks good. Lots of #ifdef MACOSX looked very confusing before. However, I feel uncomfortable with having so much code duplicated between src/solaris/bin/java_md.c and src/macosx/bin/java_md.c. This seems to increase launcher code maintenance cost. Is there any possibility to fold the code up in a common source file that is shared between solaris/linux and macosx, and only define really specific parts of the code in separate files? The simplest way to accomplish this would be to leave exactly one #ifdef MACOSX in the shared file and #include a platform specific part there. Or we could tweak the make files to compile an additional file.
Also, the major part of the JVMInit() function is identical on all three platforms - solaris/linux, macosx, and windows. It makes sense to define a function containing that code in the java.c and call it by platform-specific JVMInit() implementations where needed.
-- best regards, Anthony
On 1/17/2012 7:33 AM, Kumar Srinivasan wrote:
oops missed setting the subject line to 7ux-osx.
Kumar
Hi,
Please review the launcher refactoring work, the webrev is here: http://cr.openjdk.java.net/~ksrini/7124089/webrev.0/ Notes: 1. Moves the majority of the launcher code into platform specific directories/files, removed redundant conditionals. 2. The solaris/linux code co-exists together as before. 3. On MacOSX, "java -client" will launch the server VM, for compatibility reasons, and dual mode is left in the macosx's javamd.c, this will enable experiments with universal libraries, quite easily. Also the mac ergonomics will always return server. Testing: Tested splash screen on all platforms and launcher regression tests. Thanks Kumar
- Previous message: [7ux-osx] Please review: 7124089: launcher refactoring
- Next message: [7ux-osx] Please review: 7124089: launcher refactoring
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]