RFR: JDK-8058407: Remove Multiple JRE support in the Java launcher

2014-12-05 Thread Kumar Srinivasan
Hello, Please review the fix for JDK-8058407, contributed by Neil Toda, described by JEP 231 [1] the webrev is at: http://cr.openjdk.java.net/~ksrini/8058407/webrev.00/ Please note: The above webrev is identical to the original posted by Neal at http://cr.openjdk.java.net/~ntoda/8058407/webrev

Re: [9] RFR (S) 6762191: Setting stack size to 16K causes segmentation fault

2014-12-03 Thread Kumar Srinivasan
On 12/3/2014 11:26 AM, Chris Plummer wrote: Hi Kumar, On 12/3/14 10:58 AM, Kumar Srinivasan wrote: Hi Chris, Approved with some minor nits, typos which needs correction. yes java.c follows the JDK indenting as Alan pointed out. TooSmallStackSize.java Copyright should be 2014, Copy/paste

Re: [9] RFR (S) 6762191: Setting stack size to 16K causes segmentation fault

2014-12-03 Thread Kumar Srinivasan
Hi Chris, Approved with some minor nits, typos which needs correction. yes java.c follows the JDK indenting as Alan pointed out. TooSmallStackSize.java Copyright should be 2014, 1. 37 * stack size for the platform (as provided by the JVM error message when a very 38 * small is used),

Re: RFR 8060026: Update test/tools/launcher tests to eliminate dependency on sun.tools.jar.Main

2014-11-26 Thread Kumar Srinivasan
Ok I will get this pushed. Kumar On 11/25/2014 12:57 AM, Amy Lu wrote: Thank you for your review. I also need your help to push this change. Thanks, Amy On 11/25/14 7:46 AM, Kumar Srinivasan wrote: Looks good. Kumar On 11/24/2014 1:47 AM, Amy Lu wrote: Some test/tools/launcher tests

Re: RFR 8060026: Update test/tools/launcher tests to eliminate dependency on sun.tools.jar.Main

2014-11-24 Thread Kumar Srinivasan
Looks good. Kumar On 11/24/2014 1:47 AM, Amy Lu wrote: Some test/tools/launcher tests have dependency on sun.tools.jar.Main The dependency actually comes from TestHelper.java This fix is to remove above internal JDK API dependency from the tests. bug: https://bugs.openjdk.java.net/browse/JDK-8

RFR: 8061830: [asm] refresh internal ASM version v5.0.3

2014-10-22 Thread Kumar Srinivasan
Hello, Please review fix for JDK-8061830, this is merely a refresh of the existing source base from upstream ObjectWeb/ASM, the webrev is here: http://cr.openjdk.java.net/~ksrini/8061830/webrev.00/ All the validations performed are documented in the JBS issue. Thanks Kumar

Re: RFR 8060432: tools/pack200/TestNormal.java fails on Windows with java.io.FileNotFoundException after JDK-8058854

2014-10-16 Thread Kumar Srinivasan
Amy, I will take care of pushing this on Friday. Kumar On 10/16/2014 12:36 AM, Amy Lu wrote: Thank you Kumar for your review. I need a sponsorfor push this fix. Alan, may I get your help? Thanks, Amy On 10/16/14, 8:31 AM, Kumar Srinivasan wrote: Looks good! Kumar On 10/14/2014 7:42 PM

Re: RFR 8060432: tools/pack200/TestNormal.java fails on Windows with java.io.FileNotFoundException after JDK-8058854

2014-10-15 Thread Kumar Srinivasan
Looks good! Kumar On 10/14/2014 7:42 PM, Amy Lu wrote: On 10/15/14, 4:44 AM, Kumar Srinivasan wrote: Amy, The modifications you have made will not test pack200 compression and normalization correctly, as the test needs ".class" files. Sorry, I missed that. Please review the updat

Re: RFR 8060432: tools/pack200/TestNormal.java fails on Windows with java.io.FileNotFoundException after JDK-8058854

2014-10-14 Thread Kumar Srinivasan
Amy, The modifications you have made will not test pack200 compression and normalization correctly, as the test needs ".class" files. Do you know why the test fails on windows ? Kumar On 10/14/2014 7:19 AM, Amy Lu wrote: Please review the test fix. bug: https://bugs.openjdk.java.net/browse/J

Re: RFR 8058854: Remove dependency on dt.jar from test/tools/jar/normalize/TestNormal.java

2014-10-13 Thread Kumar Srinivasan
+1 Kumar On 10/13/2014 8:18 AM, Alan Bateman wrote: On 13/10/2014 11:56, Amy Lu wrote: Moved the test to pack200 http://cr.openjdk.java.net/~weijun/8058854/webrev.01/ This looks okay to me now. -Alan

RFR: 8059973: Broken link in Class Pack200

2014-10-09 Thread Kumar Srinivasan
Hello, Please review simple fix to remove the URL reference to Java Deployment Guide, these tend to move, causing broken link. Thanks Kumar diff --git a/src/java.base/share/classes/java/util/jar/Pack200.java b/src/java.base/share/classes/java/util/jar/Pack200.java --- a/src/java.base/share/

Re: 2d reviewer please . Re: RFR : JDK-8046545 launcher fix to check function return values

