Ready for Review : 8042469 : Launcher changes for native memory tracking scalability enhancement
Neil Toda
neil.toda at oracle.com
Wed Jun 25 20:40:34 UTC 2014
Okay, Very glad you checked. It really does need to go as early as your
prototype suggested.
I'll get right on it.
-neil
On 6/25/2014 12:05 PM, Zhengyu Gu wrote:
> Hi Neil,
>
> I tried out this patch with my hotspot, it does not work.
>
> The reason is that, the environment variable is setup too late, it has
> to be set before it launches JavaVM (before calling LoadJavaVM()) method.
>
> Thanks,
>
> -Zhengyu
>
> On 6/25/2014 1:58 PM, Neil Toda wrote:
>> 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
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
>
More information about the core-libs-dev
mailing list