[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: JDK-8027113 fix for jdk 9 code review updated
From:       David Holmes <david.holmes () oracle ! com>
Date:       2014-01-30 1:29:20
Message-ID: 52E9AAF0.4070100 () oracle ! com
[Download RAW message or body]

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@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
> > > 


[prev in list] [next in list] [prev in thread] [next in thread]