2014-07-24 Thread Kumar Srinivasan
I_RETURN and CHECK_JNI_RETRUN_EXCEPTION I also think it is cleaner since there are only two, and they are for JNI, to keep them self contained. Would someone be willing to review webrev-02, which contain Kumar's suggested change in the comments included with the macros. Thanks -neil

Re: RFR : JDK-8046545 launcher fix to check function return values

2014-07-19 Thread Kumar Srinivasan
; \ -} \ -} while (JNI_FALSE) +NULL_CHECK_RETURN_VALUE(JNI_RETURN, RETURN_VALUE); Kumar On 7/18/2014 10:40 AM, Neil Toda wrote: Thanks Kumar. Yes, misspoke here. Will correct the comment. On 7/18/2014 10:35 AM, Kumar Srinivasan wrote: Neil, The fix looks good. However there is an inaccuracy in the

Re: RFR : JDK-8046545 launcher fix to check function return values

2014-07-18 Thread Kumar Srinivasan
Neil, The fix looks good. However there is an inaccuracy in the comment: + * Normally, JNI calls do not return if an exception is thrown. + * However, this behavior can change in the future, + * so check for thrown exceptions. This is not true, JNI calls *will* return if an exception is throw

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

2014-06-27 Thread Kumar Srinivasan
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

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

2014-06-25 Thread Kumar Srinivasan
s 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

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

2014-06-24 Thread Kumar Srinivasan
.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

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

2014-06-20 Thread Kumar Srinivasan
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:

Please review: (JDK-8044866) Fix raw and unchecked lint warnings in asm

2014-06-20 Thread Kumar Srinivasan
Hi Joe, all, Please approve fix for removings warnings in asm, this is merely a mini refresh from upstream sources, I have verified there are no more warnings in asm sources. http://cr.openjdk.java.net/~ksrini/8044866/webrev/ Thanks Kumar PS: many thanks to Remi Forax for pushing these fixes

Re: RFR 8044647: sun/tools/jrunscript/jrunscriptTest.sh start failing: Output of jrunscript -l nashorn differ from expected output

2014-06-03 Thread Kumar Srinivasan
don't forget to mark the bug with noreg-self. Kumar On 6/3/2014 8:49 AM, Kumar Srinivasan wrote: Looks good. Kumar On 6/3/2014 8:26 AM, A. Sundararajan wrote: Hi, Please review http://cr.openjdk.java.net/~sundar/8044647/ Thanks, -Sundar

Re: RFR 8044647: sun/tools/jrunscript/jrunscriptTest.sh start failing: Output of jrunscript -l nashorn differ from expected output

2014-06-03 Thread Kumar Srinivasan
Looks good. Kumar On 6/3/2014 8:26 AM, A. Sundararajan wrote: Hi, Please review http://cr.openjdk.java.net/~sundar/8044647/ Thanks, -Sundar

Please review: 8044046: [asm] refresh internal ASM version to v5.0.3

2014-05-27 Thread Kumar Srinivasan
Hello, Please review the following webrev which updates internal ASM to v5.0.3, the individual bug fixes are listed in the JBS issue for reference, https://bugs.openjdk.java.net/browse/JDK-8044046#comment-13501358 All core regression tests have been run, additionally nashorn regressions, tes

Re: RFR: 8042266: [launcher] create test groups for launcher regression tests

2014-05-02 Thread Kumar Srinivasan
Point taken, I just added jdk_launcher for now. http://cr.openjdk.java.net/~ksrini/8042266/webrev.01/ Thanks On 5/2/2014 6:30 AM, Alan Bateman wrote: On 02/05/2014 14:17, Kumar Srinivasan wrote: Hi, Simple changes to add jdk_launcher to the jdk's Test.groups, this allows anyone workin

RFR: 8042266: [launcher] create test groups for launcher regression tests

2014-05-02 Thread Kumar Srinivasan
Hi, Simple changes to add jdk_launcher to the jdk's Test.groups, this allows anyone working on the launcher to run a set of regression tests which historically have helped shake out launcher regressions. http://cr.openjdk.java.net/~ksrini/8042266/webrev.00/ Usage: jtreg .. /blah/de/blah/j

Re: 8035782 : sun/launcher/LauncherHelper$FXHelper loaded unnecessarily

2014-04-30 Thread Kumar Srinivasan
For completeness the bugid line needs the bugid as shown, otherwise SQE will open another bug to have you fix this. -26 * @bug 8001533 8004547 +26 * @bug 8001533 8004547 8035782 other than that it looks good, I can push this with the above change. Anyone else have any concerns with this c

Re: review request: 8035782 : sun/launcher/LauncherHelper$FXHelper loaded unnecessarily

2014-04-29 Thread Kumar Srinivasan
be!"); + } for (String p : APP_PARMS) { if (!tr.contains(p)) { System.err.println("ERROR: Did not find " + p + " in output!"); } } } Hope this helps. Kuma

Re: review request: 8035782 : sun/launcher/LauncherHelper$FXHelper loaded unnecessarily

2014-04-29 Thread Kumar Srinivasan
Neil, The changes looks satisfactory, except for a few style nits: 1. JAVAFX_FXHELPER_CLASS_NAME_SUF -> JAVAFX_FXHELPER_CLASS_NAME_SUFFIX // 3 more characters won't make much of a difference 2. FXHelper.setFXLaunchParameters(what,mode); // missing space after comma. A Launcher regression test

