RFR: 8258917: NativeMemoryTracking is handled by launcher inconsistenly [v2]

[David Holmes]

Hi Alex,

On 28/01/2021 7:44 am, Alex Menkov wrote:
> On Tue, 19 Jan 2021 23:18:04 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
>>>
>>>    Non-lava launchers should process all "-J" arguments
>>
>> Hi Alex,
>>
>> I think this is functionally correct now - though the comment you added is confusing to me (see below).
>>
>> However I remain concerned that this requires processing the arg list twice for non-Java launchers. Is that a startup issue we should be concerned about?
>>
>> Thanks,
>> David
> 
> Hi David,
> 
>> You integrated this change but neither Zhengyu nor myself approved the
>> final changes. Zhengyu's Review was prior to the logic fix, and my
>> comment "this seems functionally correct" was just a comment and we were
>> still discussing the overhead aspect of this.
>>
>> David
> 
> I was sure I saw 2 approvals for the fix, but most likely I misread it and there were 2 reviewers.
> Do you have some suggestions how to handle this?

Just ask Zhengyu to add his final Review as a comment to show he is okay 
with the final result, and I'll do the same via this email. :)

> I don't think it makes sense to revert the change and re-do it from the beginning.

No.

> I think we don't need to worry about startup time impact. It's minimal and only non-java launchers are affected (AFAIR the main goal of performance/startup optimizations was java launcher).

Yes, it is our main concern. But that's not to say someone else may not 
be concerned. Any way the point was to discuss the overhead in case it 
was an issue and we're deciding we think it is not an issue so okay.

> I can file an RFE to reorder launcher logic and handle NMT in ParseArguments as I described earlier. If it's possible, this would remove one cycle (SetJvmEnvironment) for java/javaw launchers.

Please do. I don't know if anyone will look at it, but at least it 
acknowledges the current approach is not optimial.

Thanks,
David
-----

> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/2106
>