RFR: 8079301: Some command line options not settable via JAVA_TOOL_OPTIONS
Jeremy Manson
jeremymanson at google.com
Thu Jul 16 17:20:58 UTC 2015
Thanks for the thorough review.
http://cr.openjdk.java.net/~jmanson/8079301/webrev.06/
As for other jint->int opportunities: the only ones I found were in loop
bounds, and I decided it didn't improve the code to change those.
Jeremy
On Thu, Jul 16, 2015 at 4:06 AM, Dmitry Dmitriev <dmitry.dmitriev at oracle.com
> wrote:
> Hello Jeremy,
>
> Thank you for doing this! Looks good, just few comments:
> 1) Latest code still not check return value of
> 'match_special_option_and_act' new function on lines 3702-3704.
> I repeat comment here from one of the e-mail below just in case if you
> missed it: In one case it can return error(JNI_ERR) when
> "-XX:NativeMemoryTracking" is specified and INCLUDE_NMT is not defined.
> Thus, I think that check for return value should be added.
> 2) There are some mix with int/jint variables and return values. I think
> that all these should be converted to jint.
> Here what I see:
> -change return value of new 'set_args' method on line 3415 to 'jint'.
> 'set_args' method called only once(on line 3515) and it return value is set
> to variable 'status' with 'jint' type.
> -change return value of new 'match_special_option_and_act' function on
> line 3602 to 'jint'
> -change type of 'code' variable on line 3691 to 'jint'. In some cases
> 'code' can be returned from Arguments::parse function, but return value of
> Arguments::parse function is 'jint'.
>
> Thank you,
> Dmitry
>
> On 16.07.2015 3:27, Jeremy Manson wrote:
>
> New rev:
>
> http://cr.openjdk.java.net/~jmanson/8079301/webrev.05/
>
> Thanks for all the attention!
>
> On Wed, Jul 15, 2015 at 3:39 PM, Jeremy Manson <jeremymanson at google.com>
> wrote:
>
>> Sorry about the delay. We were finally able to get JDK8 to the point
>> where we could use it without our services falling apart, so we rolled it
>> forward at Google. I've spent a lot of the last week putting out fires.
>> Also, house renovations. :/
>>
>> I think that it is long past time for Hotspot to start using the RAII
>> classes everyone else in the world adopted years ago, and will be happy to
>> implement one for this case. It was pretty hard for me to avoid the
>> temptation when I was writing this code in the first place, but the general
>> lack of them throughout the codebase made me think that they were
>> unpopular.
>>
>> Some crazy day we might even consider rolling forward to the 21st
>> century and starting to use unique_ptr in the source base.
>>
>> I'll ping the thread when there's a new rev.
>>
>> Jeremy
>>
>> On Tue, Jul 14, 2015 at 1:26 PM, Dmitry Dmitriev <
>> <dmitry.dmitriev at oracle.com>dmitry.dmitriev at oracle.com> wrote:
>>
>>> Hello Coleen,
>>>
>>> Yes, I like your approach. It seems more accurate. Thus, desctructor can
>>> be something like that:
>>> ~EnvInitArgs() {
>>> if( _args.options != NULL ) {
>>> for (int i = 0; i < _args.nOptions; i++) {
>>> os::free(_args.options[i].optionString);
>>> }
>>> FREE_C_HEAP_ARRAY(JavaVMOption, _args.options);
>>> }
>>> }
>>>
>>> Jeremy, what you think about that?
>>>
>>> Thank you,
>>> Dmitry
>>>
>>>
>>> On 14.07.2015 22:46, Coleen Phillimore wrote:
>>>
>>>
>>>
>>> On 7/12/15 10:27 AM, Dmitry Dmitriev wrote:
>>>
>>> Hello Coleen,
>>>
>>> I think that adding destructor not help with this, since all this code
>>> with free_memory_for_vm_init_args is a special workaround for processing
>>> options in env variables and not applied to the options from command line
>>> which passed in 'args' to the Arguments::parse function.
>>>
>>>
>>> I see - JavaVMInitArgs is a C struct in jni.h
>>>
>>> If you wrap JavaVMInitArgs for the options for environment variables in
>>> a scoped object that calls the destructor to clean up JavaVMInitArgs, this
>>> would eliminate all these extra calls that you have to make to
>>> free_memory_for_vm_init_args:
>>>
>>> class EnvInitArgs : public StackObj {
>>> JavaVMInitArgs _args;
>>> public:
>>> ~EnvInitArgs() {
>>> // Code to conditionally delete option memory that you have in
>>> free_memory_for_vm_init_args
>>> }
>>> JavaVMInitArgs* init_args() const { return &_args; }
>>> }
>>>
>>> You can put this in the constructor:
>>>
>>> + vm_args->version = JNI_VERSION_1_2;+ vm_args->options = NULL;+ vm_args->nOptions = 0;+ vm_args->ignoreUnrecognized = false;
>>>
>>> I think this would be neater than having to check for JNI_OK and call
>>> free everywhere.
>>>
>>> Coleen
>>>
>>>
>>>
>>> About match_special_option_and_act. Probably it can be processed in the
>>> following way(raw idea):
>>> code = match_special_option_and_act(&java_tool_options_args,
>>> &flags_file);
>>> if (code == JNI_OK) {
>>> // call next
>>> code = match_special_option_and_act(args, &flags_file);
>>> }
>>>
>>> if (code == JNI_OK) {
>>> // call next
>>> code = match_special_option_and_act(&java_options_args, &flags_file);
>>> }
>>>
>>> if (code != JNI_OK) {
>>> free_memory_for_vm_init_args(&java_tool_options_args);
>>> free_memory_for_vm_init_args(&java_options_args);
>>> return code;
>>> }
>>>
>>> Thanks,
>>> Dmitry
>>>
>>> On 11.07.2015 0:16, Coleen Phillimore wrote:
>>>
>>>
>>> Hi, I have a couple of suggestions.
>>>
>>> Can you add a destructor to JavaVMInitArgs which will delete the options
>>> things if they exist? Then you won't need these
>>> free_memory_for_vm_init_args functions. I think this would work.
>>>
>>> Secondly, I was also going to comment about match_special_option_and_act
>>> not using it's return value. The destructor code would make this less
>>> cumbersome. I like the refactoring though.
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 7/10/15 2:40 PM, Dmitry Dmitriev wrote:
>>>
>>> Jeremy, thank you for fixing that! Please, see my comments inline for 2
>>> item and one more comment.
>>>
>>> On 10.07.2015 20:15, Jeremy Manson wrote:
>>>
>>> Thanks for the detailed look, Dmitry. I have taken your advice, with a
>>> couple of exceptions:
>>>
>>> 1) I named the method free_memory_for_vm_init_args instead of
>>> free_memory_for_environment_variables.
>>> 2) I did not free the memory in 3661-3663 or in 3679-3681, because those
>>> are error paths where the memory may not have been allocated in the first
>>> place. So freeing it might lead to a crash.
>>>
>>> Yes, you are right. But I think we can guard from this by adding check
>>> for args->options in new free_memory_for_vm_init_args function and free
>>> memory in all execution paths. If args->options is not NULL, then memory is
>>> allocated and must be freed:
>>>
>>> 3485 static void free_memory_for_vm_init_args(JavaVMInitArgs* args) {
>>> 3486 if( args->options != NULL ) {
>>> 3487 for (int i = 0; i < args->nOptions; i++) {
>>> 3488 os::free(args->options[i].optionString);
>>> 3489 }
>>> 3490 FREE_C_HEAP_ARRAY(JavaVMOption, args->options);
>>> 3491 }
>>> 3492 }
>>>
>>> As I see you add free memory for error paths at lines(webrev.04)
>>> 3668-3671 and 3687-3691. Thus, with that guard you can add two calls to
>>> free_memory_for_vm_init_args function before "return JNI_EINVAL;" on line
>>> 3696:
>>>
>>> 3693 #ifdef ASSERT
>>> 3694 // Parse default .hotspotrc settings file
>>> 3695 if (!process_settings_file(".hotspotrc", false,
>>> args->ignoreUnrecognized)) {
>>> 3696 return JNI_EINVAL;
>>> 3697 }
>>> 3698 #else
>>>
>>>
>>> And one more comment. Unfortunately I oversight this before... You not
>>> check return value from 'match_special_option_and_act' new function on
>>> lines 3673-3675, but in one case it can return error(JNI_ERR) when
>>> "-XX:NativeMemoryTracking" is specified and INCLUDE_NMT is not defined.
>>> Thus, I think that check for return value should be added(with freeing
>>> memory for env variables in case of error).
>>>
>>> Thank you,
>>> Dmitry
>>>
>>>
>>> New Rev:
>>>
>>> http://cr.openjdk.java.net/~jmanson/8079301/webrev.04/
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.04/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.04/>
>>>
>>> Jeremy
>>>
>>> On Fri, Jul 10, 2015 at 5:52 AM, Dmitry Dmitriev <
>>> <dmitry.dmitriev at oracle.com>dmitry.dmitriev at oracle.com
>>> <mailto:dmitry.dmitriev at oracle.com> <dmitry.dmitriev at oracle.com>>
>>> wrote:
>>>
>>> Jeremy,
>>>
>>> Two additional comments which I didn't catch earlier...
>>>
>>> 1) Order of processing command line options is
>>> following(Arguments::parse_vm_init_args function): options from
>>> JAVA_TOOL_OPTIONS environment variable, then options from command
>>> line and then options from _JAVA_OPTIONS environment variable. And
>>> last argument wins!
>>>
>>> You not change this order in Arguments::parse_vm_init_args
>>> function, but it seems that order in several other places is
>>> changed.
>>>
>>> Lines 3665-3667:
>>>
>>> 3665 match_special_option_and_act(args, &flags_file);
>>> 3666 match_special_option_and_act(&java_tool_options_args,
>>> &flags_file);
>>> 3667 match_special_option_and_act(&java_options_args, &flags_file);
>>>
>>> Here we see that order is following: options from command line,
>>> then options from JAVA_TOOL_OPTIONS environment variable and then
>>> options from _JAVA_OPTIONS environment variable. Thus, in this
>>> case "-XX:+IgnoreUnrecognizedVMOptions" option in
>>> JAVA_TOOL_OPTIONS environment variable will have bigger priority
>>> than in command line(last argument wins), but this is differ from
>>> processing options in Arguments::parse_vm_init_args function.
>>> Thus, I think that you need to swap 3665 and 3666 lines:
>>>
>>> 3665 match_special_option_and_act(&java_tool_options_args,
>>> &flags_file);
>>> 3666 match_special_option_and_act(args, &flags_file);
>>> 3667 match_special_option_and_act(&java_options_args, &flags_file);
>>>
>>> Similar situation on lines 3696-3700:
>>>
>>> 3696 if (PrintVMOptions) {
>>> 3697 print_options(args);
>>> 3698 print_options(&java_tool_options_args);
>>> 3699 print_options(&java_options_args);
>>> 3700 }
>>>
>>> Command line options are printed before options from
>>> JAVA_TOOL_OPTIONS environment variable. Thus, I think that you
>>> need to swap 3697 and 3698 lines:
>>>
>>> 3696 if (PrintVMOptions) {
>>> 3697 print_options(&java_tool_options_args);
>>> 3698 print_options(args);
>>> 3699 print_options(&java_options_args);
>>> 3700 }
>>>
>>>
>>> 2) One more thing about error processing :)
>>> I think it will be great to free allocated memory for environment
>>> variables when error occured on lines 3661-3663, 3679-3681 and
>>> 3685-3687.
>>> My suggestion is to add some static function which free memory for
>>> env variables, something like that:
>>> static void free_memory_for_environment_variables( JavaVMInitArgs
>>> *options_args ) {
>>> for (int i = 0; i < options_args->nOptions; i++) {
>>> os::free(options_args->options[i].optionString);
>>> }
>>> FREE_C_HEAP_ARRAY(JavaVMOption, options_args->options);
>>> }
>>>
>>> And change lines 3661-3663 to following:
>>>
>>> 3661 if (code != JNI_OK) {
>>> 3662 free_memory_for_environment_variables(&java_tool_options_args);
>>> 3663 return code;
>>> 3664 }
>>>
>>> Lines 3679-3681 to following:
>>>
>>> 3679 if (!process_settings_file(flags_file, true,
>>> args->ignoreUnrecognized)) {
>>> 3680 free_memory_for_environment_variables(&java_tool_options_args);
>>> 3681 free_memory_for_environment_variables(&java_options_args);
>>> 3682 return JNI_EINVAL;
>>> 3683 }
>>>
>>> Lines 3685-3687 to following:
>>>
>>> 3685 if (!process_settings_file(".hotspotrc", false,
>>> args->ignoreUnrecognized)) {
>>> 3680 free_memory_for_environment_variables(&java_tool_options_args);
>>> 3681 free_memory_for_environment_variables(&java_options_args);
>>> 3686 return JNI_EINVAL;
>>> 3687 }
>>>
>>> And lines 3706-3715 to following:
>>>
>>> 3706 // Done with the envvars
>>> 3707 free_memory_for_environment_variables(&java_tool_options_args);
>>> 3708 free_memory_for_environment_variables(&java_options_args);
>>>
>>>
>>> Thank you,
>>> Dmitry
>>>
>>>
>>> On 10.07.2015 14:36, Dmitry Dmitriev wrote:
>>>
>>> Hello Jeremy,
>>>
>>> Thank you for fixing that! But in this case we still have
>>> minor issue when quotes processing failed(lines 3438-3444).
>>> Here is my suggestion: leave 'while' cycle(lines 3423-3462) as
>>> I was in your previous webrev(without 'start' and etc.) and
>>> call strdup when copy 'options' to 'options_arr' on lines
>>> 3472-3474, i.e. make lines 3472-3474 like this:
>>>
>>> 3472 for (int i = 0; i < options->length(); i++) {
>>> 3473 options_arr[i] = options->at(i);
>>> 3474 options_arr[i].optionString =
>>> os::strdup(options_arr[i].optionString);
>>> 3475 }
>>>
>>> What you think about that?
>>>
>>> Regards,
>>> Dmitry
>>>
>>> On 10.07.2015 3:23, Jeremy Manson wrote:
>>>
>>> Hey Dmitry,
>>>
>>> Thanks for looking at it. The leak was deliberate (under
>>> the theory that the command line flags were leaked, too),
>>> but it was silly that I didn't fix it.
>>>
>>> Anyway, fixed. See:
>>>
>>> http://cr.openjdk.java.net/~jmanson/8079301/webrev.03
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.03>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.03>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.03>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.03>
>>>
>>> Jeremy
>>>
>>> On Thu, Jul 9, 2015 at 2:52 PM, Dmitry Dmitriev
>>> < <dmitry.dmitriev at oracle.com>dmitry.dmitriev at oracle.com
>>> <dmitry.dmitriev at oracle.com>
>>> <mailto:dmitry.dmitriev at oracle.com> <dmitry.dmitriev at oracle.com>
>>> < <dmitry.dmitriev at oracle.com>
>>> mailto:dmitry.dmitriev at oracle.com <dmitry.dmitriev at oracle.com>
>>> <dmitry.dmitriev at oracle.com>
>>> <mailto:dmitry.dmitriev at oracle.com> <dmitry.dmitriev at oracle.com>>>
>>> wrote:
>>>
>>> Hello Jeremy,
>>>
>>> Correct me if I am wrong, but I see small memory leak
>>> in your
>>> implementation.
>>> In Arguments::parse_options_environment_variable
>>> function memory
>>> for 'buffer' allocated by strdup function on line
>>> 3415, but now it
>>> not freed in this function because data from this
>>> buffer is used
>>> later. On the other hand when we are done with env
>>> variables, we
>>> free only options array(lines 3703-3705) and not free
>>> previously
>>> allocated memory for 'buffer' because we lost track
>>> for it.
>>>
>>> Thanks,
>>> Dmitry
>>>
>>>
>>> On 09.07.2015 21:20, Jeremy Manson wrote:
>>>
>>> Thanks for looking at this, Coleen.
>>>
>>> I merged this with the latest version of hs-rt,
>>> but Ron is or
>>> I am going to
>>> have to do some merging for 8061999. The changes
>>> are relatively
>>> orthogonal, but they touch a couple of the same
>>> places. How
>>> far away is
>>> Ron's checkin?
>>>
>>> Updated webrev:
>>>
>>> http://cr.openjdk.java.net/~jmanson/8079301/webrev.01/
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.01/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.01/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.01/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.01/>
>>>
>>> Updated test:
>>>
>>> http://cr.openjdk.java.net/~jmanson/8079301t/webrev.01/
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.01/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.01/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.01/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.01/>
>>>
>>> (Mild complaining on my part: the conflicts
>>> wouldn't have been
>>> an issue if
>>> someone had looked at this two months ago, when I
>>> first
>>> asked. I suspect
>>> Ron hadn't even started his change at that
>>> point. And if external
>>> developers were able to submit changes to Hotspot,
>>> we wouldn't
>>> have had to
>>> bother Oracle engineers at all. Neither of these
>>> is your
>>> fault or your
>>> problem, of course - I'm grateful you were able to
>>> take a look.)
>>>
>>> Jeremy
>>>
>>> On Wed, Jul 8, 2015 at 6:19 AM, Coleen Phillimore <
>>> <coleen.phillimore at oracle.com>coleen.phillimore at oracle.com
>>> <coleen.phillimore at oracle.com>
>>> <mailto:coleen.phillimore at oracle.com> <coleen.phillimore at oracle.com>
>>> < <coleen.phillimore at oracle.com>
>>> mailto:coleen.phillimore at oracle.com <coleen.phillimore at oracle.com>
>>> <mailto:coleen.phillimore at oracle.com> <coleen.phillimore at oracle.com>>>
>>> wrote:
>>>
>>> I'll sponsor it but can you repost the RFR?
>>>
>>> There has been a fair bit of work to command
>>> line
>>> processing lately so I'd
>>> like to have the others look at it too. Have
>>> you merged
>>> this up with the
>>> latest version of hs-rt repository and were
>>> there
>>> conflicts? Also, have
>>> you looked at the RFR:
>>>
>>> "Open code review for 8061999 Enhance VM
>>> option parsing to
>>> allow options
>>> to be specified"
>>>
>>> and are there conflicts with this?
>>>
>>> The hotspot change looks good to me.
>>>
>>>
>>>
>>> http://cr.openjdk.java.net/~jmanson/8079301t/webrev.00/test/com/sun/management/HotSpotDiagnosticMXBean/CheckOrigin.java.udiff.html
>>>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.00/test/com/sun/management/HotSpotDiagnosticMXBean/CheckOrigin.java.udiff.html>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.00/test/com/sun/management/HotSpotDiagnosticMXBean/CheckOrigin.java.udiff.html>
>>>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.00/test/com/sun/management/HotSpotDiagnosticMXBean/CheckOrigin.java.udiff.html>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.00/test/com/sun/management/HotSpotDiagnosticMXBean/CheckOrigin.java.udiff.html>
>>>
>>> "UseCerealGC" LOL
>>> Can you use a different option than
>>> PrintOopAddress in
>>> this test? I don't
>>> know why this command line option exists or
>>> would be
>>> useful for, and seems
>>> to be a good candidate for removal. If you
>>> need a valid
>>> argument, that is
>>> unlikely to go away, TraceExceptions would be
>>> good.
>>>
>>> Thanks,
>>> Coleen
>>>
>>>
>>> On 7/7/15 8:00 PM, Jeremy Manson wrote:
>>>
>>> No love? Is there a code owner here I
>>> should pester
>>> more directly?
>>>
>>> Jeremy
>>>
>>> On Fri, Jun 26, 2015 at 3:00 PM, Martin
>>> Buchholz
>>> < <martinrb at google.com>martinrb at google.com
>>> <martinrb at google.com><mailto:martinrb at google.com>
>>> <martinrb at google.com> <mailto:martinrb at google.com <martinrb at google.com>
>>> <martinrb at google.com><mailto:martinrb at google.com>
>>> <martinrb at google.com>>>
>>> wrote:
>>>
>>> As usual with Google patches, they are
>>> in use
>>> locally. This one has been
>>>
>>> stable for a while without complaints.
>>> Please
>>> sponsor.
>>>
>>> On Fri, Jun 26, 2015 at 1:53 PM,
>>> Jeremy Manson
>>> <jeremymanson at google.com
>>> <jeremymanson at google.com><mailto:jeremymanson at google.com>
>>> <jeremymanson at google.com>
>>> <mailto:jeremymanson at google.com <jeremymanson at google.com>
>>> <jeremymanson at google.com><mailto:jeremymanson at google.com>
>>> <jeremymanson at google.com>>>
>>> wrote:
>>>
>>> All this talk about
>>> IgnoreUnrecognizedVMOptions
>>> reminded me that this
>>>
>>> review is still outstanding. Any
>>> takers?
>>>
>>> Jeremy
>>>
>>> On Tue, May 5, 2015 at 10:01 AM,
>>> Jeremy Manson
>>> <jeremymanson at google.com
>>> <jeremymanson at google.com><mailto:jeremymanson at google.com>
>>> <jeremymanson at google.com>
>>> <mailto:jeremymanson at google.com <jeremymanson at google.com>
>>> <jeremymanson at google.com><mailto:jeremymanson at google.com>
>>> <jeremymanson at google.com>>
>>> wrote:
>>>
>>> Hi David,
>>>
>>> Thanks for taking a look.
>>>
>>> On Mon, May 4, 2015 at 8:32
>>> PM, David
>>> Holmes
>>> < <david.holmes at oracle.com>david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com> <david.holmes at oracle.com>
>>> < <david.holmes at oracle.com>mailto:david.holmes at oracle.com
>>> <david.holmes at oracle.com>
>>> <david.holmes at oracle.com><mailto:david.holmes at oracle.com>
>>> <david.holmes at oracle.com>>>
>>>
>>> wrote:
>>>
>>> Hi Jeremy,
>>>
>>> On 5/05/2015 6:51 AM,
>>> Jeremy Manson wrote:
>>>
>>> Not sure who might be
>>> willing to
>>> sponsor this. David? You
>>> did the
>>>
>>> last
>>> one. :)
>>>
>>> Might be a while
>>> before I can
>>> get to it. I did have
>>> a quick look
>>>
>>> (and
>>> will need a longer one).
>>>
>>> I understand. I'm just happy
>>> it's
>>> possible to upstream this stuff
>>> at
>>> all[1].
>>>
>>> [1] With the eternal caveat
>>> that I'd be
>>> happier if we didn't *have* to
>>> go through you folks, but
>>> we've learned to
>>> live with it.
>>>
>>>
>>>
>>> Context: A number of
>>> command line
>>> options are not settable via
>>>
>>> JAVA_TOOL_OPTIONS and _JAVA_OPTIONS:
>>>
>>> - PrintVMOptions
>>>
>>> So you have changed
>>> the
>>> semantics of this to
>>> print out the
>>> options
>>>
>>> from
>>> the command-line and each
>>> of the two
>>> env vars. Seems reasonable
>>> but
>>> may be
>>> better to know which
>>> option came from
>>> where as we can now see
>>> the same
>>> option (or conflicting
>>> variants
>>> thereof) multiple times.
>>>
>>> I'd be happy to do
>>> this. Any
>>> preferred syntax? Does
>>> someone
>>>
>>> actually
>>> own this feature?
>>>
>>> I'm unclear what the current
>>> processing
>>> order is for the different
>>>
>>> sources of options, and
>>> whether the
>>> changes affect that order?
>>>
>>> Nope. I go through sad
>>> contortions
>>> to keep the processing
>>> order the
>>>
>>> same. It's JAVA_TOOL_OPTIONS,
>>> then
>>> command line, then
>>> _JAVA_OPTIONS.
>>> See
>>> lines 2549-2567.
>>>
>>>
>>> -
>>> IgnoreUnrecognizedVMOptions
>>>
>>> - PrintFlagsInitial
>>>
>>> Unclear what
>>> "initial" actually
>>> means - is it the
>>> default?
>>>
>>> More or less. If you stick,
>>> for example,
>>> IgnoreUnrecognizedVMOptions
>>> in
>>> there first, it will get
>>> printed out as
>>> "true". I haven't really
>>> changed
>>> the semantics here either -
>>> I've just
>>> added processing of the envvars.
>>>
>>> - NativeMemoryTracking
>>>
>>> -
>>> PrintFlagsWithComments
>>>
>>> This is because these
>>> flags have
>>> to be processed before
>>> processing
>>> other
>>> flags, and no one ever
>>> bothered to
>>> do that for the flags
>>> coming from
>>> environment
>>> variables. This patch
>>> fixes that problem.
>>>
>>> I have a test, but it is
>>> a
>>> modification to a JDK
>>> test instead
>>> of a HS
>>> test,
>>> so it can go in
>>> separately (I guess).
>>>
>>> They can be pushed
>>> together to
>>> different repos in the
>>> same forest.
>>>
>>> Okay. Here's the test
>>> change:
>>>
>>> http://cr.openjdk.java.net/~jmanson/8079301t/webrev.00/
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.00/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.00/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.00/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301t/webrev.00/>
>>>
>>>
>>> As I said I'll have to get
>>> back to this.
>>> And hopefully someone else in
>>>
>>> runtime will take a good
>>> look as well ;-)
>>>
>>> I'd be happy to toss it
>>> over to
>>> whomever owns this, if
>>> anyone.
>>>
>>> Thanks!
>>>
>>> Jeremy
>>>
>>>
>>>
>>> Bug:
>>>
>>> <https://bugs.openjdk.java.net/browse/JDK-8079301>
>>> https://bugs.openjdk.java.net/browse/JDK-8079301
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~jmanson/8079301/webrev.00/
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.00/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.00/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.00/>
>>> <http://cr.openjdk.java.net/%7Ejmanson/8079301/webrev.00/>
>>>
>>> Jeremy
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>
>
More information about the hotspot-runtime-dev
mailing list