RFR: 8037529: [asm] add back @Suppress tags, removed by last bulk refresh

2014-03-17 Thread Kumar Srinivasan
Hello, Please review fix for adding back @SuppressWarnings tag http://cr.openjdk.java.net/~ksrini/8037529/webrev.0/ removed inadvertently with the push for http://cr.openjdk.java.net/~ksrini/8037221/ Remi, can you please facilitate getting this fix back into the ASM source base ? this will m

Re: RFR: JDK9: 8037221: [asm] refresh internal ASM version

2014-03-13 Thread Kumar Srinivasan
team, the only file we generate is the version.txt to help us identify what we have. Kumar Paul. On Mar 12, 2014, at 10:24 PM, Kumar Srinivasan wrote: Hello, Appreciate if someone cane review this change, so that I can push this into the jdk9 repository, refreshing the internal ASM

RFR: JDK9: 8037221: [asm] refresh internal ASM version

2014-03-12 Thread Kumar Srinivasan
Hello, Appreciate if someone cane review this change, so that I can push this into the jdk9 repository, refreshing the internal ASM sources from rev 1700 to 1721, of particular interest is Rev 1710, which fixes [1] known to cause verification errors in the VM. Thereafter I will be back-porting

Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup (re-review)

2014-02-03 Thread Kumar Srinivasan
+1 Kumar On 2/3/2014 12:17 PM, roger riggs wrote: Please re-review. I missed a warning that the CHECK_NULL macros was being redefined. Retaining the previous changes to java/util/jar/pack that removed the redefinition addresses the issue. The webrev now only has a few small changes to clea

Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup

2014-02-03 Thread Kumar Srinivasan
+1 Kumar On 2/3/2014 9:45 AM, Mandy Chung wrote: Looks good. Mandy On 2/1/2014 10:03 AM, roger riggs wrote: Hi, A few small changes to cleanup the name and use of CHECK macros. The CHECK_NULL_* macros are more generally useful without the JNU_ prefix. The JNU_CHECK_EXCEPTION* macros apply

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-22 Thread Kumar Srinivasan
Alex, I am satisfied. Thanks Kumar Kumar, see my notes inline. On 1/21/14 20:11, Kumar Srinivasan wrote: Alex, Thanks for adding the test, few comments: PackTestZip64.java: 1. compareTwoFile, I would read the entire file into ByteArrayInputStream, compare the total first ie. fast

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-21 Thread Kumar Srinivasan
Alex, Thanks for adding the test, few comments: PackTestZip64.java: 1. compareTwoFile, I would read the entire file into ByteArrayInputStream, compare the total first ie. fast fail, but what you have is also ok. 2. generateLargeJar: I would replace lines 126,127 and 128 using for-each loop

Re: RFR: (8031737) rename jni_util.h macros for checking and returning on exceptions

2014-01-16 Thread Kumar Srinivasan
Roger, one more thing, shouldn't the parameters be unique ? I think Martin had me do this for all macros in the java launcher for example please see this changeset, I recently pushed. http://hg.openjdk.java.net/jdk9/dev/jdk/rev/6c50c972a101 Kumar On 1/16/2014 9:30 AM, Kumar Srinivasan

Re: RFR: (8031737) rename jni_util.h macros for checking and returning on exceptions

2014-01-16 Thread Kumar Srinivasan
Hi Roger, Its confusing to use a JNU_ prefixed macro on a method not involvng jni, why not rename these to modulo JNU_ ? I am cc'ing Alex as he has a related bug fix in his queue for pack's jni code. Kumar Hi Alan, The macros are generally useful even without being used on a method that in

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-15 Thread Kumar Srinivasan
Hi Alex, Kumar, thanks for your findings. See my comments inline. On 1/15/14 2:10, Kumar Srinivasan wrote: Hi Alex, zip.cpp: (nit) I would keep the hex values to be in upper case just like the others for consistency, not a big deal. Fixed. typo

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-14 Thread Kumar Srinivasan
Hi Alex, zip.cpp: (nit) I would keep the hex values to be in upper case just like the others for consistency, not a big deal. typo: + // Zip64 END sugnature PackTestZip64.java: shouldn't we test a jar with 64K+ entries ? Kumar On 1/14/2014 10:04 AM, Alexande

Re: 8031494: [launcher] java launcher should check for JNI Pending exceptions.

2014-01-14 Thread Kumar Srinivasan
Srinivasan wrote: Mandy, On 1/13/2014 2:44 PM, Kumar Srinivasan wrote: The full webrev: http://cr.openjdk.java.net/~ksrini/8031494/webrev.1/ This looks good. Should it also check if there is any pending exception after the call to CallStaticVoidMethod that has no return value? It's fine

Re: 8031494: [launcher] java launcher should check for JNI Pending exceptions.

2014-01-13 Thread Kumar Srinivasan
Mandy, On 1/13/2014 2:44 PM, Kumar Srinivasan wrote: The full webrev: http://cr.openjdk.java.net/~ksrini/8031494/webrev.1/ This looks good. Should it also check if there is any pending exception after the call to CallStaticVoidMethod that has no return value? It's fine with me i

