JDK-8027113 fix for jdk 9 code review updated

[David Holmes]

On 30/01/2014 2:40 AM, Ron Durbin wrote:
> David
>
> First off thanks for taking the time to code review this fix.
>
> Fixing this defect is limited to separating the gamma launcher cruft from the rest of The rest of the launcher code and this is the first step.
> The second step is is to actually remove the gamma launcher cruft.
>
> - The primary reason for these changes is to decouple "gamma" from
>     the alternate JVM stuff so we can finish removing "gamma".
> - The alternate JVM stuff makes life easier for the JVM developer
>     so we don't want to touch the launcher if we don't have to.
> - The need for -XXaltjvm=... and -Dsun.java.launcher.is_altjvm=true
>     to be used in combination is no different than the existing code
>     that requires -XXaltjvm=... and -Dsun.java.launcher=gamma to be
>     used in combination. The goal of this change is not to clean up
>     the alternate JVM mechanism.

Well I hope there is some plan to clean it up :) The control flow seems 
very weird to me.

Cheers,
David

> Arguments::process_java_launcher_argument
> I took a look at removing Arguments::process_java_launcher_argument.
> Refactoring the code to remove the Arguments::process_java_launcher_argument is
> something that we should maybe look at some point.  However it is beyond the scope of decoupling the gamma launch option from the launcher.
>
> Thx
>
> Ron
>
>
>
>> -----Original Message-----
>> From: David Holmes
>> Sent: Monday, January 27, 2014 10:38 PM
>> To: Ron Durbin; hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: JDK-8027113 fix for jdk 9 code review updated
>>
>> Hi Ron,
>>
>> On 28/01/2014 2:18 PM, Ron Durbin wrote:
>>>
>>> JDK-8027113 decouples the '-XXaltjvm=<path>' option from the gamma
>>> launcher
>>
>> This had me somewhat confused as I (not unreasonably) assumed that the -XXaltjvm option was
>> processed by the VM. I didn't realize it was processed by the launcher (makes sense as it is
>> telling the launcher which libjvm to load)! That said I'm unclear why the launcher doesn't
>> then tell the VM that -XXaltjvm was used (by setting the property) rather than the user
>> having to specify both -XXaltjvm and -Dsun.java.launcher.is_altjvm=true on the command-line?
>> What happens if the user lies and sets -Dsun.java.launcher.is_altjvm=false, or doesn't set
>> it at all; or sets it but not -XXaltjvm ? This seems error prone.
>>
>> Aside: Arguments::process_java_launcher_argument seems somewhat unnecessary now all it does
>> is a strdup.
>>
>> Thanks,
>> David
>>
>>> Web URL http://cr.openjdk.java.net/~rdurbin/JDK-8027113-cr1-webrev
>>>
>>> Internal URL:
>>> http://javaweb.us.oracle.com/~rdurbin/JDK-8027113-cr1-webrev
>>>
>>> Summary:
>>> Refactor the code so that it is no longer tightly coupled with the "gamma launcher". This
>> will allow work on the following bug to proceed:
>>>
>>>       JDK-8005262 possible gamma launcher issues
>>>
>>> Testing included:
>>>      JPRT run
>>>      Manual launcher tests on:
>>> 	Win7-64 Cygwin,
>>> 	Win7-64 VS2010.
>>> 	Solaris 64 bit
>>>        Linux 64 bit
>>>