Ready for Review : 8042469 : Launcher changes for native memory tracking scalability enhancement

[Neil Toda]

Sorry.  One more webrev .. 06

http://cr.openjdk.java.net/~ntoda/8042469/webrev-06/

Kumar's nit was correct and in fact index was old and should have been 
removed
allowing .contains member to be used instead of .indexOf.  So cleaned up 
a bit more
as can be seen below.  Other of Kumar's nits done.

Thanks Kumar.

webrev-06

  102             // get the PID from the env var we set for the JVM
  103             String envVarPid = null;
  104             for (String line : tr.testOutput) {
  105                 if (line.contains(envVarPidString)) {
  106                     int sindex = envVarPidString.length();
  107                     envVarPid = line.substring(sindex);
  108                     break;
  109                 }
  110             }
  111             // did we find envVarPid?
  112             if (envVarPid == null) {
  113                 System.out.println(tr);
  114                 throw new RuntimeException("Error: failed to find env Var Pid in tracking info");
  115             }



webrev-05

  102             // get the PID from the env var we set for the JVM
  103             haveLauncherPid = false;
  104             String envVarPid = null;
  105             for (String line : tr.testOutput) {
  106                 int index;
  107                 if ((index = line.indexOf(envVarPidString)) >= 0) {
  108                     int sindex = envVarPidString.length();
  109                     envVarPid = line.substring(sindex);
  110                     haveLauncherPid = true;
  111                     break;
  112                 }
  113             }
  114             // did we find envVarPid?
  115             if (!haveLauncherPid) {
  116                 System.out.println(tr);
  117                 throw new RuntimeException("Error: failed to find env Var Pid in tracking info");
  118             }



On 6/24/2014 2:28 PM, Neil Toda wrote:
>
> New webrev-05 with Kumar's comments except for the "C'ish" style. 
> Explained below.
>
> http://cr.openjdk.java.net/~ntoda/8042469/webrev-05/
>
> 105 : DONE
> 146: DONE
> 168: DONE
>
> 106: Your suggestion was the way I had originally coded it for Linux.  
> However
>          on Windows/Cygwin, the code will not compile  There is some 
> interaction
>          with the doExec() preceeding it and the fact that it is a 
> varlist.  This coding
>          was the simplest workaround.
>
> Thanks for the nits Kumar.
>
>
> On 6/24/2014 5:36 AM, Kumar Srinivasan wrote:
>> Neil,
>>
>> Some nits:
>>
>> TestSpecialArgs.java:
>>
>> extra space
>> 105 for ( String line : tr.testOutput) {
>>
>>
>>
>> This is very C'ish, I suggest.
>>  -106                 int index;
>>  -107                 if ((index = line.indexOf(envVarPidString)) >= 
>> 0) {
>>
>>  +106                 int index = line.indexOf(envVarPidString);
>>  +107                 if (index >= 0) {
>>
>>
>> Needs space
>> -146 launcherPid = line.substring(sindex,tindex);
>> +146 launcherPid = line.substring(sindex, tindex);
>>
>> Needs space
>>
>> -168             if (!tr.contains("NativeMemoryTracking: got value 
>> "+NMT_Option_Value)) {
>> +168             if (!tr.contains("NativeMemoryTracking: got value " 
>> + NMT_Option_Value)) {
>>
>>
>> Thanks
>> Kumar
>>
>>
>> On 6/23/2014 10:26 AM, Neil Toda wrote:
>>>
>>> I've spun a new webrev based on the comments for webrev-03:
>>>
>>>     http://cr.openjdk.java.net/~ntoda/8042469/webrev-04/
>>>
>>> Changes are:
>>>
>>>   1) java.c
>>>         a) add free(envName) line 1063
>>>         b) change from malloc() to JLI_MemAlloc() @ lines 1048 and 1056
>>>
>>> Thanks
>>>
>>> -neil
>>>
>>> On 6/20/2014 4:45 PM, Kumar Srinivasan wrote:
>>>> Neil,
>>>>
>>>> Generally looks good, yes JLI_* functions must be used, I missed 
>>>> that one.
>>>> Are you going to post another iteration ?
>>>>
>>>> Kumar
>>>>
>>>> On 6/20/2014 4:27 PM, Neil Toda wrote:
>>>>>
>>>>> Thanks Joe.  It would have checked for NULL for me.
>>>>> I'll use the JLI wrapper.
>>>>>
>>>>> -neil
>>>>>
>>>>> On 6/20/2014 4:04 PM, Joe Darcy wrote:
>>>>>> Memory allocation in the launcher should use one of the 
>>>>>> JLI_MemAlloc wrappers, if possible.
>>>>>>
>>>>>> -Joe
>>>>>>
>>>>>>
>>>>>> On 06/20/2014 09:50 AM, Neil Toda wrote:
>>>>>>>
>>>>>>> They should complain.  Thanks Zhengyu.  I'll make sure these are 
>>>>>>> non-null.
>>>>>>>
>>>>>>> -neil
>>>>>>>
>>>>>>> On 6/20/2014 5:01 AM, Zhengyu Gu wrote:
>>>>>>>> Neil,
>>>>>>>>
>>>>>>>> Thanks for quick implementation.
>>>>>>>>
>>>>>>>> java.c:
>>>>>>>>   Did not check return values of malloc(), I wonder if source 
>>>>>>>> code analyzers will complain.
>>>>>>>>
>>>>>>>> -Zhengyu
>>>>>>>>
>>>>>>>> On 6/19/2014 8:29 PM, Neil Toda wrote:
>>>>>>>>>
>>>>>>>>> Launcher support for modified Native Memory Tracking mechanism 
>>>>>>>>> in JVM in JDK9.
>>>>>>>>>
>>>>>>>>> Webrev  : http://cr.openjdk.java.net/~ntoda/8042469/webrev-03/
>>>>>>>>> bug         : https://bugs.openjdk.java.net/browse/JDK-8042469
>>>>>>>>> CCC         : http://ccc.us.oracle.com/8042469
>>>>>>>>>
>>>>>>>>> Thanks.
>>>>>>>>>
>>>>>>>>> -neil
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
>
>