Re: 8031494: [launcher] java launcher should check for JNI Pending exceptions.

2014-01-13 Thread Kumar Srinivasan
/index.html The full webrev: http://cr.openjdk.java.net/~ksrini/8031494/webrev.1/ Thanks for the review. Kumar Hi Alan, Chris, On 11/01/2014 00:55, Kumar Srinivasan wrote: Hi, Please review fixes for launcher correctness wrt. JNI calls. http://cr.openjdk.java.net/~ksrini/8031494/webrev.0/ Looks

Re: 8031494: [launcher] java launcher should check for JNI Pending exceptions.

2014-01-13 Thread Kumar Srinivasan
Hi Alan, Chris, On 11/01/2014 00:55, Kumar Srinivasan wrote: Hi, Please review fixes for launcher correctness wrt. JNI calls. http://cr.openjdk.java.net/~ksrini/8031494/webrev.0/ Looks okay, the only thing that isn't clear is whether the calls to the static methods defined by the lau

8031494: [launcher] java launcher should check for JNI Pending exceptions.

2014-01-10 Thread Kumar Srinivasan
Hi, Please review fixes for launcher correctness wrt. JNI calls. http://cr.openjdk.java.net/~ksrini/8031494/webrev.0/ Thanks Kumar

Re: RFR: jdk9: 8029997: [infra] remove Solaris ISA directories and the links

2013-12-23 Thread Kumar Srinivasan
On 12/21/2013 12:08 AM, Alan Bateman wrote: On 20/12/2013 23:03, Kumar Srinivasan wrote: Hello, Please review the removal of ISA (Instruction Specific Architecture) directories namely sparcv9, amd64 and the symlinks in these directories, this was provided to aid transition to jdk8, where

RFR: jdk9: 8029997: [infra] remove Solaris ISA directories and the links

2013-12-20 Thread Kumar Srinivasan
Hello, Please review the removal of ISA (Instruction Specific Architecture) directories namely sparcv9, amd64 and the symlinks in these directories, this was provided to aid transition to jdk8, where solaris 32-bit was removed, and the 32-bit binaries were replaced with 64-bit versions. ht

Re: RFR: JDK 9: 8024033: [launcher] remove solaris dual mode support

2013-12-12 Thread Kumar Srinivasan
anks Kumar Otherwise, looks fine. Thanks, -Joe On 12/11/2013 01:12 PM, Kumar Srinivasan wrote: On 12/11/2013 12:25 PM, Martin Buchholz wrote: Although I have a small emotional attachment to the idea of fat binaries, there doesn't seem to be too much support for this in the larger java

Re: RFR: JDK9: 8029388: java.exe consumes argument intended for launched java class

2013-12-11 Thread Kumar Srinivasan
Mandy, Thanks for the review. On 12/11/2013 3:21 PM, Kumar Srinivasan wrote: Hello, Please review a fix for Windows launcher where it consumes application args -d32 and -d64, the fix is to stop the scan when it hits the Application sentinel ie. class-name or jar-name. http

RFR: JDK9: 8029388: java.exe consumes argument intended for launched java class

2013-12-11 Thread Kumar Srinivasan
Hello, Please review a fix for Windows launcher where it consumes application args -d32 and -d64, the fix is to stop the scan when it hits the Application sentinel ie. class-name or jar-name. http://cr.openjdk.java.net/~ksrini/8029388/webrev.0/ Thanks Kumar

Re: RFR: JDK 9: 8024033: [launcher] remove solaris dual mode support

2013-12-11 Thread Kumar Srinivasan
.. OK. There's one remaining mention of "LD_LIBRARY_PATH_32". Maybe you want to scrub that as well? 589 (void)UnsetEnv((wanted == 32) ? "LD_LIBRARY_PATH_32" : "LD_LIBRARY_PATH_64"); Good catch I will scrub that. Thanks Martin. Kumar

RFR: JDK 9: 8024033: [launcher] remove solaris dual mode support

2013-12-11 Thread Kumar Srinivasan
Hello Joe, Martin, et. al., In JDK8, solaris 32-bit support was removed entirely, this is the only platform that required dual-mode support, ie. 32-bit and 64-bit binaries co-located in the same binary hierarchy on the disk. In JDK 8 the DUAL_MODE support was disabled in the launcher, using c

Re: RFR: 8023978: [TEST_BUG] launcher tests must exclude platforms without server vm

2013-11-15 Thread Kumar Srinivasan
On 11/15/2013 9:07 AM, David Holmes wrote: On 16/11/2013 2:37 AM, Kumar Srinivasan wrote: Hi David, Hi Kumar, I don't quite see how this gets the "jre" part of a JDK: ! JAVA_JRE_BIN = new File(JAVAHOME, "bin").getAbsolutePath(); ! !

Re: RFR: 8023978: [TEST_BUG] launcher tests must exclude platforms without server vm

2013-11-15 Thread Kumar Srinivasan
On 11/14/2013 7:11 PM, Mandy Chung wrote: On 11/14/2013 6:54 PM, Kumar Srinivasan wrote: New full webrev: http://cr.openjdk.java.net/~ksrini/8023978/webrev.1/index.html Delta webrev wrt. webrev.0 http://cr.openjdk.java.net/~ksrini/8023978/webrev.1/webrev.delta/index.html This looks

