RFR (S) round 0 for 8141068 Refactor -XXFlags= code in preparation for removal (original) (raw)

Daniel D. Daugherty daniel.daugherty at oracle.com
Wed Nov 11 17:34:56 UTC 2015

On 11/11/15 9:43 AM, Coleen Phillimore wrote:

On 11/11/15 11:36 AM, Daniel D. Daugherty wrote: This needs a little bit of clarification:

3976 jint Arguments::matchspecialoptionandact(const JavaVMInitArgs* args, 3977 char ** vmoptionsfile, 3978 ScopedVMInitArgs* argsout) { The 'vmoptionsfile' parameter is currently used for two things: 1) when the parameter is non-NULL, that indicates the caller supports '-XX:VMOptionsFile'; currently only the cmd line option bucket supports '-XX:VMOptionsFile' The argsout != NULL parameter indicates the same thing.

Yes. That's the check that used to be done. However, Ron previously consolidated all of the checks to use the same variable (vm_options_file). It seemed to confuse folks that one check was done with 'vm_options_file' and the other check was done with 'args_out'.

2) makes sure that only one -XX:VMOptionsFile=foo option is specified on the cmd line You can do that with a local variable vmoptionsfile in matchspecialoptionandact. So it can be trivially removed. But it's not a big deal.

Yes, that's possible. And then we would be back in the situation where one check is done via 'args_out' and the other check is done via 'vm_options_file'.

The other reason for having 'vm_options_file' have a more global scope was that we didn't know whether the restriction for having only one '-XX:VMOptionsFile' would apply across all buckets when support was added for the env vars. Again, that's something that will be dealt with JDK-8135198.

Dan

Coleen

Restriction #1 is implemented by: 3994 if (vmoptionsfile != NULL) { 4022 } else { 4023 jiofprintf(defaultStream::errorstream(), 4024 "VM options file is only supported on the command line\n"); 4025 return JNIEINVAL; 4026 } Restriction #2 is implemented by: 3994 if (vmoptionsfile != NULL) { 3995 // The caller accepts -XX:VMOptionsFile 3996 if (*vmoptionsfile != NULL) { 3997 jiofprintf(defaultStream::errorstream(), 3998 "The VM Options file can only be specified once and " 3999 "only on the command line.\n"); 4000 return JNIEINVAL; 4001 } So the 'vmoptionsfile' parameter cannot be removed at this time without changing the current set of restrictions on the '-XX:VMOptionsFile' option. Ron has another bug looking at lifting those restrictions: JDK-8135198 Specifying a vm options file should not be limited to only once on the command line https://bugs.openjdk.java.net/browse/JDK-8135198 JDK-8135198 will require a CCC request in order to change the semantics that were defined in the original work (JDK-8061999). Dan

On 11/10/15 2:02 PM, Ron Durbin wrote: Coleen This is to formally respond to your email. Dan and I agreed the change you are requesting should be done, but not in this change set. Keeping this change set to only XX:Flags changes. A future change set that could clean up the options file processing should address your issue. Ron

-----Original Message----- From: Coleen Phillimore Sent: Monday, November 09, 2015 2:03 PM To: hotspot-runtime-dev at openjdk.java.net Subject: Re: RFR (S) round 0 for 8141068 Refactor -XXFlags= code in preparation for removal

Ron, This looks really good! Although I don't see why you need to pass vmoptionsfile argument into matchspecialoptionsandact now since it's not used afterward, so you only have to pass one NULL in the cases where vmoptionsfile is not allowed. thanks, Coleen On 11/6/15 2:05 PM, Ron Durbin wrote: This bug fix consists of a refactoring/cleanup of the -XX:Flags code. Webrev: http://cr.openjdk.java.net/~rdurbin/8141068OCR0JDK9webrev

Bug report: https://bugs.openjdk.java.net/browse/JDK-8141068 This bug fix has been tested on: OS: Solaris, MAC, Windows, Linux Tests: Manual unit tests JPRT with -testset hotspot(including the SQE current test coverage for this feature.)

More information about the hotspot-runtime-dev mailing list