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

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Tue Jun 24 12:36:53 UTC 2014


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




More information about the core-libs-dev mailing list