Re: RFR: 8023978: [TEST_BUG] launcher tests must exclude platforms without server vm

2013-11-15 Thread Kumar Srinivasan
if it needs one. thanks for pointing that out, I had forgotten all about compilejdk. Kumar Thanks, David On 15/11/2013 12:54 PM, Kumar Srinivasan wrote: Hi Mandy, Thanks for reviewing this, comments inlined. Hi Kumar, On 11/14/2013 2:57 PM, Kumar Srinivasan wrote: Hello, Please r

Re: RFR: 8023978: [TEST_BUG] launcher tests must exclude platforms without server vm

2013-11-14 Thread Kumar Srinivasan
Hi Mandy, Thanks for reviewing this, comments inlined. Hi Kumar, On 11/14/2013 2:57 PM, Kumar Srinivasan wrote: Hello, Please review fixes to the launcher tests, which allows for missing server vm variant. All SE platforms have server vm, however Embedded SE systems do not contain server

RFR: 8023978: [TEST_BUG] launcher tests must exclude platforms without server vm

2013-11-14 Thread Kumar Srinivasan
Hello, Please review fixes to the launcher tests, which allows for missing server vm variant. All SE platforms have server vm, however Embedded SE systems do not contain server, most of them have client, thus these tests needed to be compensated for that. http://cr.openjdk.java.net/~ksrini/802

Re: RFR (XXS) 8027803: NPE in test infrastructure aka: test/sun/reflect/AnonymousNewInstance/ManyNewInstanceAnonTest.java fails

2013-11-10 Thread Kumar Srinivasan
Looks good. Kumar - robert.fi...@oracle.com wrote: > From: robert.fi...@oracle.com > To: core-libs-dev@openjdk.java.net > Cc: joel.fra...@oracle.com, kumar.x.sriniva...@oracle.com, > alan.bate...@oracle.com > Sent: Saturday, November 9, 2013 11:24:40 PM GMT -08:00 US/Canada Pacific >

RFR: JDK-8027227 and JDK-8027232, updates to ASM code base required by modification to java/lang/invoke for JSR-335 conformance

2013-10-31 Thread Kumar Srinivasan
Hi, 1. JDK-8027227, updates the internal ASM code base, for fixes made by ASM team for JSR-335 byte code compliance http://cr.openjdk.java.net/~ksrini/8027227/webrev.0/ 2. JDK-8027232, updates the java/lang/invoke and lambda classes for JSR-335 byte code compliance. http://cr.openjdk.java.n

Re: RFR 8020802: Need an ability to create jar files that are invariant to the pack200 packing/unpacking

2013-10-22 Thread Kumar Srinivasan
Approved. Kumar On 10/22/2013 9:08 AM, Alexander Zuev wrote: Kumar, new version of webrev where i have fixed all the coding style glitches i've found is here: http://cr.openjdk.java.net/~kizune/8020802/webrev.04 /Alex On 10/22/13 19:32, Kumar Srinivasan wrote: Hi Alex, thank

Re: RFR 8020802: Need an ability to create jar files that are invariant to the pack200 packing/unpacking

2013-10-22 Thread Kumar Srinivasan
.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449 Thanks Kumar /Alex On 10/22/13 24:33, Kumar Srinivasan wrote: Alex, The @test should be just TestNormal not TestNormal.java if you need an alternate/small jar you can use one in $JDK/lib/ ex: jconsole.jar or dt

Re: RFR 8020802: Need an ability to create jar files that are invariant to the pack200 packing/unpacking

2013-10-21 Thread Kumar Srinivasan
wrote: On 19/10/2013 16:14, Kumar Srinivasan wrote: Hi Alex, This looks good. Hi Sherman, Alan, Could one of you review this, please The "-n" options seems okay but I wonder if there has any thought given to having an option on jarsigner to normalize it before signing? That was r

Re: RFR: 8026504: [asm] update internal ASM to the Beta version

2013-10-20 Thread Kumar Srinivasan
PING Hello, As per Nashorn teams request, I have also run the octane test, and it passes. I need reviewers from Lambda, Nashorn and ASM projects to approve this. Thanks Kumar Hello, Please review refreshing the internal ASM sources to ASM_5_0_BETA, I have run all the lambda test-ng test

Re: RFR 8020802: Need an ability to create jar files that are invariant to the pack200 packing/unpacking

2013-10-20 Thread Kumar Srinivasan
Hi Alex, This looks good. Hi Sherman, Alan, Could one of you review this, please The "-n" options seems okay but I wonder if there has any thought given to having an option on jarsigner to normalize it before signing? The PM requirement was for jar, in that when a jar is created it sho

Re: RFR 8020802: Need an ability to create jar files that are invariant to the pack200 packing/unpacking

2013-10-20 Thread Kumar Srinivasan
Martin, as for the documentation as to why this is required, I missed adding them last night, this is spelled out in several places since JDK5. See item #4 http://docs.oracle.com/javase/7/docs/technotes/guides/pack200/pack-spec.html#tocGenQue See para #4 http://javaweb.us.oracle.com/java/re

