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