[7ux-osx] Please review: 7124089: launcher refactoring

[Anthony Petrov]

On 1/17/2012 8:35 PM, Kumar Srinivasan wrote:
>> src/share/bin/java.c
>>>  987         } else if (IsMacOSX() && JLI_StrCmp(arg, 
>>> "-XstartOnFirstThread") == 0) {
>>>  988             ProcessSpecialArg(arg);
>>>  989         } else if (IsMacOSX() && JLI_StrCCmp(arg, "-Xdock:") == 
>>> 0) {
>>>  990             ProcessSpecialArg(arg);
> If we don't have those runtime checks then Windows,
> Linux and Solaris will accept those flags without a fuss, so what should 
> the behavior be ?

I just thought that instead of the run-time conditional this could 
somehow be folded into a compile-type kind of check. This could be an 
#ifdef, or at the end of this chained if{}else{} block we could have 
something like:

} else if (ProcessPlatformArg(arg)) {
} else if (RemovableOption...

and then have the ProcessPlatformArg() be defined in java_md.c.


> Was a specification (ccc) filed for these flags ? Will this be 
> documented in the help
> section ? though X flags are unsupported, in the past we have documented 
> them, both
> in the man pages as well as "java -X", if these will be left 
> undocumented should we take
> the vm approach and mark it -XX ?

These flags are commonly recognized since Apple has introduced them ages 
ago, so developers expect these flags to work. I think it makes sense to 
document their usage.


>> 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.
> Yes it is a lot more readable, and yes it increases duplication , but we
> already have some duplication between  windows and unix. Since
> this is platform specific code, and 7uX will go into maintenance mode,
> the exposure will be limited.

Ah, I didn't realize that this fix isn't going to be ported to JDK 8. In 
this case I guess I'm fine with this. But in JDK 8 we should certainly 
think of something more maintainable and less duplicating. BTW, adding 
comments to both java_md.c's for macosx and solaris stating that lots of 
the code is duplicated between the two would be great.

--
best regards,
Anthony

> However,  for jdk8 we need a  hierarchical structure, as I heard some 
> noise about
> restructuring the platform code, at that time it will be a good idea to 
> coalesce
> these things, and remove the LD_LIBRARY_PATH cloaking in solaris/java_md.c
> which is not relevant to MacOSX.
> 
>>
>> 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.
> Yes this can be done, I will look into it.
> 
> Kumar
> 
>>
>> -- 
>> 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 java_md.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
>>>>
>>>>
>>>>
>>>
>