Re: RFR 8020802: Need an ability to create jar files that are invariant to the pack200 packing/unpacking

2013-10-19 Thread Kumar Srinivasan
Hi Martin, I'm not convinced this feature pulls its weight. I'd like to see more motivation/documentation for this change. It seems like this just adds a post-processing phase which packs and unpacks the jar file (why?) Pack200 tends to change the internal structure and ordering within a class

Re: RFR 8020802: Need an ability to create jar files that are invariant to the pack200 packing/unpacking

2013-10-19 Thread Kumar Srinivasan
Hi Alex, This looks good. Hi Sherman, Alan, Could one of you review this, please Thanks Kumar On 10/14/2013 11:44 AM, Alexander Zuev wrote: Hi, please review my fix for 8020802: Need an ability to create jar files that are invariant to the pack200 packing/unpacking The fix adds new optio

RFR-EZ: 8026794: Test tools/pack200/TimeStamp.java fails while opening golden.jar.native.IST on linux-ppc(v2)

2013-10-18 Thread Kumar Srinivasan
Hello, Please review the fix for a ppc failure, a simple ifdef change to use the correct macro for big and little endian switching. Webrev http://cr.openjdk.java.net/~ksrini/8026794/webrev/ Thanks Kumar

Re: RFR: 8026504: [asm] update internal ASM to the Beta version

2013-10-16 Thread Kumar Srinivasan
Hello, As per Nashorn teams request, I have also run the octane test, and it passes. I need reviewers from Lambda, Nashorn and ASM projects to approve this. Thanks Kumar Hello, Please review refreshing the internal ASM sources to ASM_5_0_BETA, I have run all the lambda test-ng tests using

RFR: 8026504: [asm] update internal ASM to the Beta version

2013-10-15 Thread Kumar Srinivasan
Hello, Please review refreshing the internal ASM sources to ASM_5_0_BETA, I have run all the lambda test-ng tests using the lambda repo as well as the nashorn regression tests and test262parallel. Bug: https://bugs.openjdk.java.net/browse/JDK-8026504 Webrev: http://cr.openjdk.java.net/~ksrini

Re: RFR: 8025342: NLS: unsupported translation format in jar/pack/DriverResource.java

2013-10-01 Thread Kumar Srinivasan
Hi Kumar, Just a cosmetic comment. Looks like the description columns at line 103 (and before) and line 104 (and after) do not align to each other. yes indeed, I will fix that. Thanks Naoto! Kumar Otherwise it looks good to me. Naoto On 10/1/13 11:46 AM, Kumar Srinivasan wrote: Hi

RFR: 8025342: NLS: unsupported translation format in jar/pack/DriverResource.java

2013-10-01 Thread Kumar Srinivasan
Hi Naoto, Michael, Please review the webrev below. Apparently using variables to hold the common message strings between pack200(1) and unpack200(1) utilities don't bode well, with the new globalization tools. Thus the request is to remove these common variable and inline these in the messag

Re: RFR: 8016110: Japanese char (MS932) 0x5C cannot be used as an argument when quoted

2013-09-30 Thread Kumar Srinivasan
:59 PM, Kumar Srinivasan wrote: Hi Naoto, Sherman, Akhil, Can you please review this: http://cr.openjdk.java.net/~ksrini/8016110/webrev.0 Bug: http://bugs.sun.com/view_bug.do?bug_id=8016110 The code was provided by IBM, I wrote up the test. Thanks Kumar

Re: RFR: 8025502: Exclude tests due to JDK-8025427

2013-09-26 Thread Kumar Srinivasan
Hi Erik, Looks good!, the bug must be tagged with keyword "noreg-build". Kumar Hi all, I need to exclude some tests due a recent change to jstat that caused to jstat to output the \ufffd for Double.NaN, which confused the tests. I am working on a solution to this, but in the meantime I would

RFR: 8016110: Japanese char (MS932) 0x5C cannot be used as an argument when quoted

2013-09-20 Thread Kumar Srinivasan
Hi Naoto, Sherman, Akhil, Can you please review this: http://cr.openjdk.java.net/~ksrini/8016110/webrev.0 Bug: http://bugs.sun.com/view_bug.do?bug_id=8016110 The code was provided by IBM, I wrote up the test. Thanks Kumar

Re: RFR 8025076: Fix for JDK-8017248 breaks jprt submission for non-unicode locales

2013-09-19 Thread Kumar Srinivasan
Hi Alex, The class can be compiled into the current directory (scratch), this will eliminate: a. the deletion of the files and allow jtreg to clean out the scratch directory b. uses of TEST_CLASSES_DIR.getAbsolutePath(). Thanks Kumar Hi, please review my fix for 8025076: Fix for JDK-

Re: RFR: 8024826: (s) : Remove alt-rt.jar, used by +AggressiveOps"

2013-09-17 Thread Kumar Srinivasan
+1, good riddance to this, which has caused other woes. Thanks! Kumar Hello all; This is a cross-repo patch which disables building and enabling of the alt-rt.jar file. The alt-rt.jar file has been used with the -XX:+AggressiveOpts option (which will be remaining for other optimizations)

Re: RFR: Changes to disable and/or remove Solaris 32-bit from JDK8

2013-09-10 Thread Kumar Srinivasan
th = exe; 205 } I think this should be: 198 if (home.length() > 0) { 201 exePath = home + File.separator + "bin" + File.separator + exe; 203 } else { 204 exePath = exe; 205 } Thanks, /St

Re: RFR: Changes to disable and/or remove Solaris 32-bit from JDK8

2013-09-09 Thread Kumar Srinivasan
hannel/lib/solaris-amd64/libLauncher.so I know this already exist but I thought binaries were disallowed in the open repo? Alan, are the nio changes acceptable? Let me know if you need more time to go over all the changes. Kumar Davud On 9/09/2013 1:09 PM, Kumar Srinivasan wrote: Hi Davi

Re: RFR: Changes to disable and/or remove Solaris 32-bit from JDK8

2013-09-08 Thread Kumar Srinivasan
/sun/tools/jhat/HatRun.java It isn't clear to me why you need to retain the d64 variable at all. --- test/tools/launcher/ExecutionEnvironment.java typo: appopriate Thanks, David On 7/09/2013 2:47 AM, Kumar Srinivasan wrote: Hello, Please review the changes to remove Solaris 32-bi

Re: RFR: Changes to disable and/or remove Solaris 32-bit from JDK8

2013-09-06 Thread Kumar Srinivasan
On 9/6/2013 12:21 PM, Alan Bateman wrote: On 06/09/2013 17:47, Kumar Srinivasan wrote: Hello, Please review the changes to remove Solaris 32-bit binaries from JDK8 distros, at this time the dual mode support in the launcher is being disabled. Message regarding this: http

Re: RFR: Changes to disable and/or remove Solaris 32-bit from JDK8

2013-09-06 Thread Kumar Srinivasan
appy ? this is specific to solaris you are free to have something else that suits your purposes. #ifdef __solaris__ -# define DUAL_MODE +# ifdef DUAL_MODE +#undef DUAL_MODE +# endif Kumar On Fri, Sep 6, 2013 at 9:47 AM, Kumar Srinivasan mailto:kumar.x.sriniva...@oracle.co

RFR: Changes to disable and/or remove Solaris 32-bit from JDK8

2013-09-06 Thread Kumar Srinivasan
Hello, Please review the changes to remove Solaris 32-bit binaries from JDK8 distros, at this time the dual mode support in the launcher is being disabled. Message regarding this: http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-September/003159.html The jdk changes are here: http://cr.op

Re: RFR 8012940 More than 50 tests failed in Serialization/DeSerialization testing (test-mangled)

2013-08-13 Thread Kumar Srinivasan
Hi Paul, Looks good, you can add me as reviewer. Thanks for doing this. Kumar Hi, The updates to stream tests below are in preparation for when the lambda serialization/deserialization tests (SAND tests) transition from testing with lambda to tl. Doing this now so we don't forget later. T

Re: RFR: 8022547: [verifier] move DefaultMethodRegressionTests.java to jdk

2013-08-10 Thread Kumar Srinivasan
ely. Thanks Kumar thanks, Karen On Aug 7, 2013, at 8:25 PM, Kumar Srinivasan wrote: Hello, Testing this functionality in langtools does not seem to be the appropriate location, and the teams have decided to move it to jdk/test/vm/verifier, which seems to be the logical place. I have modif

RFR: 8022547: [verifier] move DefaultMethodRegressionTests.java to jdk

2013-08-07 Thread Kumar Srinivasan
Hello, Testing this functionality in langtools does not seem to be the appropriate location, and the teams have decided to move it to jdk/test/vm/verifier, which seems to be the logical place. I have modified the test to remove testng dependencies, also added a variant which involves loading

Re: RFR: 8015666: test/tools/pack200/TimeStamp.java failing

2013-08-06 Thread Kumar Srinivasan
/28/2013 07:47 AM, Kumar Srinivasan wrote: Some nits while reading the changes: 1. ZipEntry.java a. typo: + * Sets the laste access time of the entry. b. extra space +case EXTID_ZIP64 : 2. ZipOutputStream.java I think it would be nice to have the flags 0x1, 0x2 and 0x4

Re: RFR: 8019799: api/java_util/jar/Pack200 test failed with compactX profiles. (EZ)

2013-07-11 Thread Kumar Srinivasan
Hi David, thanks for reviewing this! On 10/07/2013 6:25 AM, Kumar Srinivasan wrote: Hi, We had earlier added a fail over to the java implementation, for reference: http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/1432a6247ac9 The first time the unpacker is called an UnsatisfiedLinkError will

RFR: 8019799: api/java_util/jar/Pack200 test failed with compactX profiles. (EZ)

2013-07-09 Thread Kumar Srinivasan
Hi, We had earlier added a fail over to the java implementation, for reference: http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/1432a6247ac9 The first time the unpacker is called an UnsatisfiedLinkError will be thrown, but subsequent invocations in the same JDK/VM instance, a NoClassDefFoundErro

Re: RFR: 8017463: [TEST_BUG] 2 tests from tools/pack200/ remain about 1 GB of data in work directory after execution

2013-07-01 Thread Kumar Srinivasan
On 7/1/2013 2:45 PM, Mandy Chung wrote: On 7/1/2013 2:11 PM, Kumar Srinivasan wrote: In Utils.cleanup, it will delete the files with certain extensions in the current directory. I skimmed on the tests and look to me that those files can be altered with a different path and not necessary in

Re: RFR: 8017463: [TEST_BUG] 2 tests from tools/pack200/ remain about 1 GB of data in work directory after execution

2013-07-01 Thread Kumar Srinivasan
On 7/1/2013 11:14 AM, Mandy Chung wrote: On 6/29/13 7:02 PM, Kumar Srinivasan wrote: Hi, Please review changes to cleanup the test area after pack200 test completion. Some tests generally use large jars and a local copy of the JDK to test with, created a generic utility to nuke most of the

Re: JDK 8 code review request for doclint fixes in java.util.jar.Pack200

2013-06-30 Thread Kumar Srinivasan
Hi Joe, The changes looks good, thanks for doing this!. I should've peeked at the generated reports. :-[ But, there are 2 more errors j.u.j.Attributes.java and some missing @throws for j.u.j.JarEntry.java, are you planning on doing this separately ? Thanks Kumar Hello, Pack200 has some doc

RFR: 8017463: [TEST_BUG] 2 tests from tools/pack200/ remain about 1 GB of data in work directory after execution

2013-06-29 Thread Kumar Srinivasan
Hi, Please review changes to cleanup the test area after pack200 test completion. Some tests generally use large jars and a local copy of the JDK to test with, created a generic utility to nuke most of the remains. The webrev: http://cr.openjdk.java.net/~ksrini/8017463/webrev.0/ The bug: http

Re: RFR: 8015666: test/tools/pack200/TimeStamp.java failing

2013-06-28 Thread Kumar Srinivasan
help a casual reader as to what this means. Besides my previous concern with finish(), everything else appears to be ok. Kumar On 06/27/2013 10:04 AM, Kumar Srinivasan wrote: Hi Sherman, I started looking at this, my initial comment, the Unpacker.unpack does not close its output and we allow

Re: RFR: 8015666: test/tools/pack200/TimeStamp.java failing

2013-06-27 Thread Kumar Srinivasan
Hi Sherman, I started looking at this, my initial comment, the Unpacker.unpack does not close its output and we allow multiple pack files to be concatenated, I am assuming out.finish() will allow further jar files to be appended ? or would this cause a problem ? Kumar Hi, The zip time rel

Re: Review request for JDK-8016760: failure of regression test langtools/tools/javac/T6725036.java

2013-06-27 Thread Kumar Srinivasan
If this is to be undone after the correct zip fix, why not add the @ignore for now ? and enable this when 8015666 is fixed correctly. Kumar This is fine to be a workaround for the test case for now. It probably will need to be undo-ed after the propose change for #8015666 get integrated. htt

RFR: JDK-8016209: TEST_BUG: non-compliant jmc in the bin directory hangs testing (XS)

2013-06-07 Thread Kumar Srinivasan
Hi, Can I get a quick review please, the fix excludes j2sdk-image/bin/jmc{.exe} from the test, which invokes the tool and verifies that -version and -Joptions work. As the subject says invoking this tool with any option puts up a window, hanging up the test. Thanks Kumar diff --git a/test/t

RFR: JDK-8015813: add test/tools/pack200/TimeStamp.java to ProblemsList

2013-06-03 Thread Kumar Srinivasan
Hi, Please review adding a failing test to ProblemList, until Sherman and I determine the correct fix for the underlying problem in zip/pack200. Thanks Kumar diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -331,6 +331,8 @@ # Te

Re: Review request for JDK-8014834 : shell tests don't begin with #!/bin/sh

2013-05-30 Thread Kumar Srinivasan
Hi Erik, This looks good, historically the #!shell was inserted after the CopyRight, because it was thought that the automated CR updaters might get confused. I am cc'ing Dave K, just so that he is aware of this. Thanks Kumar Hello, Please review this simple patch which fixes an issue wi

Re: [8] Review Request for 8009911 : [macosx] SWT app freeze when going full screen using Java 7 on Mac

2013-05-28 Thread Kumar Srinivasan
the test? Yes go for it. Kumar With best regards. Petr. On May 24, 2013, at 4:51 AM, Kumar Srinivasan wrote: Hi Petr, Sorry for the delay, I somehow missed this one. I don't understand objective-c, if Mr Petrov is happy I am ok with it. There seems to be indenting issues 1049-1051

Re: RFR: 8007333: [launcher] removes multiple back slashes / trivial fix

2013-05-24 Thread Kumar Srinivasan
.1/webrev.delta/index.html If there are no more comments I will push this by Tuesday. Thanks Kumar On 22/05/2013 21:30, Kumar Srinivasan wrote: Hello, Please review trivial fix where the launcher did not parse multiple back-slashes correctly. I also added some additional test contributed by

Re: [8] Review Request for 8009911 : [macosx] SWT app freeze when going full screen using Java 7 on Mac

2013-05-23 Thread Kumar Srinivasan
Hi Petr, Sorry for the delay, I somehow missed this one. I don't understand objective-c, if Mr Petrov is happy I am ok with it. There seems to be indenting issues 1049-1051 Is there a regression test for this somewhere ? Thanks Kumar Hello, Yes; Kumar should be one of the reviewers here.

<    1   2   3   4   5   >