Re: RFR(S) : 8166262 : failurehandler should not use jtreg internal API

2016-09-19 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

> On 19 sep. 2016, at 11:17, Igor Ignatyev  wrote:
> 
> http://cr.openjdk.java.net/~iignatyev/8166262/webrev.00/
>> 62 lines changed: 56 ins; 2 del; 4 mod;
> 
> Hi all,
> 
> could you please review this small patch which removes usage of jtreg 
> internal API from failurehandler lib?
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8166262
> webrev: http://cr.openjdk.java.net/~iignatyev/8166262/webrev.00/
> 
> Thanks,
> — Igor



Re: RFR : 8132961 : JEP 279 Improve Test-Failure Troubleshooting

2015-12-01 Thread Staffan Larsen
Looks good and sorry for the delay.

Thanks,
/Staffan

> On 24 nov. 2015, at 20:13, Igor Ignatyev  wrote:
> 
> http://cr.openjdk.java.net/~iignatyev/8132961/webrev.00/
>> 3579 lines changed: 3579 ins; 0 del; 0 mod; 0 unchg
> 
> Hi,
> 
> Could you please review the webrev[0] for JEP 279[1]?
> 
> The scope of the JEP is an implementation of a library which uses jtreg 
> timeout handler and observer extension points to collect information about 
> environment in case of test failures (including timeouts) and about test 
> processes in case of timeouts. This data is then presented together with the 
> test failure to simplify analysis.
> 
> To make it easier to specify which tools should be run by the library on 
> which platform when a test failure or timeout occurs, we use properties files 
> to configure the library. Each platform family uses its own property file 
> (named .properties) and common.properties, which contains platform 
> independent tools, such as jps. Using property files allows to easily extend 
> the tools that are used to collect information on test failure or timeout in 
> the future. See the JEP for a more thorough overview of the collected data. 
> Currently, we are using the following tools:
> - on all platforms[3]: jps, jstack, jmap, jinfo, jcmd
> - on linux[4]: ps, pmap, lsof, lslocks, gdb, gcore, id, who, last, df, env, 
> dmesg, sysctl, top, free, vmstat, netstat
> - on solaris[5]: pgrep, pmap, pfiles, pstack, gcore, id, who, last, df, env, 
> dmesg, prtconf, sysdef, swap, ps, top, vmstat, pagesize, netstat
> - on mac[6]: pgrep, vmmap, heap, leaks, spindump, lldb, gcore, id, who, last, 
> df, env, dmesg, sysctl, ps, top, vm_stat, netstat
> - on windows[7]: wmic, pmap, handle, cdb, id, who, last, df, env, powershell, 
> tasklist, ps, top, free, vmstat, openfiles, netstat
> 
> More information can be found in the JEP[1] and README[2].
> 
> The library integration into makefiles will be done later as the fix for 
> JDK-8132962[8].
> 
> [0] http://cr.openjdk.java.net/~iignatyev/8132961/webrev.00/
> [1] https://bugs.openjdk.java.net/browse/JDK-8075621
> [2] 
> http://cr.openjdk.java.net/~iignatyev/8132961/webrev.00/test/failure_handler/README.html
> [3] 
> http://cr.openjdk.java.net/~iignatyev/8132961/webrev.00/test/failure_handler/src/share/conf/common.properties.html
> [4] 
> http://cr.openjdk.java.net/~iignatyev/8132961/webrev.00/test/failure_handler/src/share/conf/linux.properties.html
> [5] 
> http://cr.openjdk.java.net/~iignatyev/8132961/webrev.00/test/failure_handler/src/share/conf/solaris.properties.html
> [6] 
> http://cr.openjdk.java.net/~iignatyev/8132961/webrev.00/test/failure_handler/src/share/conf/mac.properties.html
> [7] 
> http://cr.openjdk.java.net/~iignatyev/8132961/webrev.00/test/failure_handler/src/share/conf/windows.properties.html
> [8] https://bugs.openjdk.java.net/browse/JDK-8132962
> 
> Thanks,
> — Igor



Re: [preview] Adding java.lang.Runtime.getVMArguments() method

2015-11-25 Thread Staffan Larsen
Also remember that that java launcher is not the only launcher used with the 
JVM. There are lots of apps that use the invocation API to start the JVM, and 
there is no standardized way to propagate the command line or even the 
Main-Class to the JVM. This is currently an outage in serviceability tools such 
as jps and jcmd. 

https://bugs.openjdk.java.net/browse/JDK-6174973: 
 "Process names don't appear 
in jps output when JNI is used to launch”
https://bugs.openjdk.java.net/browse/JDK-6456333: 
 "Incorrect jar name handle 
by jps in case java -jar.”
https://bugs.openjdk.java.net/browse/JDK-7012002: 
 "JPS output is wrong”

Thanks,
/Staffan

> On 25 nov. 2015, at 03:53, Roger Riggs  wrote:
> 
> Hi,
> 
> The ProcessHandle.current().info().arguments() has the operating system's view
> of the command and arguments.  Which may differ from what is passed to main[] 
> args.
> Perhaps for current() it should be special cased to exactly and always match 
> main(args)
> but that's a different question.
> 
> If the desired set of arguments is different from what is typed on the command
> line or is passed to main, the perhaps it should remain in RuntimeMXBean.
> A method on Runtime to return the arguments would be understood to be
> more general and match either main(args) or the true command line as typed.
> 
> Roger
> 
> 
> On 11/24/15 9:24 PM, David Holmes wrote:
>> On 25/11/2015 10:06 AM, Mandy Chung wrote:
>>> 
 On Nov 24, 2015, at 3:45 PM, Peter Levart  wrote:
 
 
 
 On 11/24/2015 05:49 PM, Jaroslav Bachorik wrote:
> Hi,
> 
> while working on an issue to clean up a code in java.base module using 
> reflection to access RuntimeMXBean (from java.management module) in order 
> to get hold of the VM arguments (yes, this won't work with module 
> boundaries in place) it was pointed out that this functionality should be 
> available in java.base without going through JMX.
 
 Isn't the following JDK9 API already providing that:
 
 ProcessHandle.current().info().arguments();
>>> 
>>> This is what I also start going after.
>>> 
>>> The launcher does some job on the command-line before passing to the VM, 
>>> e.g. @argfile support that expands the options specified in the file, add 
>>> -Djava.class.path and some system properties passing to the VM, take out -J 
>>> if they are JDK tool launchers etc.
>> 
>> I haven't looked at the two APIs but the command-line is potentially very 
>> different from the "VM arguments". The VM can get its arguments from the 
>> command-line, the launcher, options file and environment variables.
>> 
>> David
>> 
>>> Mandy
>>> 
> 



Re: JDK 9 RFR of JDK-8143583: Several tests don't work with latest jtreg due to non-existing files in @build

2015-11-22 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

> On 23 nov. 2015, at 08:10, Amy Lu  wrote:
> 
> Below tests failed with latest nightly jtreg due to non-existing files in 
> @build
> 
> com/sun/management/HotSpotDiagnosticMXBean/DumpHeap.java
> sun/tools/jmap/BasicJMapTest.java
> com/sun/jdi/DoubleAgentTest.java
> com/sun/jdi/SuspendNoFlagTest.java
> 
> Please review this patch to fix the typo in @build
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8143583
> webrev: http://cr.openjdk.java.net/~amlu/8143583/webrev.00/
> 
> Thanks,
> Amy
> 
> 
> --- old/test/com/sun/jdi/DoubleAgentTest.java2015-11-23 
> 13:36:04.0 +0800
> +++ new/test/com/sun/jdi/DoubleAgentTest.java2015-11-23 
> 13:36:04.0 +0800
> @@ -31,7 +31,7 @@
>  *
>  * @library /lib/testlibrary
>  * @modules java.management
> - * @build jdk.testlibarary.*
> + * @build jdk.testlibrary.*
>  * @build DoubleAgentTest Exit0
>  * @run driver DoubleAgentTest
>  */
> --- old/test/com/sun/jdi/SuspendNoFlagTest.java  2015-11-23 
> 13:36:06.0 +0800
> +++ new/test/com/sun/jdi/SuspendNoFlagTest.java  2015-11-23 
> 13:36:05.0 +0800
> @@ -29,7 +29,7 @@
>  * @summary Test for JDWP: -agentlib:jdwp=suspend=n hanging
>  * @library /lib/testlibrary
>  * @modules java.management
> - * @build jdk.testlibarary.*
> + * @build jdk.testlibrary.*
>  * @compile -g HelloWorld.java
>  * @run driver SuspendNoFlagTest
>  */
> --- old/test/com/sun/management/HotSpotDiagnosticMXBean/DumpHeap.java  
> 2015-11-23 13:36:07.0 +0800
> +++ new/test/com/sun/management/HotSpotDiagnosticMXBean/DumpHeap.java  
> 2015-11-23 13:36:07.0 +0800
> @@ -41,9 +41,9 @@
>  * @library /test/lib/share/classes
>  * @build jdk.testlibrary.*
>  * @build jdk.test.lib.hprof.*
> - * @build jdk.test.lib.hprof.module.*
> + * @build jdk.test.lib.hprof.model.*
>  * @build jdk.test.lib.hprof.parser.*
> - * @build jdk.test.lib.hprof.utils.*
> + * @build jdk.test.lib.hprof.util.*
>  * @run main DumpHeap
>  */
> public class DumpHeap {
> --- old/test/sun/tools/jmap/BasicJMapTest.java  2015-11-23 13:36:08.0 
> +0800
> +++ new/test/sun/tools/jmap/BasicJMapTest.java  2015-11-23 13:36:08.0 
> +0800
> @@ -42,9 +42,9 @@
>  * @modules java.management
>  * @build jdk.testlibrary.*
>  * @build jdk.test.lib.hprof.*
> - * @build jdk.test.lib.hprof.module.*
> + * @build jdk.test.lib.hprof.model.*
>  * @build jdk.test.lib.hprof.parser.*
> - * @build jdk.test.lib.hprof.utils.*
> + * @build jdk.test.lib.hprof.util.*
>  * @run main/timeout=240 BasicJMapTest
>  */
> public class BasicJMapTest {
> 
> 
> 
> 



Re: [RFR] (XS) 8141489: [TESTBUG] requiredVersion in TEST.ROOT needs to updated to 4.1 b12

2015-11-04 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

> On 5 nov. 2015, at 03:25, Chris Plummer  wrote:
> 
> Please review the following changes:
> 
> http://cr.openjdk.java.net/~cjplummer/8141489/
> https://bugs.openjdk.java.net/browse/JDK-8141489
> 
> The changes I did for 8140189 require that version 4.1 b12 of jtreg be used, 
> so TEST.ROOT has been updated to reflect this.
> 
> Testing with JPRT right now. I also ran with b11 and confirmed that the 
> following errors are produced for hotspot and jdk tests:
> 
>Error: The testsuite at /local/ws/jdk9/hs-rt.8141489/jdk/test requires 
> jtreg version 4.1 b12 or higher and this is jtreg version 4.1 b11.
> 
>Error: The testsuite at /local/ws/jdk9/hs-rt.8141489/hotspot/test requires 
> jtreg version 4.1 b12 or higher and this is jtreg version 4.1 b11.
> 
> thanks,
> 
> Chris



Re: [RFR] (S) 8140189: [TESTBUG] Get rid of "@library /../../test/lib" in jtreg tests

2015-10-29 Thread Staffan Larsen
Looks good (still)!

Thanks,
/Staffan
> On 28 okt. 2015, at 06:42, Chris Plummer <chris.plum...@oracle.com> wrote:
> 
> Hello,
> 
> I've fixed the new jvmci tests. New webrevs found here:
> 
> http://cr.openjdk.java.net/~cjplummer/8140189/webrev.01/webrev.hotspot
> http://cr.openjdk.java.net/~cjplummer/8140189/webrev.01/webrev.jdk
> 
> Only the hotspot/test/compiler/jvmci files have changed since the previous 
> webrev. If you just want to look at a patch of those changes, they can be 
> found here:
> 
> http://cr.openjdk.java.net/~cjplummer/8140189/webrev.01/jvmci.patch
> 
> The changes were straight forward. In all cases for the jvmci tests the diff 
> is:
> 
> - * @library / /testlibrary /../../test/lib
> + * @library / /testlibrary /test/lib
> 
> thanks,
> 
> Chris
> 
> On 10/26/15 3:19 PM, Ioi Lam wrote:
>> Hi Chris,
>> 
>> Your changes look good to me. I think it's better to fix the jvmci tests as 
>> well in a single push.
>> 
>> Thanks
>> - Ioi
>> 
>> On 10/26/15 2:13 PM, Chris Plummer wrote:
>>> I just pulled the latest hs-rt, and got about 30 new jvmci tests that are 
>>> using "/../../test/lib". I can fix them with this push, or file a separate 
>>> bug or send out a fix after I do this push. If I fix with this push, do you 
>>> want another review? I'll test with jprt and run the jvmci tests locally.
>>> 
>>> thanks,
>>> 
>>> Chris
>>> 
>>> 
>>> On 10/23/15 7:50 AM, Staffan Larsen wrote:
>>>> Looks good! Thanks for doing this.
>>>> 
>>>> /Staffan
>>>> 
>>>>> On 23 okt. 2015, at 07:54, Chris Plummer <chris.plum...@oracle.com> wrote:
>>>>> 
>>>>> Hello,
>>>>> 
>>>>> Please review the following fix for 8140189:
>>>>> 
>>>>> http://cr.openjdk.java.net/~cjplummer/8140189/webrev.00/webrev.hotspot
>>>>> http://cr.openjdk.java.net/~cjplummer/8140189/webrev.00/webrev.jdk
>>>>> 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8140189
>>>>> 
>>>>> Please also see the following CR, which has much more extensive 
>>>>> discussion of the problem:
>>>>> 
>>>>> jtreg produces class files outside the JTwork directory
>>>>> https://bugs.openjdk.java.net/browse/CODETOOLS-7901527
>>>>> 
>>>>> All the diffs for the tests simply replace "/../../test/lib" with 
>>>>> "/test/lib". The changes in TEST.ROOT are what allow this. It is probably 
>>>>> much easier to look at the patch than to look at each file in the webrev. 
>>>>> All the test diffs look pretty much like the following:
>>>>> 
>>>>> - * @library /testlibrary /../../test/lib
>>>>> + * @library /testlibrary /test/lib
>>>>> 
>>>>> or
>>>>> 
>>>>> - * @library /../../test/lib/share/classes
>>>>> + * @library /test/lib/share/classes
>>>>> 
>>>>> Tested with jprt. Also ran the following jtreg tests on a linux/x64 host 
>>>>> with a fastdebug build:
>>>>> 
>>>>> -Ran all hotspot jtreg tests.
>>>>> -Ran all modified jdk jtreg tests.
>>>>> -Ran jdk tier1 and tier2 jtreg tests.
>>>>> 
>>>>> There were some failures and errors, but they were replicated when 
>>>>> testing with a clean repo also and are unrelated to my changes.
>>>>> 
>>>>> thanks,
>>>>> 
>>>>> Chris
>>>>> 
>>> 
>> 
> 



Re: [RFR] (S) 8140189: [TESTBUG] Get rid of "@library /../../test/lib" in jtreg tests

2015-10-23 Thread Staffan Larsen
Looks good! Thanks for doing this.

/Staffan

> On 23 okt. 2015, at 07:54, Chris Plummer  wrote:
> 
> Hello,
> 
> Please review the following fix for 8140189:
> 
> http://cr.openjdk.java.net/~cjplummer/8140189/webrev.00/webrev.hotspot
> http://cr.openjdk.java.net/~cjplummer/8140189/webrev.00/webrev.jdk
> 
> https://bugs.openjdk.java.net/browse/JDK-8140189
> 
> Please also see the following CR, which has much more extensive discussion of 
> the problem:
> 
> jtreg produces class files outside the JTwork directory
> https://bugs.openjdk.java.net/browse/CODETOOLS-7901527
> 
> All the diffs for the tests simply replace "/../../test/lib" with 
> "/test/lib". The changes in TEST.ROOT are what allow this. It is probably 
> much easier to look at the patch than to look at each file in the webrev. All 
> the test diffs look pretty much like the following:
> 
> - * @library /testlibrary /../../test/lib
> + * @library /testlibrary /test/lib
> 
> or
> 
> - * @library /../../test/lib/share/classes
> + * @library /test/lib/share/classes
> 
> Tested with jprt. Also ran the following jtreg tests on a linux/x64 host with 
> a fastdebug build:
> 
> -Ran all hotspot jtreg tests.
> -Ran all modified jdk jtreg tests.
> -Ran jdk tier1 and tier2 jtreg tests.
> 
> There were some failures and errors, but they were replicated when testing 
> with a clean repo also and are unrelated to my changes.
> 
> thanks,
> 
> Chris
> 



Re: RFR(M, v14): JDK-8059036 : Implement Diagnostic Commands for heap and finalizerinfo

2015-06-05 Thread Staffan Larsen
Dmitry,

I’d like to propose the following change to get prettier output (more in line 
with GC.class_histogram):

diff --git a/src/share/vm/services/diagnosticCommand.cpp 
b/src/share/vm/services/diagnosticCommand.cpp
--- a/src/share/vm/services/diagnosticCommand.cpp
+++ b/src/share/vm/services/diagnosticCommand.cpp
@@ -341,7 +341,6 @@
 void FinalizerInfoDCmd::execute(DCmdSource source, TRAPS) {
   ResourceMark rm;

-  output()-print_cr(Unreachable instances awaiting finalization:);
   Klass* k = SystemDictionary::resolve_or_null(
 vmSymbols::finalizer_histogram_klass(), THREAD);
   assert(k != NULL, FinalizerHistogram class is not accessible);
@@ -375,12 +374,15 @@

   assert(count_res != NULL  name_res != NULL, Unexpected layout of 
FinalizerHistogramEntry);

+  output()-print_cr(Unreachable instances waiting for finalization);
+  output()-print_cr(#instances  class name);
+  output()-print_cr(---);
   for (int i = 0; i  result_oop-length(); ++i) {
 oop element_oop = result_oop-obj_at(i);
 oop str_oop = element_oop-obj_field(name_fd.offset());
 char *name = java_lang_String::as_utf8_string(str_oop);
 int count = element_oop-int_field(count_fd.offset());
-output()-print_cr(Class %s - %d, name, count);
+output()-print_cr(%10d  %s, count, name);
   }
 }


A couple of nits:

diagnosticCommand.cpp:359 - extra space after =
diagnosticCommand.cpp:361 - spelling: finlalization - finalization
FinalizerInfoTest.java:69: - spelling: intialized - initialized
FinalizerInfoTest.java:71 - I’d like to see the ; on a new line


Thanks,
/Staffan


 On 5 jun 2015, at 00:20, Mandy Chung mandy.ch...@oracle.com wrote:
 
 
 On Jun 4, 2015, at 8:49 AM, Dmitry Samersoff dmitry.samers...@oracle.com 
 wrote:
 
 Mandy,
 
 Webrev updated in-place (press shift-reload).
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.14
 
 getFinalizerHistogram() now returns Entry[]
 
 @library and @build removed from the test.
 
 
 Looks fine.
 
 Thanks
 Mandy
 
 
 -Dmitry
 
 On 2015-06-04 16:56, Mandy Chung wrote:
 
 On Jun 4, 2015, at 6:38 AM, Dmitry Samersoff dmitry.samers...@oracle.com 
 wrote:
 
 Mandy,
 
 Thank you for the review.
 
 Updated webrev is:
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.14
 
 
 Thanks for the update and the test.
 
 addressed comments, added a unit test to JDK workspace.
 
 
 This test does not need @library and @build, right?  
 
 
 On 2015-06-03 21:36, Mandy Chung wrote:
 
 Should getFinalizerHistogram() return FinalizerHistogramEntry[]?
 The VM knows the returned type anyway.
 
 [java/lang/ref/Entry signature would need a separate symbol entry in
 swollen vmSymbols::init() so I would prefer to stay with more generic
 Object[]
 
 
 You already add several symbols - why is it an issue for another one?  Or 
 if adding vm symbols is a concern, you should revert to String and int[] 
 that you decide not to.   The decision should apply consistently (use 
 String and int, or new Java type).
 
 
 Perhaps the getFinalizerHistogram method should be renamed
 e.g. dump?
 
 The line in vmSymbols looks like:
 
 template(
 get_finalizer_histogram_name, getFinalizerHistogram)
 
 I would prefer to keep method name specific enough to be able to
 find it by grep in jdk code.
 
 
 Grep in jdk code is easy as you have a new class :)  
 
 Mandy
 
 
 (other comments are addressed)
 
 -Dmitry
 
 
 On 2015-06-03 21:36, Mandy Chung wrote:
 
 
 On 06/03/2015 08:29 AM, Dmitry Samersoff wrote:
 Updated webrev:
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.13
 
 I reviewed the jdk change.  The version looks okay and some comments:
 
 ReferenceQueue.java - you should eliminate the use of rawtype:
 
 84 Reference rn = r.next;
 
 Change Reference to Reference? extends T
 
 done.
 
 
 The forEach method - eliminate the use of raw type and
 move @SuppressWarning to the field variable in line 182:
 
@SuppressWarnings(unchecked)
Reference? extends T rn = r.next;
 
 done.
 
 
 FinalizerHistogram.java
 Copyright year needs update.
 
 done.
 
 I personally think the VM code would be much simpler if you stay with
 alternative entry of String and int than dealing with a
 FinalizerHistogramEntry private class.  It's okay as FinalizerHistogram
 class is separated.
 
 The comment in line 35 is suitable to move to the class description and
 this is the suggestion.
 
  // This FinalizerHistogram class is for GC.finalizer_info diagnostic
 command support.
  // It is invoked by the VM.
 
 done.
 
 
 Should getFinalizerHistogram() return FinalizerHistogramEntry[]? The VM
 knows the returned type anyway.  
 
 [java/lang/ref/Entry signature would need a separate symbol entry in
 swollen vmSymbols::init() so I would prefer to stay with more generic
 Object[]
 
 It's an inner class of
 FinalizerHistogram and it can simply be named as Entry.   I suggest to
 have Entry::increment method to replace .instanceCount++.
 
 done.
 
 
 The tests are in 

Re: RFR(M, v14): JDK-8059036 : Implement Diagnostic Commands for heap and finalizerinfo

2015-06-05 Thread Staffan Larsen
Thanks - this version looks good to me.

/Staffan

 On 5 jun 2015, at 13:00, Dmitry Samersoff dmitry.samers...@oracle.com wrote:
 
 Staffan,
 
 Thank you for review!
 
 Done. Webrev updated in-place (press shift-reload).
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.14
 
 -Dmitry
 
 On 2015-06-05 11:20, Staffan Larsen wrote:
 Dmitry,
 
 I’d like to propose the following change to get prettier output (more in 
 line with GC.class_histogram):
 
 diff --git a/src/share/vm/services/diagnosticCommand.cpp 
 b/src/share/vm/services/diagnosticCommand.cpp
 --- a/src/share/vm/services/diagnosticCommand.cpp
 +++ b/src/share/vm/services/diagnosticCommand.cpp
 @@ -341,7 +341,6 @@
 void FinalizerInfoDCmd::execute(DCmdSource source, TRAPS) {
   ResourceMark rm;
 
 -  output()-print_cr(Unreachable instances awaiting finalization:);
   Klass* k = SystemDictionary::resolve_or_null(
 vmSymbols::finalizer_histogram_klass(), THREAD);
   assert(k != NULL, FinalizerHistogram class is not accessible);
 @@ -375,12 +374,15 @@
 
   assert(count_res != NULL  name_res != NULL, Unexpected layout of 
 FinalizerHistogramEntry);
 
 +  output()-print_cr(Unreachable instances waiting for finalization);
 +  output()-print_cr(#instances  class name);
 +  output()-print_cr(---);
   for (int i = 0; i  result_oop-length(); ++i) {
 oop element_oop = result_oop-obj_at(i);
 oop str_oop = element_oop-obj_field(name_fd.offset());
 char *name = java_lang_String::as_utf8_string(str_oop);
 int count = element_oop-int_field(count_fd.offset());
 -output()-print_cr(Class %s - %d, name, count);
 +output()-print_cr(%10d  %s, count, name);
   }
 }
 
 
 A couple of nits:
 
 diagnosticCommand.cpp:359 - extra space after =
 diagnosticCommand.cpp:361 - spelling: finlalization - finalization
 FinalizerInfoTest.java:69: - spelling: intialized - initialized
 FinalizerInfoTest.java:71 - I’d like to see the ; on a new line
 
 
 Thanks,
 /Staffan
 
 
 On 5 jun 2015, at 00:20, Mandy Chung mandy.ch...@oracle.com wrote:
 
 
 On Jun 4, 2015, at 8:49 AM, Dmitry Samersoff dmitry.samers...@oracle.com 
 wrote:
 
 Mandy,
 
 Webrev updated in-place (press shift-reload).
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.14
 
 getFinalizerHistogram() now returns Entry[]
 
 @library and @build removed from the test.
 
 
 Looks fine.
 
 Thanks
 Mandy
 
 
 -Dmitry
 
 On 2015-06-04 16:56, Mandy Chung wrote:
 
 On Jun 4, 2015, at 6:38 AM, Dmitry Samersoff 
 dmitry.samers...@oracle.com wrote:
 
 Mandy,
 
 Thank you for the review.
 
 Updated webrev is:
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.14
 
 
 Thanks for the update and the test.
 
 addressed comments, added a unit test to JDK workspace.
 
 
 This test does not need @library and @build, right?  
 
 
 On 2015-06-03 21:36, Mandy Chung wrote:
 
 Should getFinalizerHistogram() return FinalizerHistogramEntry[]?
 The VM knows the returned type anyway.
 
 [java/lang/ref/Entry signature would need a separate symbol entry in
 swollen vmSymbols::init() so I would prefer to stay with more generic
 Object[]
 
 
 You already add several symbols - why is it an issue for another one?  Or 
 if adding vm symbols is a concern, you should revert to String and int[] 
 that you decide not to.   The decision should apply consistently (use 
 String and int, or new Java type).
 
 
 Perhaps the getFinalizerHistogram method should be renamed
 e.g. dump?
 
 The line in vmSymbols looks like:
 
 template(
 get_finalizer_histogram_name, getFinalizerHistogram)
 
 I would prefer to keep method name specific enough to be able to
 find it by grep in jdk code.
 
 
 Grep in jdk code is easy as you have a new class :)  
 
 Mandy
 
 
 (other comments are addressed)
 
 -Dmitry
 
 
 On 2015-06-03 21:36, Mandy Chung wrote:
 
 
 On 06/03/2015 08:29 AM, Dmitry Samersoff wrote:
 Updated webrev:
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.13
 
 I reviewed the jdk change.  The version looks okay and some comments:
 
 ReferenceQueue.java - you should eliminate the use of rawtype:
 
 84 Reference rn = r.next;
 
 Change Reference to Reference? extends T
 
 done.
 
 
 The forEach method - eliminate the use of raw type and
 move @SuppressWarning to the field variable in line 182:
 
   @SuppressWarnings(unchecked)
   Reference? extends T rn = r.next;
 
 done.
 
 
 FinalizerHistogram.java
 Copyright year needs update.
 
 done.
 
 I personally think the VM code would be much simpler if you stay with
 alternative entry of String and int than dealing with a
 FinalizerHistogramEntry private class.  It's okay as FinalizerHistogram
 class is separated.
 
 The comment in line 35 is suitable to move to the class description and
 this is the suggestion.
 
 // This FinalizerHistogram class is for GC.finalizer_info diagnostic
 command support.
 // It is invoked by the VM.
 
 done.
 
 
 Should getFinalizerHistogram() return FinalizerHistogramEntry[]? The VM
 knows

Re: JDK 9 RFR of JDK-8081775: two lib/testlibrary tests are failing with Error. failed to clean up files after test with jtreg 4.1 b12

2015-06-03 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

 On 3 jun 2015, at 05:31, Amy Lu amy...@oracle.com wrote:
 
 lib/testlibrary/OutputAnalyzerReportingTest.java
 lib/testlibrary/OutputAnalyzerTest.java
 
 These tests fail with jtreg4.1/b12 because jtreg4.1/b12 adds stricter 
 checking of @library tags and the library directory defined in the test 
 doesn't exist.
 
 Please review the fix, removed the unneeded @library.
 
 bug: https://bugs.openjdk.java.net/browse/JDK-8081775
 webrev: http://cr.openjdk.java.net/~amlu/8081775/webrev.00/
 
 
 --- old/test/lib/testlibrary/OutputAnalyzerReportingTest.java  2015-06-03 
 10:48:57.0 +0800
 +++ new/test/lib/testlibrary/OutputAnalyzerReportingTest.java  2015-06-03 
 10:48:57.0 +0800
 @@ -28,7 +28,6 @@
  * @summary Test the OutputAnalyzer reporting functionality,
  * such as printing additional diagnostic info
  * (exit code, stdout, stderr, command line, etc.)
 - * @library /testlibrary
  * @modules java.management
  * @build jdk.testlibrary.*
  * @run main jdk.testlibrary.OutputAnalyzerReportingTest
 --- old/test/lib/testlibrary/OutputAnalyzerTest.java  2015-06-03 
 10:48:58.0 +0800
 +++ new/test/lib/testlibrary/OutputAnalyzerTest.java  2015-06-03 
 10:48:58.0 +0800
 @@ -25,7 +25,6 @@
 /*
  * @test
  * @summary Test the OutputAnalyzer utility class
 - * @library /testlibrary
  * @modules java.management
  * @build jdk.testlibrary.*
  * @run main jdk.testlibrary.OutputAnalyzerTest
 
 
 Thanks,
 Amy
 



Re: RFR(M, v10): JDK-8059036 : Implement Diagnostic Commands for heap and finalizerinfo

2015-06-01 Thread Staffan Larsen
Dmitry,

Instead of hardcoding the field offsets, you can use InstanceKlass::find_field 
and fieldDescriptor::offset to find the offset at runtime. 

Thanks,
/Staffan

 On 31 maj 2015, at 13:43, Dmitry Samersoff dmitry.samers...@oracle.com 
 wrote:
 
 Everyone,
 
 Please take a look at new version of the fix.
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.10/
 
 Changes (to previous version) are in
 Finalizer.java and diagnosticCommand.cpp
 
 This version copy data from Map.Entry to array of
 FinalizerHistogramEntry instances then,
 VM prints content of this array.
 
 -Dmitry
 
 
 On 2015-05-28 21:06, Mandy Chung wrote:
 
 On 05/28/2015 07:35 AM, Peter Levart wrote:
 Hi Mandy,
 
 On 05/27/2015 03:32 PM, Mandy Chung wrote:
 Taking it further - is it simpler to return String[] of all
 classnames including the duplicated ones and have the VM do the
 count?  Are you concerned with the size of the String[]?
 
 Yes, the histogram is much smaller than the list of all instances.
 There can be millions of instances waiting in finalizer queue, but
 only a few distinct classes.
 
 Do you happen to know what libraries are the major contributors to these
 millions of finalizers?
 
 It has been strongly recommended to avoid finalizers (see Effective Java
 Item 7).   I'm not surprised that existing code is still using
 finalizers while we should really encourage them to migrate away from it.
 
 What could be done in Java to simplify things in native code but still
 not format the output is to convert the array of Map.Entry(s) into an
 Object[] array of alternating {String, int[], String, int[],  }
 
 Would this simplify native code for the price of a little extra work
 in Java? The sizes of old and new arrays are not big (# of distinct
 classes of finalizable objects in the queue).
 
 I also prefer writing in Java and writing less native code (much
 simplified).  It's an interface that we have to agree upon and keep it
 simple.  Having the returned Object[] as alternate String and int[] is a
 reasonable compromise.
 
 ReferenceQueue.java - you can move @SuppressWarning from the method to
 just the field variable rn
 @SuppressWarnings(unchecked)
 Reference? extends T rn = r.next;
 
 Finalizer.java
 It's better to rename printFinalizationQueue as it inspects the
 finalizer reference queue (maybe getFinalizerHistogram?).  Can you move
 this method to the end of this class and add the comment saying that
 this is invoked by VM for jcmd -finalizerinfo support and @return to
 describe the returned content.  I also think you can remove
 @SuppressWarnings for this method.
 
 Mandy
 
 
 -- 
 Dmitry Samersoff
 Oracle Java development team, Saint Petersburg, Russia
 * I would love to change the world, but they won't give me the sources.



Re: RFR(M, v9): JDK-8059036 : Implement Diagnostic Commands for heap and finalizerinfo

2015-05-28 Thread Staffan Larsen

 On 28 maj 2015, at 20:06, Mandy Chung mandy.ch...@oracle.com wrote:
 
 
 On 05/28/2015 07:35 AM, Peter Levart wrote:
 Hi Mandy,
 
 On 05/27/2015 03:32 PM, Mandy Chung wrote:
 Taking it further - is it simpler to return String[] of all classnames 
 including the duplicated ones and have the VM do the count?  Are you 
 concerned with the size of the String[]?
 
 Yes, the histogram is much smaller than the list of all instances. There can 
 be millions of instances waiting in finalizer queue, but only a few distinct 
 classes.
 
 Do you happen to know what libraries are the major contributors to these 
 millions of finalizers?
 
 It has been strongly recommended to avoid finalizers (see Effective Java Item 
 7).   I'm not surprised that existing code is still using finalizers while we 
 should really encourage them to migrate away from it.

Having ways to introspect the finalizer queue is one way to make people aware 
that they have a problem :-)

 
 What could be done in Java to simplify things in native code but still not 
 format the output is to convert the array of Map.Entry(s) into an Object[] 
 array of alternating {String, int[], String, int[],  }
 
 Would this simplify native code for the price of a little extra work in 
 Java? The sizes of old and new arrays are not big (# of distinct classes of 
 finalizable objects in the queue).
 
 I also prefer writing in Java and writing less native code (much simplified). 
  It's an interface that we have to agree upon and keep it simple.  Having the 
 returned Object[] as alternate String and int[] is a reasonable compromise.
 
 ReferenceQueue.java - you can move @SuppressWarning from the method to just 
 the field variable rn
 @SuppressWarnings(unchecked)
 Reference? extends T rn = r.next;
 
 Finalizer.java
 It's better to rename printFinalizationQueue as it inspects the finalizer 
 reference queue (maybe getFinalizerHistogram?).  Can you move this method to 
 the end of this class and add the comment saying that this is invoked by VM 
 for jcmd -finalizerinfo support and @return to describe the returned content. 
  I also think you can remove @SuppressWarnings for this method.
 
 Mandy



Re: RFR(M, v7): JDK-8059036 : Implement Diagnostic Commands for heap and finalizerinfo

2015-05-20 Thread Staffan Larsen
/
 
 Now it iterates over queue and output result sorted by number of 
 instances.
 
 -Dmitry
 
 On 2015-05-07 00:51, Derek White wrote:
 Hi Dmitry, Staffan,
 
 Lots of good comments here.
 
 On the topic of what list should be printed out, I think we should 
 focus
 on objects waiting to be finalized - e.g. the contents of the
 ReferenceQueue. It's more of a pain to walk the ReferenceQueue, but 
 you
 could add a summerizeQueue(TreeMapString, Integer) method, or a
 general iterator and lambda.
 
 A histogram of objects with finalize methods might also be 
 interesting,
 but you can get most of the same information from a heap histogram
 (ClassHistogramDCmd, and search for count of Finalizer instances).
 
 BTW, by only locking the ReferenceQueue, we should only be blocking 
 the
 FinalizerThread from making progress (and I think there's a GC 
 thread
 that runs after GC that sorts found References objects that need
 processing into their respective ReferenceQueues). But locking the
 unfinalized list blocks initializing any object with a finalize() 
 method.
 
 The sorting suggestion is a nice touch.
 
 - Derek White, GC team
 
 On 5/5/15 10:40 AM, Peter Levart wrote:
 Hi Dmitry, Staffan,
 
 On 05/05/2015 12:38 PM, Staffan Larsen wrote:
 Dmitry,
 
 I think this should be reviewed on hotspot-gc and core-libs-dev as
 well considering the changes to Finalizer.
 
 I’m a little worried about the potentially very large String that 
 is
 returned from printFinalizationQueue(). A possible different 
 approach
 would be to write printFinalizationQueue() in C++ inside Hotspot.
 That would allow for outputting one line at a time. The output 
 would
 still be saved in memory (since the stream is buffered), but at 
 least
 the data is only saved once in memory, then. It would make the 
 code a
 bit harder to write, so its a question of how scared we are of
 running out of memory.
 If the output is just a histogram of # of instances per class name,
 then it should not be too large, as there are not many different
 classes overriding finalize() method (I counted 72 overriddings of
 finalize() method in the whole jdk/src tree).
 
 I'm more concerned about the fact that while traversing the list, a
 lock is held, which might impact prompt finalization(). Is it
 acceptable for diagnostic output to impact performance and/or
 interfere with synchronization?
 
 It might be possible to scan the list without holding a lock for
 diagnostic purposes if Finalizer.remove() didn't set the removed
 Finalizer.next pointer to point back to itself:
 
private void remove() {
synchronized (lock) {
if (unfinalized == this) {
if (this.next != null) {
unfinalized = this.next;
} else {
unfinalized = this.prev;
}
}
if (this.next != null) {
this.next.prev = this.prev;
}
if (this.prev != null) {
this.prev.next = this.next;
}
// this.next = this; must not be set so that we can
 traverse the list unsynchronized
this.prev = this;   /* Indicates that this has been
 finalized */
}
}
 
 For detecting whether a Finalizer is already processed, the 'prev'
 pointer could be used instead:
 
private boolean hasBeenFinalized() {
return (prev == this);
}
 
 Also, to make sure a data race dereferencing 'unfinalized' in
 unsynchronized printFinalizationQueue() would get you a fully
 initialized Finalizer instance (in particular the next pointer), 
 you
 would have to make the 'unfinalized' field volatile or insert an
 Unsafe.storeFence() before assigning to 'unfinalized':
 
private void add() {
synchronized (lock) {
if (unfinalized != null) {
this.next = unfinalized;
unfinalized.prev = this;
}
// make sure a data race dereferencing 'unfinalized'
// in printFinalizationQueue() does see the Finalizer
// instance fully initialized
// (in particular the 'next' pointer)
U.storeFence();
unfinalized = this;
}
}
 
 
 By doing these modifications, I think you can remove
 synchronized(lock) {} from printFinalizationQueue().
 
 I see you are traversing the ‘unfinalized’ list in Finalizer, 
 whereas
 the old SA code for ‘-finalizerinfo' traverses the ‘queue’ list. 
 I am
 not sure of the difference, perhaps someone from the GC team can 
 help
 out?
 The 'queue' is a ReferenceQueue of Finalizer (FinalReference)
 instances pending processing by finalizer thread because their
 referents are eligible for finalization (they are not reachable any
 more). The 'unfinalized' is a doubly-linked list of all Finalizer
 instances for which their referents have not been finalized yet
 (including those that are still reachable and alive). The later 
 serves
 two purposes:
 - it keeps

Re: RFR(M): JDK-8059036 : Implement Diagnostic Commands for heap and finalizerinfo

2015-05-05 Thread Staffan Larsen
Dmitry,

I think this should be reviewed on hotspot-gc and core-libs-dev as well 
considering the changes to Finalizer.

I’m a little worried about the potentially very large String that is returned 
from printFinalizationQueue(). A possible different approach would be to write 
printFinalizationQueue() in C++ inside Hotspot. That would allow for outputting 
one line at a time. The output would still be saved in memory (since the stream 
is buffered), but at least the data is only saved once in memory, then. It 
would make the code a bit harder to write, so its a question of how scared we 
are of running out of memory.

I see you are traversing the ‘unfinalized’ list in Finalizer, whereas the old 
SA code for ‘-finalizerinfo' traverses the ‘queue’ list. I am not sure of the 
difference, perhaps someone from the GC team can help out?

For the output, it would be a nice touch to sort it on the number of references 
for each type. Perhaps outputting it more like a table (see the old code again) 
would also make it easier to digest.

In diagnosticCommand.hpp, the new commands should override permission() and 
limit access:

  static const JavaPermission permission() {
JavaPermission p = {java.lang.management.ManagementPermission,
monitor, NULL};
return p;
  }

The two tests don’t validate the output in any way. Would it be possible to add 
validation? Perhaps hard to make sure an object is on the finalizer queue… Hmm.

Thanks,
/Staffan


 On 5 maj 2015, at 12:01, Dmitry Samersoff dmitry.samers...@oracle.com wrote:
 
 Everyone,
 
 Please review the fix:
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.01/
 
 heap dcmd outputs the same information as SIGBREAK
 
 finalizerinfo dcmd outputs a list of all classes in finalization queue
 with count
 
 -Dmitry
 
 -- 
 Dmitry Samersoff
 Oracle Java development team, Saint Petersburg, Russia
 * I would love to change the world, but they won't give me the sources.



Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread Staffan Larsen

 On 16 apr 2015, at 21:01, Thomas Stüfe thomas.stu...@gmail.com wrote:
 
 Hi Roger,
 
 thank you for your answer!
 
 The reason I take an interest is not just theoretical. We (SAP) use our JVM
 for our test infrastructure and we had exactly the problem allChildren() is
 designed to solve: killing a process tree related to a specific tests
 (similar to jtreg tests) in case of errors or hangs. We have test machines
 running large workloads of tests in parallel and we reach pid wraparound -
 depending on the OS - quite fast.
 
 We solved this by adding process groups to Process.java and we are very
 happy with this solution. We are able to quickly kill a whole process tree,
 cleanly and completely, without ambiguity or risk to other tests. Of course
 we had to add this support as a sideways hack in order to not change the
 official Process.java interface. Therefore I was hoping that with JEP 102,
 we would get official support for process groups. Unfortunately, seems the
 decision is already done and we are too late in the discussion :(

Interestingly we are hoping to use allChildren() to kill process trees in jtreg 
- exactly the use case you are describing. I haven’t been testing the current 
approach in allChildren(), but it seems your experience indicates that it will 
not be a perfect fit for the use case. In a previous test framework I was 
involved in we also used process groups for this with good results. This does 
beg the question: if the current approach isn’t useful for our own testing 
purposes, when is it useful?

Thanks,
/Staffan

 
 see my other comments inline.
 
 On Sat, Apr 11, 2015 at 8:55 PM, Roger Riggs roger.ri...@oracle.com 
 mailto:roger.ri...@oracle.com wrote:
 
 Hi Thomas,
 
 Thanks for the comments.
 
 On 4/11/2015 8:31 AM, Thomas Stüfe wrote:
 
 Hi Roger,
 
 I have a question about getChildren() and getAllChildren().
 
 I assume the point of those functions is to implement point 4 of JEP 102
 (The ability to deal with process trees, in particular some means to
 destroy a process tree.), by returning a collection of PIDs which are the
 children of the process and then killing them?
 
 Earlier versions included a killProcess tree method but it was recommended
 to leave
 the exact algorithm to kill processes to the caller.
 
 
 However, I am not sure that this can be implemented in a safe way, at
 least on UNIX, because - as Martin already pointed out - of PID recycling.
 I do not see how you can prevent allChildren() from returning PIDs which
 may be already reaped and recyled when you use them later. How do you
 prevent that?
 
 Unless there is an extended time between getting the children and
 destroying them the pids will still be valid.
 
 
 Why? Child process may be getting reaped the instant you are done reading
 it from /proc, and pid may have been recycled by the OS right away and
 already pointing to another process when allChildren() returns. If a
 process lives about as long as it takes the system to reach a pid
 wraparound to the same pid value, its pid could be recycled right after it
 is reaped, or? Sure, the longer you wait, the higher the chance of this to
 happen, but it may happen right away.
 
 As Martin said, we had those races in the kill() code since a long time,
 but children()/allChildren() could make those error more probable, because
 now more processes are involved. Especially if you use allChildren to kill
 a deep process tree. And there is nothing in the javadoc warning the user
 about this scenario. You would just happen from time to time to kill an
 unrelated process. Those problems are hard to debug.
 
 The technique of caching the start time can prevent that case; though it
 has AFAIK not been a problem.
 
 
 How would that work? User should, before issuing the kill, compare start
 time of process to kill with cached start time?
 
 Note even if your coding is bulletproof, that allChildren() will also
 return PIDs of sub processes which are completely unrelated to you and
 Process.java - they could have been forked by some third party native code
 which just happens to run in parallel in the same process. There, you have
 no control about when it gets reaped. It might already have been reaped by
 the time allChildren() returns, and now the same PID got recycled as
 another, unrelated process.
 
 Of course, the best case is for an application to spawn and manage its own
 processes
 and handle there proper termination.
 The use cases for children/allChildren are focused on
 supervisory/executive functions
 that monitor a running system and can cleanup even in the case of
 unexpected failures.
 
 All management of processes is subject to OS limitations, if the PID were
 from a completely
 different process tree, the ordinary destroy/info functions would not be
 available
 unless the process was running as a privileged os user (same as any other
 native application).
 
 
 Could you explain this please? If both trees run under the same user, why
 should 

Re: RFR: JDK-8025636 Hide lambda proxy frames in stacktraces

2015-03-06 Thread Staffan Larsen
I would like to backport this bug fix to jdk8 - does anyone see any problems 
with that? The patch applies cleanly (after shuffling) and the 
java/lang/invoke/ tests succeed.

Thanks,
/Staffan

Re: RFR: JDK-8072842 Add support for building native JTReg tests

2015-02-26 Thread Staffan Larsen
As far as I can tell (I’m not a makefile expert) this looks good.

Thanks,
/Staffan

 On 25 feb 2015, at 12:21, Magnus Ihse Bursie magnus.ihse.bur...@oracle.com 
 wrote:
 
 On 2015-02-11 13:08, Staffan Larsen wrote:
 
 Okay so if I just cd into hotspot/test and use the Makefile to try
 and run some jtreg tests it looks to me that it will define an
 invalid -nativepath
 
 I'm not sure if that is a supported use case. David or Staffan will
 have to answer to that. I have not tested that, only the whole
 forest approach.
 
 I’ve never done that. I’m always running all make commands from the top
 level. Is there a problem with that?
 
 I must confess I also haven't done that - though I often run jtreg 
 directly from there. Other hotspot engineers may use it. If nothing else 
 it would be a way to test out what you expect JPRT to be running.
 
 But perhaps we just don't add the -nativepath component if TESTNATIVE_DIR 
 remains unset?
 
 Not adding -nativepath or adding it with an empty path will lead to the 
 same errors I think: tests that need native code will fail. So it does not 
 really matter.
 
 If you add it with an invalid path (won't be empty as the variable is only 
 a prefix) then tests that don't need native code may also fail. Though I 
 don't know how jtreg responds to a non-existent nativepath.
 
 You are right. Jtreg validates the that the path is a directory. So better 
 not to specify it.
 
 Ok. I have updated the webrev, so the -nativepath: argument is only specified 
 if we indeed have been given a valid path to the native libraries.
 
 The only changes between this and the previous webrev is in 
 hotspot/test/Makefile and jdk/test/Makefile.
 
 http://cr.openjdk.java.net/~ihse/JDK-8072842-build-native-jtreg-tests/webrev.02
  
 http://cr.openjdk.java.net/~ihse/JDK-8072842-build-native-jtreg-tests/webrev.02
 
 /Magnus



Re: RFR: JDK-8025636 Hide lambda proxy frames in stacktraces

2015-02-16 Thread Staffan Larsen
Good point!

new webrev: http://cr.openjdk.java.net/~sla/8025636/webrev.02/ 
http://cr.openjdk.java.net/~sla/8025636/webrev.02/

Thanks,
/Staffan

 On 16 feb 2015, at 12:40, Remi Forax fo...@univ-mlv.fr wrote:
 
 Hi Staffan,
 ASM MethodVisitor API requires to call visitAnnotation before calling 
 visitCode so
 I think you shoud call visitAnnotation before calling new 
 ForwardingMethodGenerator(mv).generate().
 
 cheers,
 Rémi
 
 On 02/16/2015 08:47 AM, Staffan Larsen wrote:
 Brian pointed out to me that this change missed to add the annotation to 
 bridge methods. Here is an updated version that takes those into account. I 
 also needed to update the test to verify that bridge methods were correctly 
 annotated - it got a little bit more complex since I had to force bridges 
 being used.
 
 new webrev: http://cr.openjdk.java.net/~sla/8025636/webrev.01/ 
 http://cr.openjdk.java.net/%7Esla/8025636/webrev.01/
 
 Thanks,
 /Staffan
 
 
 On 3 feb 2015, at 10:15, Staffan Larsen staffan.lar...@oracle.com 
 mailto:staffan.lar...@oracle.com wrote:
 
 Hi,
 
 Please review this patch for hiding the lambda proxy frame in stack traces:
 
 bug: https://bugs.openjdk.java.net/browse/JDK-8025636 
 https://bugs.openjdk.java.net/browse/JDK-8025636
 webrev: http://cr.openjdk.java.net/~sla/8025636/webrev.00/ 
 http://cr.openjdk.java.net/%7Esla/8025636/webrev.00/
 
 This is a straightforward addition of the LambdaForm$Hidden annotation to 
 the generated methods. What is surprising is that this works even if 
 LambdaForm$Hidden is a package-private class in java.lang.invoke and thus 
 not accessible from most of the generated classes. There is some discussion 
 of and answers to this in the bug, but essentially this works because the 
 annotation class is never resolved and the code in Hotspot that looks for 
 the annotation amounts to nothing more than string comparisons.
 
 Hidden stack frames can be shown by running with 
 “-XX:+UnlockDiagnosticVMOptions -XX:+ShowHiddenFrames”.
 
 For an example of what this patch does, consider this code:
 
Runnable r = () - { throw new RuntimeException(); };
r.run();
 
 Previously, this would output:
 
 java.lang.RuntimeException
  at pkg.Foo.lambda$main$0(Foo.java:5)
  at pkg.Foo$$Lambda$1/2001112025.run(Unknown:100)
  at pkg.Foo.main(Foo.java:15)
 
 With the patch it looks like this:
 
 java.lang.RuntimeException
  at pkg.Foo.lambda$main$0(Foo.java:5)
  at pkg.Foo.main(Foo.java:15)
 
 
 Thanks,
 /Staffan
 
 
 



Re: RFR: JDK-8025636 Hide lambda proxy frames in stacktraces

2015-02-16 Thread Staffan Larsen

 On 17 feb 2015, at 02:16, John Rose john.r.r...@oracle.com wrote:
 
 On Feb 16, 2015, at 6:25 AM, Staffan Larsen staffan.lar...@oracle.com 
 mailto:staffan.lar...@oracle.com wrote:
 
 new webrev: http://cr.openjdk.java.net/~sla/8025636/webrev.02/ 
 http://cr.openjdk.java.net/~sla/8025636/webrev.02/
 Looks good; ship it.

Thanks.

 
 To me this fix raises more questions:
 
 1. Are there other places where we generate ACC_SYNTHETIC that should also 
 get @Hidden annotations?
 
 2. Should the JVM be filtering stack frames for ACC_SYNTHETIC (or ACC_BRIDGE 
 or ACC_MANDATED) calls?  (By default?  Or perhaps as a new option?)

The first shot at fixing this bug was to filter out ACC_SYNTHETIC. The drawback 
was that the actual lambda method are marked ACC_SYNTHETIC, so that filtered 
too much.

/Staffan

 
 3. When will we have a reasonable stack walking mechanism where we can 
 program such policies in JDK library code, instead of hacking the JVM?
 
 — John
 
 P.S.  https://bugs.openjdk.java.net/browse/JDK-8043814 
 https://bugs.openjdk.java.net/browse/JDK-8043814


Re: RFR: JDK-8025636 Hide lambda proxy frames in stacktraces

2015-02-15 Thread Staffan Larsen
Brian pointed out to me that this change missed to add the annotation to bridge 
methods. Here is an updated version that takes those into account. I also 
needed to update the test to verify that bridge methods were correctly 
annotated - it got a little bit more complex since I had to force bridges being 
used.

new webrev: http://cr.openjdk.java.net/~sla/8025636/webrev.01/ 
http://cr.openjdk.java.net/~sla/8025636/webrev.01/

Thanks,
/Staffan


 On 3 feb 2015, at 10:15, Staffan Larsen staffan.lar...@oracle.com wrote:
 
 Hi,
 
 Please review this patch for hiding the lambda proxy frame in stack traces:
 
 bug: https://bugs.openjdk.java.net/browse/JDK-8025636 
 https://bugs.openjdk.java.net/browse/JDK-8025636
 webrev: http://cr.openjdk.java.net/~sla/8025636/webrev.00/ 
 http://cr.openjdk.java.net/~sla/8025636/webrev.00/
 
 This is a straightforward addition of the LambdaForm$Hidden annotation to the 
 generated methods. What is surprising is that this works even if 
 LambdaForm$Hidden is a package-private class in java.lang.invoke and thus not 
 accessible from most of the generated classes. There is some discussion of 
 and answers to this in the bug, but essentially this works because the 
 annotation class is never resolved and the code in Hotspot that looks for the 
 annotation amounts to nothing more than string comparisons.
 
 Hidden stack frames can be shown by running with 
 “-XX:+UnlockDiagnosticVMOptions -XX:+ShowHiddenFrames”.
 
 For an example of what this patch does, consider this code:
 
Runnable r = () - { throw new RuntimeException(); };
r.run();
 
 Previously, this would output:
 
 java.lang.RuntimeException
   at pkg.Foo.lambda$main$0(Foo.java:5)
   at pkg.Foo$$Lambda$1/2001112025.run(Unknown:100)
   at pkg.Foo.main(Foo.java:15)
 
 With the patch it looks like this:
 
 java.lang.RuntimeException
   at pkg.Foo.lambda$main$0(Foo.java:5)
   at pkg.Foo.main(Foo.java:15)
 
 
 Thanks,
 /Staffan
 



Re: RFR: JDK-8072842 Add support for building native JTReg tests

2015-02-11 Thread Staffan Larsen

 On 11 feb 2015, at 09:39, David Holmes david.hol...@oracle.com wrote:
 
 On 11/02/2015 6:34 PM, Staffan Larsen wrote:
 
 On 11 feb 2015, at 09:27, Magnus Ihse Bursie
 magnus.ihse.bur...@oracle.com mailto:magnus.ihse.bur...@oracle.com
 wrote:
 
 On 2015-02-11 09:23, David Holmes wrote:
 On 11/02/2015 6:09 PM, Magnus Ihse Bursie wrote:
 On 2015-02-11 02:35, David Holmes wrote:
 Hi Magnus,
 
 On 11/02/2015 12:23 AM, Magnus Ihse Bursie wrote:
 Here is an addition to the build system, that will compile native
 libraries and executables and make them available for JTReg tests
 written in Java.
 
 Sorry I'm missing the basics here: when does this compilation take
 place? As part of a normal build? Where will the build artifacts go?
 
 This is the first application of the new test-image/product-images
 separation we discussed previously. :)
 
 These tests are built as part of the test-image target. (Actually,
 they are built by individual rules like build-test-jdk-jtreg-native, and
 the relevant parts are put into the test image by
 test-image-jdk-jtreg-native, which test-image depends on.)
 
 Okay so if I just cd into hotspot/test and use the Makefile to try
 and run some jtreg tests it looks to me that it will define an
 invalid -nativepath
 
 I'm not sure if that is a supported use case. David or Staffan will
 have to answer to that. I have not tested that, only the whole
 forest approach.
 
 I’ve never done that. I’m always running all make commands from the top
 level. Is there a problem with that?
 
 I must confess I also haven't done that - though I often run jtreg directly 
 from there. Other hotspot engineers may use it. If nothing else it would be a 
 way to test out what you expect JPRT to be running.
 
 But perhaps we just don't add the -nativepath component if TESTNATIVE_DIR 
 remains unset?

Not adding -nativepath or adding it with an empty path will lead to the same 
errors I think: tests that need native code will fail. So it does not really 
matter.

/Staffan

 
 Cheers,
 David
 
 /Staffan
 
 
 /Magnus
 



Re: RFR: JDK-8072842 Add support for building native JTReg tests

2015-02-11 Thread Staffan Larsen

 On 11 feb 2015, at 09:27, Magnus Ihse Bursie magnus.ihse.bur...@oracle.com 
 wrote:
 
 On 2015-02-11 09:23, David Holmes wrote:
 On 11/02/2015 6:09 PM, Magnus Ihse Bursie wrote:
 On 2015-02-11 02:35, David Holmes wrote:
 Hi Magnus,
 
 On 11/02/2015 12:23 AM, Magnus Ihse Bursie wrote:
 Here is an addition to the build system, that will compile native
 libraries and executables and make them available for JTReg tests
 written in Java.
 
 Sorry I'm missing the basics here: when does this compilation take
 place? As part of a normal build? Where will the build artifacts go?
 
 This is the first application of the new test-image/product-images
 separation we discussed previously. :)
 
 These tests are built as part of the test-image target. (Actually,
 they are built by individual rules like build-test-jdk-jtreg-native, and
 the relevant parts are put into the test image by
 test-image-jdk-jtreg-native, which test-image depends on.)
 
 Okay so if I just cd into hotspot/test and use the Makefile to try and run 
 some jtreg tests it looks to me that it will define an invalid -nativepath
 
 I'm not sure if that is a supported use case. David or Staffan will have to 
 answer to that. I have not tested that, only the whole forest approach.

I’ve never done that. I’m always running all make commands from the top level. 
Is there a problem with that?

/Staffan

 
 /Magnus



Re: RFR: JDK-8072842 Add support for building native JTReg tests

2015-02-11 Thread Staffan Larsen

 On 11 feb 2015, at 12:15, David Holmes david.hol...@oracle.com wrote:
 
 On 11/02/2015 8:36 PM, Staffan Larsen wrote:
 
 On 11 feb 2015, at 09:39, David Holmes david.hol...@oracle.com wrote:
 
 On 11/02/2015 6:34 PM, Staffan Larsen wrote:
 
 On 11 feb 2015, at 09:27, Magnus Ihse Bursie
 magnus.ihse.bur...@oracle.com mailto:magnus.ihse.bur...@oracle.com
 wrote:
 
 On 2015-02-11 09:23, David Holmes wrote:
 On 11/02/2015 6:09 PM, Magnus Ihse Bursie wrote:
 On 2015-02-11 02:35, David Holmes wrote:
 Hi Magnus,
 
 On 11/02/2015 12:23 AM, Magnus Ihse Bursie wrote:
 Here is an addition to the build system, that will compile native
 libraries and executables and make them available for JTReg tests
 written in Java.
 
 Sorry I'm missing the basics here: when does this compilation take
 place? As part of a normal build? Where will the build artifacts go?
 
 This is the first application of the new test-image/product-images
 separation we discussed previously. :)
 
 These tests are built as part of the test-image target. (Actually,
 they are built by individual rules like build-test-jdk-jtreg-native, and
 the relevant parts are put into the test image by
 test-image-jdk-jtreg-native, which test-image depends on.)
 
 Okay so if I just cd into hotspot/test and use the Makefile to try
 and run some jtreg tests it looks to me that it will define an
 invalid -nativepath
 
 I'm not sure if that is a supported use case. David or Staffan will
 have to answer to that. I have not tested that, only the whole
 forest approach.
 
 I’ve never done that. I’m always running all make commands from the top
 level. Is there a problem with that?
 
 I must confess I also haven't done that - though I often run jtreg directly 
 from there. Other hotspot engineers may use it. If nothing else it would be 
 a way to test out what you expect JPRT to be running.
 
 But perhaps we just don't add the -nativepath component if TESTNATIVE_DIR 
 remains unset?
 
 Not adding -nativepath or adding it with an empty path will lead to the same 
 errors I think: tests that need native code will fail. So it does not really 
 matter.
 
 If you add it with an invalid path (won't be empty as the variable is only a 
 prefix) then tests that don't need native code may also fail. Though I don't 
 know how jtreg responds to a non-existent nativepath.

You are right. Jtreg validates the that the path is a directory. So better not 
to specify it.

/Staffan

 
 David
 
 /Staffan
 
 
 Cheers,
 David
 
 /Staffan
 
 
 /Magnus



Re: RFR: JDK-8072842 Add support for building native JTReg tests

2015-02-10 Thread Staffan Larsen

 On 10 feb 2015, at 15:23, Magnus Ihse Bursie magnus.ihse.bur...@oracle.com 
 wrote:
 
 Here is an addition to the build system, that will compile native libraries 
 and executables and make them available for JTReg tests written in Java.
 
 This patch is the result of the work (in serial, mostly) of Staffan Larsen, 
 David Simms and me. (And possible more that I don't know about.)
 
 In it current form, the patch only provides the framework on which to attach 
 real tests. To prove the concept, some initial dummy tests are present. These 
 are supposed to be removed as soon as real tests starts to propagate into the 
 jdk and hotspot jtreg test suites.
 
 The behavior is based on the following design: For directories with native 
 jtreg compilation enabled, the build system searches for native files 
 prefixed with either lib or exe, and compiles a free-standing library or 
 an executable, respectively, for each such file. Since all compiled files are 
 placed in a single directory (this is currently a requirement from the jtreg 
 native support), there is the additional requirement that all files have 
 unique names.
 
 My personal opinion is that a better long-term approach is to compile all 
 tests into a single library, if possible. (I realize some tests need to be 
 separate to test library loading, etc.) For that to work, however, JTReg 
 needs to be changed. The build framework in the patch is designed in such a 
 way that it will be easy to add compilation to a common library in the 
 future, if that restriction is lifted from JTReg.

To clarify: The restriction in jtreg is that all tests are loaded in separate 
class loaders (when running in samevm mode). A native library can only be 
loaded in one class loader at a time. So if two tests tries to load the same 
library we get errors. It would be possible to change this if samevm mode is 
removed from jtreg.

/Staffan

 
 This patch also lays the foundation for building additional tests, not only 
 native jtreg tests, by the build system in the future. For instance, it 
 codifies the suggested correspondence between make targets, intermediate test 
 output and test image final build results. With the on-going test co-location 
 project, we expect a stream of tests to be added to the build system in the 
 future, and we hope this project can serve as an example of a suitable way of 
 integration.
 
 The patch modifies hotspot code, but it's mostly due to the addition of the 
 new dummy tests. My preference would be to integrate this patch via jdk9-dev, 
 since it modifies the build system most of all, but I'm open for discussion.
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-8072842
 WebRev: 
 http://cr.openjdk.java.net/~ihse/JDK-8072842-build-native-jtreg-tests/webrev.01
 
 /Magnus



Re: JEP 102 Process Updates revised API draft

2015-02-10 Thread Staffan Larsen
Happy to see this!

In ProcessHandle.Info would it be possible to include the environment variables 
of the process as well?

How does ProcessHandle.allChildren() behave when process A launches B which 
launches C, and B terminates? Is C included in allChildren() of A?

Thanks,
/Staffan

 On 10 feb 2015, at 00:25, Roger Riggs roger.ri...@oracle.com wrote:
 
 Hi,
 
 After a protracted absence from working on JEP 102, the updated API draft
 provides access to process hierarchies and individual process information;
 as permitted by the OS. The relationship between Process and ProcessHandle
 is clarified and the security model validated.
 
 Both Processes and ProcessHandles can be monitored using CompletableFuture
 for termination and to trigger additional actions on Process exit.
 Information about processes includes the total cputime, starttime, user,
 executable, and arguments.
 
 Please review and comment:
   http://cr.openjdk.java.net/~rriggs/ph-apidraft/
 
 Thanks, Roger
 
 
 
 



RFR: JDK-8072456 @since tags missing from TimeUnit

2015-02-04 Thread Staffan Larsen
The MINUTES, HOURS, and DAYS fields of TimeUnit are missing an @since 1.6 tag.

webrev: http://cr.openjdk.java.net/~sla/8072456/webrev.00/ 
http://cr.openjdk.java.net/~sla/8072456/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-8072456 
https://bugs.openjdk.java.net/browse/JDK-8072456

Thanks,
/Staffan

RFR: JDK-8072458 jdk/test/Makefile references (to be removed) win32 directory in jtreg

2015-02-04 Thread Staffan Larsen
The platform specific directories in jtreg are going away in favor of a 
platform-agnostic directory.

Small fix below.

Thanks,
/Staffan


diff --git a/test/Makefile b/test/Makefile
--- a/test/Makefile
+++ b/test/Makefile
@@ -267,8 +267,8 @@
   EXTRA_JTREG_OPTIONS += -concurrency:$(CONCURRENCY)
 endif

-# Default JTREG to run (win32 script works for everybody)
-JTREG = $(JT_HOME)/win32/bin/jtreg
+# Default JTREG to run
+JTREG = $(JT_HOME)/bin/jtreg
 # run in agentvm mode
 JTREG_BASIC_OPTIONS += -agentvm
 # Only run automatic tests

Re: RFR - 8072450: 9-dev build failed on elinux-i586 and rlinux-i586

2015-02-04 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan


 On 4 feb 2015, at 12:31, Daniel Fuchs daniel.fu...@oracle.com wrote:
 
 Hi,
 
 Please find below a fix for:
 
 8072450: 9-dev build failed on elinux-i586 and rlinux-i586
 
 My fix for JDK-8068730 which was integrated from hs-rt into jdk9-dev
 yesterday is causing a build failure in the jdk9/dev nightly on two platforms.
 It appears that these platforms have a slightly older version
 of the compiler - which chokes on jvm.cpp with the following error:
 
 hotspot/src/share/vm/prims/jvm.cpp:307: error: integer constant is too large 
 for ���long��� type
 
 Adding a LL suffix should solve the issue:
 
 ##
 
 diff --git a/src/share/vm/prims/jvm.cpp b/src/share/vm/prims/jvm.cpp
 --- a/src/share/vm/prims/jvm.cpp
 +++ b/src/share/vm/prims/jvm.cpp
 @@ -304,7 +304,7 @@
 // java.lang.System, but we choose to keep it here so that it stays next
 // to JVM_CurrentTimeMillis and JVM_NanoTime
 
 -const jlong MAX_DIFF_SECS = 0x01;   //  2^32
 +const jlong MAX_DIFF_SECS = 0x01LL; //  2^32
 const jlong MIN_DIFF_SECS = -MAX_DIFF_SECS; // -2^32
 
 JVM_LEAF(jlong, JVM_GetNanoTimeAdjustment(JNIEnv *env, jclass ignored, jlong 
 offset_secs))
 
 ##
 
 ( or if you prefer here is a webrev:
  http://cr.openjdk.java.net/~dfuchs/webrev_8072450/webrev.00 )
 
 best regards,
 
 -- daniel
 



RFR: JDK-8025636 Hide lambda proxy frames in stacktraces

2015-02-03 Thread Staffan Larsen
Hi,

Please review this patch for hiding the lambda proxy frame in stack traces:

bug: https://bugs.openjdk.java.net/browse/JDK-8025636 
https://bugs.openjdk.java.net/browse/JDK-8025636
webrev: http://cr.openjdk.java.net/~sla/8025636/webrev.00/ 
http://cr.openjdk.java.net/~sla/8025636/webrev.00/

This is a straightforward addition of the LambdaForm$Hidden annotation to the 
generated methods. What is surprising is that this works even if 
LambdaForm$Hidden is a package-private class in java.lang.invoke and thus not 
accessible from most of the generated classes. There is some discussion of and 
answers to this in the bug, but essentially this works because the annotation 
class is never resolved and the code in Hotspot that looks for the annotation 
amounts to nothing more than string comparisons.

Hidden stack frames can be shown by running with 
“-XX:+UnlockDiagnosticVMOptions -XX:+ShowHiddenFrames”.

For an example of what this patch does, consider this code:

   Runnable r = () - { throw new RuntimeException(); };
   r.run();

Previously, this would output:

java.lang.RuntimeException
at pkg.Foo.lambda$main$0(Foo.java:5)
at pkg.Foo$$Lambda$1/2001112025.run(Unknown:100)
at pkg.Foo.main(Foo.java:15)

With the patch it looks like this:

java.lang.RuntimeException
at pkg.Foo.lambda$main$0(Foo.java:5)
at pkg.Foo.main(Foo.java:15)


Thanks,
/Staffan



Re: RFR (S) 8059677: Thread.getName() instantiates Strings

2014-11-11 Thread Staffan Larsen
I was able to provoke the failure with a “jstack -F”. I think this patch solves 
the problem: http://cr.openjdk.java.net/~sla/8059677-thread.name.sa.patch 
http://cr.openjdk.java.net/~sla/8059677-thread.name.sa.patch. Feel free to 
not include the changes in StackTrace.java if you don’t want to complicate your 
review.

Thanks,
/Staffan


 On 10 nov 2014, at 19:09, Aleksey Shipilev aleksey.shipi...@oracle.com 
 wrote:
 
 On 10.11.2014 19:39, Staffan Larsen wrote:
 On 10 nov 2014, at 15:55, Aleksey Shipilev aleksey.shipi...@oracle.com 
 wrote:
 Ow, it seems very like it.
 So, what testlist have I missed to catch this?
 
 Probably vm.tmtools.testlist and/or nsk.sajdi.testlist. Just a warning that 
 these tests are far from stable. Sorry about that.
 
 Alas, both these testlists pass with current change without a hitch.
 That probably tells something about the test coverage. Any other ideas
 how to test for it? Maybe some manual way?
 
 Anyhow, there is a synonymous block in ThreadGroup handling, I can copy
 the relevant bits from there. Updated webrev follows soon. Still need to
 test if that change is safe.
 
 -Aleksey.
 



Re: RFR (S) 8059677: Thread.getName() instantiates Strings

2014-11-11 Thread Staffan Larsen
SA changes look good.

/Staffan

 On 11 nov 2014, at 15:40, Aleksey Shipilev aleksey.shipi...@oracle.com 
 wrote:
 
 Hi,
 
 On 11/09/2014 09:45 PM, Aleksey Shipilev wrote:
 Thread.getName() returns String, and does new String instantiation every
 time, because the thread name is stored in char[]. Even though we use a
 private String constructor that shares the char[] array without copying
 it, this still hurts some use cases (think extra-fast logging). To the
 extent some people actually maintain MapThread, String to avoid it.
 https://bugs.openjdk.java.net/browse/JDK-8059677
 
 Here's the attempt to maintain String instead of char[]:
 http://cr.openjdk.java.net/~shade/8059677/webrev.01.jdk/
 http://cr.openjdk.java.net/~shade/8059677/webrev.01.hs/
 
 Updated webrevs:
  http://cr.openjdk.java.net/~shade/8059677/webrev.02.jdk/
  http://cr.openjdk.java.net/~shade/8059677/webrev.02.hs/
 
 This version incorporates feedbacks from Chris, Staffan and David. I
 think it is very close to what we would like to push. Opinions?
 
 Testing: JPRT, jdk/test/java/lang/Thread jtreg, hotspot/test/runtime/
 jtreg, vm.quick.testlist, nsk.jvmti.testlist, svc.quick.testlist,
 vm.tmtools.testlist
 
 Thanks,
 -Aleksey.
 
 
 
 



Re: RFR (S) 8059677: Thread.getName() instantiates Strings

2014-11-10 Thread Staffan Larsen
I’m afraid this change requires changes in the Serviceability Agent as well. 
See OopUtilities.threadOopGetName() for example.

/Staffan

 On 10 nov 2014, at 15:19, Aleksey Shipilev aleksey.shipi...@oracle.com 
 wrote:
 
 Hi David, Chris,
 
 On 11/10/2014 04:53 PM, Chris Hegarty wrote:
 On 10/11/14 12:56, David Holmes wrote:
 On 10/11/2014 9:52 PM, Chris Hegarty wrote:
 I have only looked at the libraries changes, and I think they make sense
 . As in, I can find no reason why the name cannot be changed to be a
 String.
 
 Very quick response, but IIRC this has been examined in the past and
 there were reasons why it can't/shouldn't be done. Will try to dig out
 more details in the morning.
 
 If there was previous discussion on this, that revealed some substantial
 issue, that would be great, but I can't recall, or find, it now.
 
 Hotspot express, and the desire for hotspot to run with different
 library versions, would certainly cause complication, but I don't
 believe that is an issue now.
 
 Just on that, the library changes are minimal, and if this were to
 proceed then they can accompany the hotspot change, as they make their
 way into jdk9/dev.
 
 Anyway, this should await your reply.
 
 Alan was having the same concern, there is an issue with JNI/JVMTI and
 other power users that might break when exposed to under-constructed
 Thread, e.g:
 https://bugs.openjdk.java.net/browse/JDK-6412693
 
 This is why I ran jvmti and serviceability tests for this change,
 yielding no failures. This reinforces my belief this patch does not
 break the important invariant: if there is a problem with Thread.name =
 name.toCharArray() anywhere in Thread code, then Thread.name = name
 does neither regress it further nor fixes it.
 
 Then I speculated that having char[] name would help VM initialize the
 name if we wanted to switch to complete VM-side initialization of
 Thread, but it seems we can do String oop instantiation in the similar vein.
 
 Caching the name feels like a band-aid, that will probably complicate
 the Thread initialization on VM side even more. Let's wait and see if
 David can come up with some horror issue we are overlooking. :)
 
 Thanks,
 -Aleksey.
 



Re: RFR (S) 8059677: Thread.getName() instantiates Strings

2014-11-10 Thread Staffan Larsen

 On 10 nov 2014, at 15:55, Aleksey Shipilev aleksey.shipi...@oracle.com 
 wrote:
 
 Hi Staffan,
 
 Ow, it seems very like it.
 So, what testlist have I missed to catch this?

Probably vm.tmtools.testlist and/or nsk.sajdi.testlist. Just a warning that 
these tests are far from stable. Sorry about that.

/Staffan

 
 -Aleksey.
 
 On 11/10/2014 05:51 PM, Staffan Larsen wrote:
 I’m afraid this change requires changes in the Serviceability Agent as well. 
 See OopUtilities.threadOopGetName() for example.
 
 /Staffan
 
 On 10 nov 2014, at 15:19, Aleksey Shipilev aleksey.shipi...@oracle.com 
 wrote:
 
 Hi David, Chris,
 
 On 11/10/2014 04:53 PM, Chris Hegarty wrote:
 On 10/11/14 12:56, David Holmes wrote:
 On 10/11/2014 9:52 PM, Chris Hegarty wrote:
 I have only looked at the libraries changes, and I think they make sense
 . As in, I can find no reason why the name cannot be changed to be a
 String.
 
 Very quick response, but IIRC this has been examined in the past and
 there were reasons why it can't/shouldn't be done. Will try to dig out
 more details in the morning.
 
 If there was previous discussion on this, that revealed some substantial
 issue, that would be great, but I can't recall, or find, it now.
 
 Hotspot express, and the desire for hotspot to run with different
 library versions, would certainly cause complication, but I don't
 believe that is an issue now.
 
 Just on that, the library changes are minimal, and if this were to
 proceed then they can accompany the hotspot change, as they make their
 way into jdk9/dev.
 
 Anyway, this should await your reply.
 
 Alan was having the same concern, there is an issue with JNI/JVMTI and
 other power users that might break when exposed to under-constructed
 Thread, e.g:
 https://bugs.openjdk.java.net/browse/JDK-6412693
 
 This is why I ran jvmti and serviceability tests for this change,
 yielding no failures. This reinforces my belief this patch does not
 break the important invariant: if there is a problem with Thread.name =
 name.toCharArray() anywhere in Thread code, then Thread.name = name
 does neither regress it further nor fixes it.
 
 Then I speculated that having char[] name would help VM initialize the
 name if we wanted to switch to complete VM-side initialization of
 Thread, but it seems we can do String oop instantiation in the similar vein.
 
 Caching the name feels like a band-aid, that will probably complicate
 the Thread initialization on VM side even more. Let's wait and see if
 David can come up with some horror issue we are overlooking. :)
 
 Thanks,
 -Aleksey.
 
 
 
 



Re: Review request: JDK-8043277: Update jdk regression tests to extend the default security policy instead of override

2014-10-27 Thread Staffan Larsen

On 24 okt 2014, at 18:16, Mandy Chung mandy.ch...@oracle.com wrote:

 On 10/24/2014 6:33 AM, Staffan Larsen wrote:
 
 Since this is the first use of jtreg 4.1b10 features, this would also be 
 good time to tag the jdk test suite to require at least 4.1b10.
 
 This latest version of jtreg has support for checking which version of jtreg 
 the test suite requires. So you can add a line saying:
requiredVersion=4.1 b10
 to TEST.ROOT and jtreg will verify that its version number is higher than 
 “requiredVersion when it runs.
 
 It’s not until we move from b10 to b11 that this will actually be useful, 
 but it could be a good time to introduce it.
 
 Good point since now it depends on b10 feature.  Here is
 the patch.
 
 diff --git a/test/TEST.ROOT b/test/TEST.ROOT
 --- a/test/TEST.ROOT
 +++ b/test/TEST.ROOT
 @@ -12,3 +12,6 @@
  # Group definitions
 groups=TEST.groups [closed/TEST.groups]
 +
 +# Tests using jtreg 4.1 b10 features
 +requiredVersion=4.1 b10
 
 Mandy

Looks good!

Thanks,
/Staffan

 
 
 
 Thanks,
 /Staffan
 
 On 23 okt 2014, at 15:40, Alan Bateman alan.bate...@oracle.com wrote:
 
 On 23/10/2014 02:08, Mandy Chung wrote:
 jtreg policy option overrides the system security policy file and hence
 some existing test policy files have to duplicate the entries to grant
 permissions for JDK.
 
 This looks okay to me too. I think this will be the first use of a 
 jtreg4.1-b10 feature and maybe someone should send a note to jdk9-dev to 
 tell folks that they will need an up-to-date jtreg in order to test 
 jdk9/dev.
 
 -Alan
 
 



Re: Review request: JDK-8043277: Update jdk regression tests to extend the default security policy instead of override

2014-10-24 Thread Staffan Larsen
Since this is the first use of jtreg 4.1b10 features, this would also be good 
time to tag the jdk test suite to require at least 4.1b10. 

This latest version of jtreg has support for checking which version of jtreg 
the test suite requires. So you can add a line saying:
   requiredVersion=4.1 b10
to TEST.ROOT and jtreg will verify that its version number is higher than 
“requiredVersion when it runs.

It’s not until we move from b10 to b11 that this will actually be useful, but 
it could be a good time to introduce it.

Thanks,
/Staffan

On 23 okt 2014, at 15:40, Alan Bateman alan.bate...@oracle.com wrote:

 On 23/10/2014 02:08, Mandy Chung wrote:
 jtreg policy option overrides the system security policy file and hence
 some existing test policy files have to duplicate the entries to grant
 permissions for JDK.
 
 This looks okay to me too. I think this will be the first use of a 
 jtreg4.1-b10 feature and maybe someone should send a note to jdk9-dev to tell 
 folks that they will need an up-to-date jtreg in order to test jdk9/dev.
 
 -Alan
 



Re: [8u40] RFR: JDK-8058632: Revert JDK-8054984 from 8u40

2014-09-18 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 18 sep 2014, at 11:00, Joel Borggrén-Franck joel.fra...@oracle.com wrote:

 Hi,
 
 Can I get a review for this anti-patch for JDK-8054984  (which is a backport  
 of JDK-8044629 (reflect) Constructor.getAnnotatedReceiverType() returns 
 wrong value” to 8u).
 
 The fix shouldn’t have been back ported, so this change reverts it in 8u only.
 
 Webrev: http://cr.openjdk.java.net/~jfranck/8058632/webrev.00/
 Side-by-side-diff: 
 http://cr.openjdk.java.net/~jfranck/8058632/side-by-side.diff
 
 Original jdk8u webrev for reference: 
 http://cr.openjdk.java.net/~jfranck/8044629/jdk8u/
 
 cheers
 /Joel



Re: RFR: 8032901: WaitForMultipleObjects() return value not handled appropriately

2014-06-04 Thread Staffan Larsen
Looks ok to me.

/Staffan

On 3 jun 2014, at 15:49, Aleksej Efimov aleksej.efi...@oracle.com wrote:

 Staffan, David,
 
 Returning back to our WaitForMultipleObjects()/WAIT_ABANDONED discussions:
 Thank you for your comments and remarks. I can't disagree with motivation 
 that it's better to have a fatal error during the incorrect mutex handling 
 then data corruption (the consequence of the previous fix). In case of such 
 error it'll be much more easier to debug/catch it (but as Staffan said - we 
 have tried to check all call paths and don't think that we'll encounter such 
 behavior).
 I have modified the discussed code according to your suggestions: 
 http://cr.openjdk.java.net/~aefimov/8032901/9/webrev.01/
 To abort the process the 'exitTransportWithError' function was utilized.
 Also I have tried to check that behavior isn't changed by running svc 
 regression tests set. There was no related test failures observed.
 
 Best Regards,
 Aleksej
 
 On 05/15/2014 01:11 PM, Staffan Larsen wrote:
 On 15 maj 2014, at 03:48, David Holmes david.hol...@oracle.com wrote:
 
 On 14/05/2014 11:18 PM, Aleksej Efimov wrote:
 David, Vitaly,
 
 I totally agree with Vitaly's explanation (Vitaly - thank you for that)
 and code in shmemBase.c (the usage of enterMutex() function for
 sending/receiving bytes through shared memory connection) illustrates on
 how the connection shutdown event is used as a cancellation token.
 Thanks for clarifying that.
 
 So if we were to encounter an abandoned mutex the code would presently have 
 acquired the mutex but return an error, thus preventing a subsequent 
 release, and preventing other threads from acquiring (but allowing current 
 thread to recurisevely acquire. So this could both hang and cause data 
 corruption.
 
 The new code will still return an error but release the mutex. So no more 
 hangs (other than by conditions caused by data corruption) but more 
 opportunity for data corruption.
 
 Obviously it depends on exactly what happens in the critical sections 
 guarded by this mutex, but in general I don't agree with this rationale for 
 making the change:
 
 204 /* If the mutex is abandoned we want to return an error
 205  * and also release the mutex so that we don't cause
 206  * other threads to be blocked. If a mutex was abandoned
 207  * we are in scary state. Data may be corrupted or inconsistent
 208  * but it is still better to let other threads run (and possibly
 209  * crash) than having them blocked (on the premise that a crash
 210  * is always easier to debug than a hang).
 
 Considering something has to have gone drastically wrong for the mutex to 
 become abandoned, I'm more inclined to consider this a fatal error and 
 abort.
 
 But I'll let the serviceability folk chime in here.
 I was involved in fixing this and writing the comment, so obviously I 
 thought it a good solution :-)
 
 I do agree that it would probably be a good idea to consider this a fatal 
 error and abort. At that point in the code we don’t have any really nice 
 ways of doing that, though. We could just print an error and call abort(). 
 What we are doing now is returning an error from sysIPMutexEnter() and 
 delegating the error handling to the caller. We have tried to check all call 
 paths to verify that they do “the right thing” in the face of an error. It 
 is obviously hard to verify, but it looks like they all terminate the 
 connection with some kind of error message.
 
 /Staffan
 
 
 Thanks,
 David
 
 
 Thank you,
 -Aleksej
 
 
 On 05/14/2014 01:05 PM, David Holmes wrote:
 On 14/05/2014 11:06 AM, Vitaly Davidovich wrote:
 In windows, you acquire a mutex by waiting on it using one of the wait
 functions, one of them employed in the code in question.  If
 WaitForMultipleObjects succeeds and returns the index of the mutex,
 current thread has ownership now.
 Yes I understand the basic mechanics :)
 
 It's also common to use multi wait functions where the event is a
 cancelation token, e.g. manual reset event; this allows someone to
 cancel waiting on mutex acquisition and return from the wait function.
 Presumably that's the case here, but I'll let Aleksej confirm; just
 wanted to throw this out there in the meantime :).
 Ah I see - yes cancellable lock acquisition would make sense.
 
 Thanks,
 David
 
 Sent from my phone
 
 On May 13, 2014 6:46 PM, David Holmes david.hol...@oracle.com
 mailto:david.hol...@oracle.com wrote:
 
Hi Aleksej,
 
Thanks for the doc references regarding abandonment.
 
Let me rephrase my question. What is this logic trying to achieve by
waiting on both a mutex and an event? Do we already own the mutex
when this function is called?
 
David
 
On 13/05/2014 11:19 PM, Aleksej Efimov wrote:
 
David,
 
The Windows has a different terminology for mutex objects (much
differs
from the POSIX one). This one link gave me some understanding of
it [1].
 
Here

Re: RFR(S): 8043520: Serviceability tests using @library failing with java.lang.NoClassDefFoundError

2014-05-20 Thread Staffan Larsen
test/sun/management/jmxremote/bootstrap/PasswordFilePermissionTest.java
  “Dummy” is being built twice.

Otherwise good!

/Staffan


On 20 maj 2014, at 14:24, Yekaterina Kantserova 
yekaterina.kantser...@oracle.com wrote:

 Staffan, Alan,
 
 could you please review the following fix.
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-8043520
 Webrev: http://cr.openjdk.java.net/~ykantser/8043520/webrev.00/
 
 I've missed somehow several tests in 
 http://cr.openjdk.java.net/~ykantser/8034960/webrev.01/ which existed in 
 http://cr.openjdk.java.net/~ykantser/8034960/webrev.00/.
 
 Thanks,
 Katja
 



Re: RFR(S): 8043520: Serviceability tests using @library failing with java.lang.NoClassDefFoundError

2014-05-20 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 20 maj 2014, at 15:48, Yekaterina Kantserova 
yekaterina.kantser...@oracle.com wrote:

 Thanks Staffan!
 
 New webrev can be found here: 
 http://cr.openjdk.java.net/~ykantser/8043520/webrev.01/
 
 // Katja
 
 
 
 On 05/20/2014 03:07 PM, Staffan Larsen wrote:
 test/sun/management/jmxremote/bootstrap/PasswordFilePermissionTest.java
   “Dummy” is being built twice.
 
 Otherwise good!
 
 /Staffan
 
 
 On 20 maj 2014, at 14:24, Yekaterina Kantserova 
 yekaterina.kantser...@oracle.com wrote:
 
 Staffan, Alan,
 
 could you please review the following fix.
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-8043520
 Webrev: http://cr.openjdk.java.net/~ykantser/8043520/webrev.00/
 
 I've missed somehow several tests in 
 http://cr.openjdk.java.net/~ykantser/8034960/webrev.01/ which existed in 
 http://cr.openjdk.java.net/~ykantser/8034960/webrev.00/.
 
 Thanks,
 Katja
 
 



Re: RFR: 8032901: WaitForMultipleObjects() return value not handled appropriately

2014-05-15 Thread Staffan Larsen

On 15 maj 2014, at 03:48, David Holmes david.hol...@oracle.com wrote:

 On 14/05/2014 11:18 PM, Aleksej Efimov wrote:
 David, Vitaly,
 
 I totally agree with Vitaly's explanation (Vitaly - thank you for that)
 and code in shmemBase.c (the usage of enterMutex() function for
 sending/receiving bytes through shared memory connection) illustrates on
 how the connection shutdown event is used as a cancellation token.
 
 Thanks for clarifying that.
 
 So if we were to encounter an abandoned mutex the code would presently have 
 acquired the mutex but return an error, thus preventing a subsequent release, 
 and preventing other threads from acquiring (but allowing current thread to 
 recurisevely acquire. So this could both hang and cause data corruption.
 
 The new code will still return an error but release the mutex. So no more 
 hangs (other than by conditions caused by data corruption) but more 
 opportunity for data corruption.
 
 Obviously it depends on exactly what happens in the critical sections guarded 
 by this mutex, but in general I don't agree with this rationale for making 
 the change:
 
 204 /* If the mutex is abandoned we want to return an error
 205  * and also release the mutex so that we don't cause
 206  * other threads to be blocked. If a mutex was abandoned
 207  * we are in scary state. Data may be corrupted or inconsistent
 208  * but it is still better to let other threads run (and possibly
 209  * crash) than having them blocked (on the premise that a crash
 210  * is always easier to debug than a hang).
 
 Considering something has to have gone drastically wrong for the mutex to 
 become abandoned, I'm more inclined to consider this a fatal error and abort.
 
 But I'll let the serviceability folk chime in here.

I was involved in fixing this and writing the comment, so obviously I thought 
it a good solution :-)

I do agree that it would probably be a good idea to consider this a fatal error 
and abort. At that point in the code we don’t have any really nice ways of 
doing that, though. We could just print an error and call abort(). What we are 
doing now is returning an error from sysIPMutexEnter() and delegating the error 
handling to the caller. We have tried to check all call paths to verify that 
they do “the right thing” in the face of an error. It is obviously hard to 
verify, but it looks like they all terminate the connection with some kind of 
error message.

/Staffan


 
 Thanks,
 David
 
 
 Thank you,
 -Aleksej
 
 
 On 05/14/2014 01:05 PM, David Holmes wrote:
 On 14/05/2014 11:06 AM, Vitaly Davidovich wrote:
 In windows, you acquire a mutex by waiting on it using one of the wait
 functions, one of them employed in the code in question.  If
 WaitForMultipleObjects succeeds and returns the index of the mutex,
 current thread has ownership now.
 
 Yes I understand the basic mechanics :)
 
 It's also common to use multi wait functions where the event is a
 cancelation token, e.g. manual reset event; this allows someone to
 cancel waiting on mutex acquisition and return from the wait function.
 Presumably that's the case here, but I'll let Aleksej confirm; just
 wanted to throw this out there in the meantime :).
 
 Ah I see - yes cancellable lock acquisition would make sense.
 
 Thanks,
 David
 
 Sent from my phone
 
 On May 13, 2014 6:46 PM, David Holmes david.hol...@oracle.com
 mailto:david.hol...@oracle.com wrote:
 
Hi Aleksej,
 
Thanks for the doc references regarding abandonment.
 
Let me rephrase my question. What is this logic trying to achieve by
waiting on both a mutex and an event? Do we already own the mutex
when this function is called?
 
David
 
On 13/05/2014 11:19 PM, Aleksej Efimov wrote:
 
David,
 
The Windows has a different terminology for mutex objects (much
differs
from the POSIX one). This one link gave me some understanding of
it [1].
 
Here is the MSDN [1] description of what abandoned mutex is:
 If a thread terminates without releasing its ownership of a
 mutex
object, the mutex object is considered to be abandoned. A
waiting thread
can acquire ownership of an abandoned mutex object, but the wait
function will return*WAIT_ABANDONED*to indicate that the mutex
object is
abandoned. An abandoned mutex object indicates that an error has
occurred and that any shared resource being protected by the
 mutex
object is in an undefined state. If the thread proceeds as
though the
mutex object had not been abandoned, it is no longer considered
abandoned after the thread releases its ownership. This restores
normal
behavior if a handle to the mutex object is subsequently
specified in a
wait function.
 
 
What does it mean to wait on mutex and ownership of the mutex
object:
Any thread with a handle to a mutex object 

Re: RFR(S): 8034960: Serviceability tests using @library failing with java.lang.NoClassDefFoundError

2014-05-12 Thread Staffan Larsen
Pushed: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/05e9c9216e26

Thanks,
/Staffan

On 12 maj 2014, at 10:28, Yekaterina Kantserova 
yekaterina.kantser...@oracle.com wrote:

 Staffan,
 
 could you please be my sponsor and push this fix?
 
 I have discovered three more tests that should be changed:
 sun/management/jmxremote/bootstrap/CustomLauncherTest.java
 java/lang/management/MemoryMXBean/CollectionUsageThreshold.java
 java/lang/management/MemoryMXBean/LowMemoryTest.java
 
 The webrev that includes them can be found here: 
 http://cr.openjdk.java.net/~ykantser/8034960/webrev.02/
 
 Thanks,
 Katja
 
 
 
  Original Message 
 Subject:  Re: RFR(S): 8034960: Serviceability tests using @library 
 failing with java.lang.NoClassDefFoundError
 Date: Fri, 9 May 2014 12:48:56 +0200
 From: Staffan Larsen staffan.lar...@oracle.com
 To:   Yekaterina Kantserova yekaterina.kantser...@oracle.com
 CC:   Alan Bateman alan.bate...@oracle.com, 
 serviceability-...@openjdk.java.net serviceability-...@openjdk.java.net 
 serviceability-...@openjdk.java.net, core-libs-dev Libs 
 core-libs-dev@openjdk.java.net
 
 Looks good!
 
 Thanks,
 /Staffan
 
 On 9 maj 2014, at 12:43, Yekaterina Kantserova 
 yekaterina.kantser...@oracle.com wrote:
 
  Hi,
  
  The version b09 of JTreg which contains 
  https://bugs.openjdk.java.net/browse/CODETOOLS-7900178 has been promoted. 
  So it seems to be time to push the fix for JDK-8034960. I've made a new 
  webrev to be sure the changes fit in in the latest jdk9 source. The webrev 
  can be found here: cr.openjdk.java.net/~ykantser/8034960/webrev.01.
  
  Thanks,
  Katja
  
  
  
  On 03/25/2014 01:14 PM, Staffan Larsen wrote:
  I’ve looked at a random sample of these changes and they look ok.
  
  Since some of the changes are in non-serviceability code I have also added 
  core-libs to the review thread.
  
  I’m sure you know this, but for the record: please don’t push this until 
  jtreg with the fix has been promoted.
  
  Thanks,
  /Staffan
  
  On 25 mar 2014, at 13:07, Yekaterina Kantserova 
  yekaterina.kantser...@oracle.com wrote:
  
  Hi,
  
  Could I please have a review of this fix.
  
  webrev: http://cr.openjdk.java.net/~ykantser/8034960/webrev.00/
  bug: https://bugs.openjdk.java.net/browse/JDK-8034960
  
  When using @library in a JTreg test even @build need to be specify for 
  all library files used by the test. If @build is not specified it can 
  lead to intermittent failures when running tests concurrently, since 
  javac implicit compilation and @library and -concurrency don't play well 
  together.
  
  Verified locally since no JTreg with fix has been promoted yet.
  
  
  Thanks,
  Katja
  
 
 
 
 8034960.open.patch



Re: RFR(S): 8034960: Serviceability tests using @library failing with java.lang.NoClassDefFoundError

2014-05-09 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 9 maj 2014, at 12:43, Yekaterina Kantserova 
yekaterina.kantser...@oracle.com wrote:

 Hi,
 
 The version b09 of JTreg which contains 
 https://bugs.openjdk.java.net/browse/CODETOOLS-7900178 has been promoted. So 
 it seems to be time to push the fix for JDK-8034960. I've made a new webrev 
 to be sure the changes fit in in the latest jdk9 source. The webrev can be 
 found here: cr.openjdk.java.net/~ykantser/8034960/webrev.01.
 
 Thanks,
 Katja
 
 
 
 On 03/25/2014 01:14 PM, Staffan Larsen wrote:
 I’ve looked at a random sample of these changes and they look ok.
 
 Since some of the changes are in non-serviceability code I have also added 
 core-libs to the review thread.
 
 I’m sure you know this, but for the record: please don’t push this until 
 jtreg with the fix has been promoted.
 
 Thanks,
 /Staffan
 
 On 25 mar 2014, at 13:07, Yekaterina Kantserova 
 yekaterina.kantser...@oracle.com wrote:
 
 Hi,
 
 Could I please have a review of this fix.
 
 webrev: http://cr.openjdk.java.net/~ykantser/8034960/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8034960
 
 When using @library in a JTreg test even @build need to be specify for all 
 library files used by the test. If @build is not specified it can lead to 
 intermittent failures when running tests concurrently, since javac implicit 
 compilation and @library and -concurrency don't play well together.
 
 Verified locally since no JTreg with fix has been promoted yet.
 
 
 Thanks,
 Katja
 



Re: Build failures on solaris

2014-05-09 Thread Staffan Larsen
Looks good.

Many apologies.

/Staffan


On 9 maj 2014, at 21:08, Eric McCorkle eric.mccor...@oracle.com wrote:

 The following patch will fix it:
 
 diff -r 7426549b1e3b
 src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c
 --- a/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c
 +++ b/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c
 @@ -1,4 +1,4 @@
 -''/*
 +/*
  * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights
 reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
 
 On 05/09/14 15:01, Eric McCorkle wrote:
 The problem is a stray '' as the first two characters in the file.
 
 I have posted a patch that removes it to serviceability-dev.
 
 All we need is someone with Reviewer/committer rights to step in and
 apply it.
 
 On 05/09/14 14:48, Alejandro E Murillo wrote:
 
 Definitively a P1.
 This is also blocking this week  hotspot snapshot:
 http://prt-web.us.oracle.com//archive/2014/05/2014-05-09-174238.amurillo.jdk9-hs-2014-05-09-jdk9-dev-control/logs/solaris_sparcv9_5.10-fastdebug.log.FAILED.log
 
 please fix ASAP
 
 Thanks
 Alejandro
 
 On 5/9/2014 12:16 PM, Eric McCorkle wrote:
 Bug created: https://bugs.openjdk.java.net/browse/JDK-8042859
 
 This patch seems to be the culprit:
 
 changeset:   9908:7426549b1e3b
 tag: tip
 user:sla
 date:Fri May 09 12:06:13 2014 +0200
 summary: 8039173: Propagate errors from Diagnostic Commands as
 exceptions in the attach framework
 
 
 On 05/09/14 14:08, Eric McCorkle wrote:
 I am currently seeing build failures on solaris, coming from within the
 JDK repo:
 
 /opt/jprt/products/P1/SS12u1/SS12u1/prod/bin/cc -DTRACING
 -DMACRO_MEMSYS_OPS -DBREAKPTS -D_BIG_ENDIAN= -DSOLARIS
 -DARCH='sparcv9' -Dsparcv9 -DNDEBUG -DTRIMMED
 -DRELEASE='1.9.0-internal'
 -I/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/include
 
 -I/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/include/solaris
 
 -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/share/javavm/export
 -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/javavm/export
 -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/share/native/common
 -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/common -m64
 -D__solaris__ -xc99=%none -xCC -errshort=tags -Xa -v -mt -W0,-noglobal
 -KPIC -xstrconst -xregs=no%appl
 -I/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/gensrc_headers
 
 -errtags -errwarn=%all  -g -xs -xO2 -Wc,-Qrm-s -Wc,-Qiselect-T0
 -DTHIS_FILE='SolarisVirtualMachine.c' -c -xMMD -xMF
 /tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/objs/libattach/SolarisVirtualMachine.d.tmp
 
 -o
 /tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/objs/libattach/SolarisVirtualMachine.o
 
 /tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c
 
 /tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c,
 
 line 1: empty character constant
 /tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c,
 
 line 1: syntax error before or at: ''
 gmake[2]: ***
 [/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/objs/libattach/SolarisVirtualMachine.o]
 
 Error 1
 
 This is blocking integration of patches that need to go in promptly.
 This needs to be addressed ASAP.
 
 



Broken build. Re: RFR: 8033104 sun/jvmstat/monitor/MonitoredVm/CR6672135.java failing on all platforms

2014-04-25 Thread Staffan Larsen
It looks like a completely messed this up by not pushing the hotspot parts 
first and now I have broken the build in jdk9-dev. 

Should I push an anti-delta of the patch? I can prepare a review of it in a 
moment.

/Staffan

On 25 apr 2014, at 17:16, Staffan Larsen staffan.lar...@oracle.com wrote:

 Thanks Keith!
 
 As far as I can tell there was no good reason for making the bug 
 Confidential, but I can’t undo it. Sorry about that.
 
 /Staffan
 
 On 25 apr 2014, at 17:02, Keith McGuigan kmcgui...@twitter.com wrote:
 
 Hi Staffan - 
 
 It looks good to me.  Why is the bug marked closed though?
 
 
 On Fri, Apr 25, 2014 at 8:56 AM, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 Still looking for a Review of this change.
 
 Thanks,
 /Staffan
 
 On 7 apr 2014, at 21:19, Staffan Larsen staffan.lar...@oracle.com wrote:
 
  And the links:
 
  bug: https://bugs.openjdk.java.net/browse/JDK-8033104
  webrev: http://cr.openjdk.java.net/~sla/8033104/webrev.00/
 
  Sorry about that,
  /Staffan
 
  On 7 apr 2014, at 20:08, Staffan Larsen staffan.lar...@oracle.com wrote:
 
 
  The problem here is that the code for finding local VMs is not looking 
  for the data in the correct place.
 
  When a JVM is started it will create the perf-data file in a 
  user-specific directory inside /tmp (*). The code in the JDK 
  (PerfDataFile.java) that lists all active JVMs looks for the 
  user-specific directory inside java.io.tmpdir. If a user sets 
  -Djava.io.tmpdir on the command line, the code in PerfDataFile will look 
  in the wrong place.
 
  (*) It's a little bit more complex. /tmp is used on Linux and Solaris. On 
  OS X and Windows, there are user-specific temp directories that should be 
  used, and so the VM queries the OS for these paths.
 
  The solution would be for PerfDataFile to use the same locations as the 
  VM creates them in. The simplest way to guarantee that the same directory 
  is used is to ask the VM to provide the location. Thus I have introduced 
  a new JVM_ function: JVM_GetTemporaryDirectory.
 
  (Since this change touches both hotspot and jdk repos I will submit the 
  hotspot part first under a different bug id (provided that the review 
  goes well)).
 
  The newly added test starts two VM with all possible combinations of 
  setting and not setting java.io.tmpdir to verify that the mechanism is 
  indeed not looking at that variable. I also removed an if-statement in 
  BasicTests.java which would have found this issue a long time ago had it 
  not been there.
 
  Thanks,
  /Staffan
 
 
 
 
 
 -- 
 
 Keith McGuigan
 @kamggg
 kmcgui...@twitter.com
 



Urgent: Broken build. Re: RFR: 8033104 sun/jvmstat/monitor/MonitoredVm/CR6672135.java failing on all platforms

2014-04-25 Thread Staffan Larsen
Here is an anti-delta for the broken push. I prepared it using “hg backout”.

webrev: http://cr.openjdk.java.net/~sla/8041948/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-8041948

If I can get this reviewed quickly I can push the fix soon (and I will spend 
the weekend in shame).

Thanks,
/Staffan


On 25 apr 2014, at 18:24, Staffan Larsen staffan.lar...@oracle.com wrote:

 It looks like a completely messed this up by not pushing the hotspot parts 
 first and now I have broken the build in jdk9-dev. 
 
 Should I push an anti-delta of the patch? I can prepare a review of it in a 
 moment.
 
 /Staffan
 
 On 25 apr 2014, at 17:16, Staffan Larsen staffan.lar...@oracle.com wrote:
 
 Thanks Keith!
 
 As far as I can tell there was no good reason for making the bug 
 Confidential, but I can’t undo it. Sorry about that.
 
 /Staffan
 
 On 25 apr 2014, at 17:02, Keith McGuigan kmcgui...@twitter.com wrote:
 
 Hi Staffan - 
 
 It looks good to me.  Why is the bug marked closed though?
 
 
 On Fri, Apr 25, 2014 at 8:56 AM, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 Still looking for a Review of this change.
 
 Thanks,
 /Staffan
 
 On 7 apr 2014, at 21:19, Staffan Larsen staffan.lar...@oracle.com wrote:
 
  And the links:
 
  bug: https://bugs.openjdk.java.net/browse/JDK-8033104
  webrev: http://cr.openjdk.java.net/~sla/8033104/webrev.00/
 
  Sorry about that,
  /Staffan
 
  On 7 apr 2014, at 20:08, Staffan Larsen staffan.lar...@oracle.com wrote:
 
 
  The problem here is that the code for finding local VMs is not looking 
  for the data in the correct place.
 
  When a JVM is started it will create the perf-data file in a 
  user-specific directory inside /tmp (*). The code in the JDK 
  (PerfDataFile.java) that lists all active JVMs looks for the 
  user-specific directory inside java.io.tmpdir. If a user sets 
  -Djava.io.tmpdir on the command line, the code in PerfDataFile will look 
  in the wrong place.
 
  (*) It's a little bit more complex. /tmp is used on Linux and Solaris. 
  On OS X and Windows, there are user-specific temp directories that 
  should be used, and so the VM queries the OS for these paths.
 
  The solution would be for PerfDataFile to use the same locations as the 
  VM creates them in. The simplest way to guarantee that the same 
  directory is used is to ask the VM to provide the location. Thus I have 
  introduced a new JVM_ function: JVM_GetTemporaryDirectory.
 
  (Since this change touches both hotspot and jdk repos I will submit the 
  hotspot part first under a different bug id (provided that the review 
  goes well)).
 
  The newly added test starts two VM with all possible combinations of 
  setting and not setting java.io.tmpdir to verify that the mechanism is 
  indeed not looking at that variable. I also removed an if-statement in 
  BasicTests.java which would have found this issue a long time ago had it 
  not been there.
 
  Thanks,
  /Staffan
 
 
 
 
 
 -- 
 
 Keith McGuigan
 @kamggg
 kmcgui...@twitter.com
 
 



Re: Urgent: Broken build. Re: RFR: 8033104 sun/jvmstat/monitor/MonitoredVm/CR6672135.java failing on all platforms

2014-04-25 Thread Staffan Larsen
Thanks Joe - fix has been pushed.

(I will now retreat to a dark place and grumble over the impossibility of 
pushing coordinated changes).

/Staffan

On 25 apr 2014, at 18:43, Joe Darcy joe.da...@oracle.com wrote:

 Approved!
 
 -Joe
 
 On 04/25/2014 09:36 AM, Staffan Larsen wrote:
 Here is an anti-delta for the broken push. I prepared it using “hg backout”.
 
 webrev: http://cr.openjdk.java.net/~sla/8041948/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8041948
 
 If I can get this reviewed quickly I can push the fix soon (and I will spend 
 the weekend in shame).
 
 Thanks,
 /Staffan
 
 
 On 25 apr 2014, at 18:24, Staffan Larsen staffan.lar...@oracle.com wrote:
 
 It looks like a completely messed this up by not pushing the hotspot parts 
 first and now I have broken the build in jdk9-dev.
 
 Should I push an anti-delta of the patch? I can prepare a review of it in a 
 moment.
 
 /Staffan
 
 On 25 apr 2014, at 17:16, Staffan Larsen staffan.lar...@oracle.com wrote:
 
 Thanks Keith!
 
 As far as I can tell there was no good reason for making the bug 
 Confidential, but I can’t undo it. Sorry about that.
 
 /Staffan
 
 On 25 apr 2014, at 17:02, Keith McGuigan kmcgui...@twitter.com wrote:
 
 Hi Staffan -
 
 It looks good to me.  Why is the bug marked closed though?
 
 
 On Fri, Apr 25, 2014 at 8:56 AM, Staffan Larsen 
 staffan.lar...@oracle.com wrote:
 Still looking for a Review of this change.
 
 Thanks,
 /Staffan
 
 On 7 apr 2014, at 21:19, Staffan Larsen staffan.lar...@oracle.com wrote:
 
 And the links:
 
 bug: https://bugs.openjdk.java.net/browse/JDK-8033104
 webrev: http://cr.openjdk.java.net/~sla/8033104/webrev.00/
 
 Sorry about that,
 /Staffan
 
 On 7 apr 2014, at 20:08, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 
 The problem here is that the code for finding local VMs is not looking 
 for the data in the correct place.
 
 When a JVM is started it will create the perf-data file in a 
 user-specific directory inside /tmp (*). The code in the JDK 
 (PerfDataFile.java) that lists all active JVMs looks for the 
 user-specific directory inside java.io.tmpdir. If a user sets 
 -Djava.io.tmpdir on the command line, the code in PerfDataFile will 
 look in the wrong place.
 
 (*) It's a little bit more complex. /tmp is used on Linux and Solaris. 
 On OS X and Windows, there are user-specific temp directories that 
 should be used, and so the VM queries the OS for these paths.
 
 The solution would be for PerfDataFile to use the same locations as the 
 VM creates them in. The simplest way to guarantee that the same 
 directory is used is to ask the VM to provide the location. Thus I have 
 introduced a new JVM_ function: JVM_GetTemporaryDirectory.
 
 (Since this change touches both hotspot and jdk repos I will submit the 
 hotspot part first under a different bug id (provided that the review 
 goes well)).
 
 The newly added test starts two VM with all possible combinations of 
 setting and not setting java.io.tmpdir to verify that the mechanism is 
 indeed not looking at that variable. I also removed an if-statement in 
 BasicTests.java which would have found this issue a long time ago had 
 it not been there.
 
 Thanks,
 /Staffan
 
 
 
 -- 
 
 Keith McGuigan
 @kamggg
 kmcgui...@twitter.com
 



Re: Urgent: Broken build. Re: RFR: 8033104 sun/jvmstat/monitor/MonitoredVm/CR6672135.java failing on all platforms

2014-04-25 Thread Staffan Larsen
In this case I think it would have worked ok since the dependency was from jdk 
- hotspot. If the dependency was the other way (or both ways), then such a 
push would break nightly testing in hotspot since that testing picks up the 
latest promoted JDK (instead of the JDK that is in the same forest). This is 
broken, IMO, and there is work in progress on fixing it.

/Staffan


On 25 apr 2014, at 19:04, Alejandro E Murillo alejandro.muri...@oracle.com 
wrote:

 
 what's wrong with pushing them to jdk9/hs-rt?
 We did this a couple of weeks ago with Erik (Gahlin) changes,
 it might disrupt nightly, as we still do not have the JPRT changes in place,
 but that was the agreement we have for jdk9:
 tightly coupled changes should be pushed through the hotspot repos.
 had that been pushed this week there, it would be in jdk9/dev next Tuesday
 
 
 Alejandro
 
 On 4/25/2014 10:46 AM, Staffan Larsen wrote:
 Thanks Joe - fix has been pushed.
 
 (I will now retreat to a dark place and grumble over the impossibility of 
 pushing coordinated changes).
 
 /Staffan
 
 On 25 apr 2014, at 18:43, Joe Darcy joe.da...@oracle.com wrote:
 
 Approved!
 
 -Joe
 
 On 04/25/2014 09:36 AM, Staffan Larsen wrote:
 Here is an anti-delta for the broken push. I prepared it using “hg 
 backout”.
 
 webrev: http://cr.openjdk.java.net/~sla/8041948/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8041948
 
 If I can get this reviewed quickly I can push the fix soon (and I will 
 spend the weekend in shame).
 
 Thanks,
 /Staffan
 
 
 On 25 apr 2014, at 18:24, Staffan Larsen staffan.lar...@oracle.com wrote:
 
 It looks like a completely messed this up by not pushing the hotspot 
 parts first and now I have broken the build in jdk9-dev.
 
 Should I push an anti-delta of the patch? I can prepare a review of it in 
 a moment.
 
 /Staffan
 
 On 25 apr 2014, at 17:16, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 
 Thanks Keith!
 
 As far as I can tell there was no good reason for making the bug 
 Confidential, but I can’t undo it. Sorry about that.
 
 /Staffan
 
 On 25 apr 2014, at 17:02, Keith McGuigan kmcgui...@twitter.com wrote:
 
 Hi Staffan -
 
 It looks good to me.  Why is the bug marked closed though?
 
 
 On Fri, Apr 25, 2014 at 8:56 AM, Staffan Larsen 
 staffan.lar...@oracle.com wrote:
 Still looking for a Review of this change.
 
 Thanks,
 /Staffan
 
 On 7 apr 2014, at 21:19, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 
 And the links:
 
 bug: https://bugs.openjdk.java.net/browse/JDK-8033104
 webrev: http://cr.openjdk.java.net/~sla/8033104/webrev.00/
 
 Sorry about that,
 /Staffan
 
 On 7 apr 2014, at 20:08, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 
 The problem here is that the code for finding local VMs is not 
 looking for the data in the correct place.
 
 When a JVM is started it will create the perf-data file in a 
 user-specific directory inside /tmp (*). The code in the JDK 
 (PerfDataFile.java) that lists all active JVMs looks for the 
 user-specific directory inside java.io.tmpdir. If a user sets 
 -Djava.io.tmpdir on the command line, the code in PerfDataFile will 
 look in the wrong place.
 
 (*) It's a little bit more complex. /tmp is used on Linux and 
 Solaris. On OS X and Windows, there are user-specific temp 
 directories that should be used, and so the VM queries the OS for 
 these paths.
 
 The solution would be for PerfDataFile to use the same locations as 
 the VM creates them in. The simplest way to guarantee that the same 
 directory is used is to ask the VM to provide the location. Thus I 
 have introduced a new JVM_ function: JVM_GetTemporaryDirectory.
 
 (Since this change touches both hotspot and jdk repos I will submit 
 the hotspot part first under a different bug id (provided that the 
 review goes well)).
 
 The newly added test starts two VM with all possible combinations of 
 setting and not setting java.io.tmpdir to verify that the mechanism 
 is indeed not looking at that variable. I also removed an 
 if-statement in BasicTests.java which would have found this issue a 
 long time ago had it not been there.
 
 Thanks,
 /Staffan
 
 
 -- 
 
 Keith McGuigan
 @kamggg
 kmcgui...@twitter.com
 
 -- 
 Alejandro
 



RFR: 8039256 Add sun/jvmstat/monitor/MonitoredVm/CR6672135.java to ProblemList.txt

2014-04-04 Thread Staffan Larsen
Please review the change below to add a test to ProblemList.txt. For details, 
see https://bugs.openjdk.java.net/browse/JDK-8033104

Thanks,
/Staffan



diff --git a/test/ProblemList.txt b/test/ProblemList.txt
--- a/test/ProblemList.txt
+++ b/test/ProblemList.txt
@@ -273,4 +273,7 @@
 # 8031482
 sun/tools/jcmd/TestJcmdSanity.java windows-all

+# 8033104
+sun/jvmstat/monitor/MonitoredVm/CR6672135.java generic-all
+
 

Re: RFR: 8036786: Update jdk7 testlibrary to match jdk8

2014-03-25 Thread Staffan Larsen
I’ve looked at the changes in StreamPumper and ProcessTools: they look good.

Since the rest of the code is a straight backport, I didn’t look at it.

Reviewed.

Thanks,
/Staffan

On 6 mar 2014, at 16:34, Vladimir Kempik vladimir.kem...@oracle.com wrote:

 Please review this change to update jdk part of testlibrary in jdk7.
 
 It updates testlibrary with new apis from jdk8's testlibrary.
 
 When porting jdk8's testlibrary to jdk7, I had to make few changes:
 
 1) rewrite one lambda usage in StreamPumper.java to non-lambda version.
 
 2) get rid of predicate functionality in ProcessTools.java: startProcess as 
 it wasn't used anyway and unsupported in jdk7.
 
 Every test in open part of jdk that uses testlibrary still works after update 
 (in fact there are 3 of them).
 
 Webrev:
 http://cr.openjdk.java.net/~vkempik/8036786/webrev.00/
 
 Thanks, Vladimir.



Re: RFR(S): 8034960: Serviceability tests using @library failing with java.lang.NoClassDefFoundError

2014-03-25 Thread Staffan Larsen
I’ve looked at a random sample of these changes and they look ok. 

Since some of the changes are in non-serviceability code I have also added 
core-libs to the review thread.

I’m sure you know this, but for the record: please don’t push this until jtreg 
with the fix has been promoted.

Thanks,
/Staffan

On 25 mar 2014, at 13:07, Yekaterina Kantserova 
yekaterina.kantser...@oracle.com wrote:

 Hi,
 
 Could I please have a review of this fix.
 
 webrev: http://cr.openjdk.java.net/~ykantser/8034960/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8034960
 
 When using @library in a JTreg test even @build need to be specify for all 
 library files used by the test. If @build is not specified it can lead to 
 intermittent failures when running tests concurrently, since javac implicit 
 compilation and @library and -concurrency don't play well together.
 
 Verified locally since no JTreg with fix has been promoted yet.
 
 
 Thanks,
 Katja



Re: Experimental patches for unsafe cleanup was Re: The s.m.Unsafe representation in hotspot and method registration

2014-03-25 Thread Staffan Larsen
(Adding hotspot-runtime-dev since some changes are in hotspot code).


On 25 mar 2014, at 14:20, Paul Sandoz paul.san...@oracle.com wrote:

 
 On Mar 25, 2014, at 9:34 AM, Paul Sandoz paul.san...@oracle.com wrote:
 
 
 On Mar 24, 2014, at 7:49 PM, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 
 We have abandoned the HSX model. From JDK 8 one version of Hotspot will be 
 tied to one version of the JDK. This looks like old code that has not been 
 cleaned up.
 
 
 Thanks, yes, looks like we can clean this up and also remove the deprecated 
 methods at the same time.
 
 
 Here are some preliminary webrevs to clean up unsafe and also remove all old, 
 deprecated and monitor-related methods:
 
  http://cr.openjdk.java.net/~psandoz/jdk9/jdk-unsafe-cleanup/webrev/jdk.patch
 
  http://cr.openjdk.java.net/~psandoz/jdk9/hotspot-unsafe-cleanup/webrev/
 
 Passes all tests i have thrown at it locally.
 
 There is some additional clean up that could be done to remove prefetch 
 read/write intrinsics hooked up to older versions of Unsafe:
 
  do_intrinsic(_prefetchRead, sun_misc_Unsafe,
 prefetchRead_name, prefetch_signature, F_RN)  \
   do_name( prefetchRead_name,   
 prefetchRead)\
  do_intrinsic(_prefetchWrite,sun_misc_Unsafe,
 prefetchWrite_name, prefetch_signature,F_RN)  \
   do_name( prefetchWrite_name,  
 prefetchWrite)   \
  do_intrinsic(_prefetchReadStatic,   sun_misc_Unsafe,
 prefetchReadStatic_name, prefetch_signature,   F_SN)  \
   do_name( prefetchReadStatic_name, 
 prefetchReadStatic)  \
  do_intrinsic(_prefetchWriteStatic,  sun_misc_Unsafe,
 prefetchWriteStatic_name, prefetch_signature,  F_SN)  \
   do_name( prefetchWriteStatic_name,
 prefetchWriteStatic) \
 
 That will also require updates in various other files. I presume these 
 prefetch methods are never used throughout the runtime?
 
 Paul.



Re: The s.m.Unsafe representation in hotspot and method registration

2014-03-24 Thread Staffan Larsen
We have abandoned the HSX model. From JDK 8 one version of Hotspot will be tied 
to one version of the JDK. This looks like old code that has not been cleaned 
up.

/Staffan

On 24 mar 2014, at 19:13, Peter Levart peter.lev...@gmail.com wrote:

 
 On 03/24/2014 07:02 PM, Paul Sandoz wrote:
 On Mar 24, 2014, at 6:37 PM, Joel Borggrén-Franck joel.fra...@oracle.com 
 wrote:
 
 Hi Paul,
 
 I would guess this is because of the HSX model where we could use the same 
 VM with different majors of the JDK. Would that make sense?
 
 I was wondering about too, but does that still apply to JDKs 1.4.0, 1.4.1, 
 1.5 and 1.6?
 
 Paul.
 This seems very shaky. Couldn't VM find out more reliably what major JDK it 
 is running with?
 
 Regards, Peter
 



Re: JDK 9 RFR of 8038163: Build failure on Mac OS 10.9.2 (Mavericks) due to warning treated as error

2014-03-21 Thread Staffan Larsen
Looks good to me. 

The version with JNI_TRUE/JNI_FALSE is more correct, but I’m fine with either 
one.

(note: this review request should have been sent to serviceability-dev).

Thanks,
/Staffan

On 21 mar 2014, at 20:41, Brian Burkhalter brian.burkhal...@oracle.com wrote:

 Please review at your convenience:
 
 Issue:https://bugs.openjdk.java.net/browse/JDK-8038163
 Patch:http://cr.openjdk.java.net/~bpb/8038163/
 
 My only question would be whether the changed line 291 should instead be this:
 
 return (major  1 || (major == 1  minor = 2)) ? JNI_TRUE : JNI_FALSE;
 
 Thanks,
 
 Brian



Re: RFR 9: 8035889: jdk testlibrary - add printing of values of failed assertions

2014-02-28 Thread Staffan Larsen
Looks good to me!

Thanks,
/Staffan

On 27 feb 2014, at 16:34, roger riggs roger.ri...@oracle.com wrote:

 Hi Mandy,
 
 I updated the webrev:  
   http://cr.openjdk.java.net/~rriggs/webrev-testlibrary-asserts-8035889/ 
 
 Alan suggested copying serviceability-dev so they have a chance to review if 
 desired.
 
 I want to investigate if it is possible to use the TestNG Assert classes 
 without
 the TestNG execution framework.  
 It would be necessary to compile/run against TestNG.jar but it might not 
 need the entire mechanism.
 
 Thanks, Roger
 
 On 2/26/2014 10:17 PM, Mandy Chung wrote:
 On 2/26/2014 7:09 PM, Roger Riggs wrote:
 Hi Mandy,
 
 Yes, it might be more productive to switch the tests to TestNG.
 But it did provide support in cases where TestNG could not be used, 
 for example in a directory of existing tests that had custom reporting.
 
 But I remember there is a problem with TestNG having a dependency for XML
 which is not supported in Profile1 and a number of tests had to be disabled
 in that configuration.  Will XML always be available.  Do we need to solve
 or work around that problem with TestNG?
 
 
 This is a good point.   When we want to test just the base module for 
 example, how can we run TestNG tests?  We need to address that certainly.
 
 My comment on TestNG is a question for new tests using this Asserts class.  
 Your patch is fine to go (after taking out @library tag if I got it 
 correct). 
 
 Mandy
 
 Thanks, Roger
 
 On 2/26/14 9:01 PM, Mandy Chung wrote:
 Hi Roger, 
 
 On 2/26/2014 12:34 PM, roger riggs wrote: 
 The testlibrary for the jdk should be printing the values in the failed 
 assertions to make debugging easier and quicker. 
 
 The webrev adds the printing of the failed assertions and added methods 
 for formatting and unconditional fail methods. 
 
 Webrev: 
 http://cr.openjdk.java.net/~rriggs/webrev-testlibrary-asserts-8035889/ 
 
 
 AssertsTest.java: line 28:  @library doesn't look like it's needed. There 
 is no jdk/test/testlibrary directory and I think   
 jdk.testlibrary.* are found as relative to $test.src. 
 
 Otherwise, the change looks okay. 
 
 Now that jtreg supports TestNG and I wonder if this class should retire 
 some day (there are only about 10 existing tests using this class).  Are 
 you writing new tests using this Asserts class? 
 
 Mandy 
 
 Bug: 
8035889: jdk testlibrary - add printing of values of failed assertions 
 
 Thanks, Roger 
 
 [1] https://bugs.openjdk.java.net/browse/JDK-8035889 
 
 
 
 
 
 



Re: RFR 6835233 : Fedora 9 jdk regression test failed: java/lang/instrument/ParallelTransformerLoader.sh

2014-02-26 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 26 feb 2014, at 20:34, Brent Christian brent.christ...@oracle.com wrote:

 File under chipping away at test stabilization issues.
 
 https://bugs.openjdk.java.net/browse/JDK-6835233
 
 I've done some repeated runs of this test on my Linux machine.  The test 
 fails every time with 6u3.  It fails intermittently on 7 (after 145 
 iterations for 7u45, and 62 iterations for 7u60b07).  I have not been able to 
 reproduce the failure on 8 or 9, running 1000 iterations each on 8b115, 
 8b129, and 9b02.
 
 I would like to resolve this bug by removing the @ignore tag for JDK 9, and 
 bring the test back into rotation.  If the failure comes back, I'll submit a 
 new issue for further investigation.
 
 The change is:
 
 # @bug 5088398
 -# @ignore until bug 6835233 dealt with
 # @summary Test parallel class loading by parallel transformers.
 
 Thanks,
 -Brent



RFR(S): JDK-8033911 Simplify instrumentation of FileInputStream and RandomAccessFile

2014-02-07 Thread Staffan Larsen
A few of the public read and write methods in FileInputStream and 
RandomAccessFile are declared native. This means that it is hard to instrument 
them using byte code instrumentation. Changing the public methods to be to 
non-native and instead calling private native methods simplifies 
instrumentation. 

webrev: http://cr.openjdk.java.net/~sla/8033911/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-8033911

Thanks,
/Staffan

Re: RFR(S): JDK-8033911 Simplify instrumentation of FileInputStream and RandomAccessFile

2014-02-07 Thread Staffan Larsen

On 7 feb 2014, at 11:56, Alan Bateman alan.bate...@oracle.com wrote:

 On 07/02/2014 10:46, Staffan Larsen wrote:
 A few of the public read and write methods in FileInputStream and 
 RandomAccessFile are declared native. This means that it is hard to 
 instrument them using byte code instrumentation. Changing the public methods 
 to be to non-native and instead calling private native methods simplifies 
 instrumentation.
 
 webrev: http://cr.openjdk.java.net/~sla/8033911/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8033911
 
 I assume you know this already but both JVM TI and java.lang.instrument do 
 have support for hooking into the resolution of native methods. It is of 
 course more complicated and not for the fainthearted.

Yes, but a large drawback of using SetNativeMethodPrefix is that it requires 
schema changes to the class file (adding a method). This is not possible when 
dynamically attaching an instrumentation agent.

 The proposed changes look okay. The only thing is that it might not be 
 complete but perhaps it's not too interesting to instrument methods such as 
 skip or available.

That was my thinking, too.

Thanks,
/Staffan

Re: RFR(S): JDK-8033911 Simplify instrumentation of FileInputStream and RandomAccessFile

2014-02-07 Thread Staffan Larsen
I would prefer that to be a different change.

Thanks,
/Staffan

On 7 feb 2014, at 12:07, Dmitry Samersoff dmitry.samers...@oracle.com wrote:

 Staffan,
 
 As far as you touching this.
 
 Is it possible to change all native methods in these two classes to have
 0 at the end of name?
 
 i.e. readBytes = readBytes0
 
 it's pure cosmetic, but fairly simplify core dump reading and later
 grep-ing.
 
 -Dmitry
 
 On 2014-02-07 14:46, Staffan Larsen wrote:
 A few of the public read and write methods in FileInputStream and 
 RandomAccessFile are declared native. This means that it is hard to 
 instrument them using byte code instrumentation. Changing the public methods 
 to be to non-native and instead calling private native methods simplifies 
 instrumentation. 
 
 webrev: http://cr.openjdk.java.net/~sla/8033911/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8033911
 
 Thanks,
 /Staffan
 
 
 
 -- 
 Dmitry Samersoff
 Oracle Java development team, Saint Petersburg, Russia
 * I would love to change the world, but they won't give me the sources.



RFR: JDK-8033917 Keep track of file paths in file streams and channels for instrumentation purposes

2014-02-07 Thread Staffan Larsen
Instrumentation agents that want to instrument FileInputStream/FileOutputStream 
to see which files are being accessed do not currently have access to the file 
system path of the stream. This is because the path is never stored in the 
stream class, only the file descriptor is. (This is also true for 
RandomAccessFile and FileChannel).

An agent could instrument the respective constructors to store the path. The 
problem is where to store it. To add a field, the instrumentation agent needs 
to change the schema of the class. This is not possible at runtime but can be 
done at class-loading time. However for a j.l.instrument agent these classes 
are already defined when the agent is first called. For a native JVMTI agent 
the problem becomes parsing and modifying byte codes in a native agent which is 
error prone and requires a lot of code to maintain.

If instead the stream classes were modified to store a reference to the path, 
it would be readily available for agents at a minimum of cost to the libraries. 
This is what this patch does. FileInputStream, FileOutputStream, 
RandomAccessFile and FileChannelImpl are modified to record the path they 
operate on in a private field. There are no accessors added to retrieve the 
path - it is purely stored for instrumentation purposes. The path is 
intentionally not resolved to be an absolute path since that would potentially 
add unwanted overhead. If a stream is created from a file descriptor, no path 
will be stored. 

The overhead for this path will be keeping the path String alive for a longer 
period of time. I hope this will not cause any problems.

A consumer of this feature will be Java Flight Recorder, but the implementation 
is usable by other agents as well.

webrev: http://cr.openjdk.java.net/~sla/8033917/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-8033917

Thanks,
/Staffan

Re: RFR: JDK-8033917 Keep track of file paths in file streams and channels for instrumentation purposes

2014-02-07 Thread Staffan Larsen

On 7 feb 2014, at 13:27, Alan Bateman alan.bate...@oracle.com wrote:

 On 07/02/2014 12:07, Staffan Larsen wrote:
 Instrumentation agents that want to instrument 
 FileInputStream/FileOutputStream to see which files are being accessed do 
 not currently have access to the file system path of the stream. This is 
 because the path is never stored in the stream class, only the file 
 descriptor is. (This is also true for RandomAccessFile and FileChannel).
 
 An agent could instrument the respective constructors to store the path. The 
 problem is where to store it. To add a field, the instrumentation agent 
 needs to change the schema of the class. This is not possible at runtime but 
 can be done at class-loading time. However for a j.l.instrument agent these 
 classes are already defined when the agent is first called. For a native 
 JVMTI agent the problem becomes parsing and modifying byte codes in a native 
 agent which is error prone and requires a lot of code to maintain.
 
 If instead the stream classes were modified to store a reference to the 
 path, it would be readily available for agents at a minimum of cost to the 
 libraries. This is what this patch does. FileInputStream, FileOutputStream, 
 RandomAccessFile and FileChannelImpl are modified to record the path they 
 operate on in a private field. There are no accessors added to retrieve the 
 path - it is purely stored for instrumentation purposes. The path is 
 intentionally not resolved to be an absolute path since that would 
 potentially add unwanted overhead. If a stream is created from a file 
 descriptor, no path will be stored.
 
 The overhead for this path will be keeping the path String alive for a 
 longer period of time. I hope this will not cause any problems.
 
 A consumer of this feature will be Java Flight Recorder, but the 
 implementation is usable by other agents as well.
 
 webrev: http://cr.openjdk.java.net/~sla/8033917/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8033917
 
 I have reservations about doing this as hints of code making use of private 
 fields which isn't good.
 
 For the comments in FileInputStream and other then it might be best to keep 
 the line lengths consistent with the existing code if you can (it makes 
 future side-by-side reviews a bit easier too).

I’ve updated the comments to have shorter lines and javadoc style comments.

 In WindowsChannelFactory then you've re-order and expand imports. The 
 ordering of the import groups in this area has been Java SE, JDK-specific and 
 then finally the JDK-internal. It's not a big deal of course. Personally I 
 prefer the original static imports but I know some people don’t.

I’ve reverted to the original order, and only expanded the non-static imports. 
(The danger of IDEs).

Updated webrev here:  http://cr.openjdk.java.net/~sla/8033917/webrev.01/

Thanks,
/Staffan



Re: RFR(S): JDK-8033911 Simplify instrumentation of FileInputStream and RandomAccessFile

2014-02-07 Thread Staffan Larsen
Alan, Jaroslav, Dmitry: Thanks!

On 7 feb 2014, at 12:38, Jaroslav Bachorik jaroslav.bacho...@oracle.com wrote:

 Looks good. Additional benefit is the compliance with the secure coding guide.
 
 -JB-
 
 On February 7, 2014 11:46:07 AM CET, Staffan Larsen 
 staffan.lar...@oracle.com wrote:
 A few of the public read and write methods in FileInputStream and 
 RandomAccessFile are declared native. This means that it is hard to 
 instrument them using byte code instrumentation. Changing the public methods 
 to be to non-native and instead calling private native methods simplifies 
 instrumentation. 
 
 webrev: http://cr.openjdk.java.net/~sla/8033911/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8033911
 
 Thanks,
 /Staffan
 
 -- 
 Sent from my Android device with K-9 Mail. Please excuse my brevity.



Re: RFR: JDK-8033917 Keep track of file paths in file streams and channels for instrumentation purposes

2014-02-07 Thread Staffan Larsen

On 7 feb 2014, at 16:24, Dmitry Samersoff dmitry.samers...@oracle.com wrote:

 Staffan,
 
 FileInputStream.java
 
 55:  It's better to initialize path with null.

I agree with Chris here. The value should be explicitly initialized by all 
constructors.

 134: It's better to assign name at one of first lines, in this case we
 will be able to retrieve file name ever if open fails for some reason.

This is the constructor. If anything fails it will throw and exception, and 
there won’t be an object to look at.

 171: It's not necessary

All constructors must initialize the value. 

Thanks,
/Staffan

 
 (the same is applicable to other files)
 
 I'm a bit scared changing signature of public methods of FileChannelImpl
 but if Alan says it's OK - lets go with it.
 
 -Dmitry
 
 
 On 2014-02-07 16:07, Staffan Larsen wrote:
 Instrumentation agents that want to instrument
 FileInputStream/FileOutputStream to see which files are being
 accessed do not currently have access to the file system path of the
 stream. This is because the path is never stored in the stream class,
 only the file descriptor is. (This is also true for RandomAccessFile
 and FileChannel).
 
 An agent could instrument the respective constructors to store the
 path. The problem is where to store it. To add a field, the
 instrumentation agent needs to change the schema of the class. This
 is not possible at runtime but can be done at class-loading time.
 However for a j.l.instrument agent these classes are already defined
 when the agent is first called. For a native JVMTI agent the problem
 becomes parsing and modifying byte codes in a native agent which is
 error prone and requires a lot of code to maintain.
 
 If instead the stream classes were modified to store a reference to
 the path, it would be readily available for agents at a minimum of
 cost to the libraries. This is what this patch does. FileInputStream,
 FileOutputStream, RandomAccessFile and FileChannelImpl are modified
 to record the path they operate on in a private field. There are no
 accessors added to retrieve the path - it is purely stored for
 instrumentation purposes. The path is intentionally not resolved to
 be an absolute path since that would potentially add unwanted
 overhead. If a stream is created from a file descriptor, no path will
 be stored.
 
 The overhead for this path will be keeping the path String alive for
 a longer period of time. I hope this will not cause any problems.
 
 A consumer of this feature will be Java Flight Recorder, but the
 implementation is usable by other agents as well.
 
 webrev: http://cr.openjdk.java.net/~sla/8033917/webrev.00/ bug:
 https://bugs.openjdk.java.net/browse/JDK-8033917
 
 Thanks, /Staffan
 
 
 
 -- 
 Dmitry Samersoff
 Oracle Java development team, Saint Petersburg, Russia
 * I would love to change the world, but they won't give me the sources.



Re: 8032456: vm/jni/Miscellaneous/misc001/misc00101m1/misc00101m1.html failing on OS X

2014-01-24 Thread Staffan Larsen
Looks good!  :-)

Thanks,
/Staffan

On 24 jan 2014, at 12:00, Alan Bateman alan.bate...@oracle.com wrote:

 
 I need a reviewer to fix an issue with the changes in JDK 8 to support 
 statically linked JNI libraries. The issue arises with tests that have both 
 JNI_OnLoad and JNI_OnLoad_libname functions defined, and code attempts to 
 load the library more than once. In that scenario then both functions are 
 called.
 
 Staffan Larsen did the hard work in JDK-8031968 where he observed that 
 dlopen(NULL, RTLD_LAZY) behaves differently on OS X and that we should be 
 using dlopen(NULL, RTLD_FIRST) to get the handle of the main program. Staffan 
 has fixed this in the hotspot repository for JVM TI agent libraries, we need 
 to do the same in the libjava code used by ClassLoader's native methods.
 
 Note that the include of string.h is not directly part of the issue here, 
 instead it's just a drive-by fix to the warning related to the strcat usages.
 
 -Alan
 
 
 diff --git a/src/solaris/native/common/jni_util_md.c 
 b/src/solaris/native/common/jni_util_md.c
 --- a/src/solaris/native/common/jni_util_md.c
 +++ b/src/solaris/native/common/jni_util_md.c
 @@ -23,6 +23,8 @@
  * questions.
  */
 
 +#include string.h
 +
 #include jni.h
 #include jni_util.h
 #include dlfcn.h
 @@ -40,7 +42,11 @@
 if (procHandle != NULL) {
 return procHandle;
 }
 -procHandle = (void*)dlopen(NULL, RTLD_LAZY);
 +#ifdef __APPLE__
 +procHandle = (void*)dlopen(NULL, RTLD_FIRST);
 +#else
 +procHandle = (void*)dlopen(NULL, RTLD_LAZY);
 +#endif
 return procHandle;
 }
 



Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Staffan Larsen
Volker,

I’ve look at the following files:

src/share/native/sun/management/DiagnosticCommandImpl.c:
nit: “legel” - “legal” (two times)
In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if you 
allow dcmd_info_array to become NULL, then 
jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to 
check that.

src/solaris/native/sun/management/OperatingSystemImpl.c
No comments.

src/share/transport/socket/socketTransport.c
No comments.

src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
No comments.


Thanks,
/Staffan



On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote:

 Hi,
 
 could you please review the following changes for the ppc-aix-port 
 stage/stage-9 repositories (the changes are planned for integration into 
 ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):
 
 http://cr.openjdk.java.net/~simonis/webrevs/8031581/
 
 I've build and smoke tested without any problems on Linux/x86_64 and PPC64, 
 Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.
 
 With these changes (and together with the changes from 8028537: PPC64: 
 Updated jdk/test scripts to understand the AIX os and environment and 
 8031134 : PPC64: implement printing on AIX) our port passes all but the 
 following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 
 baseline from www.java.net/download/jdk8/testresults/testresults.html‎):
 
 java/net/Inet6Address/B6558853.java
 java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically)
 java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java
 java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically)
 java/nio/channels/Selector/RacyDeregister.java
 sun/security/krb5/auto/Unreachable.java (only on IPv6)
 
 Thank you and best regards,
 Volker
 
 
 Following a detailed description of the various changes:
 src/share/native/java/util/zip/zip_util.c
 src/share/native/sun/management/DiagnosticCommandImpl.c
 
 According to ISO C it is perfectly legal for malloc to return zero if called 
 with a zero argument. Fix various places where malloc can potentially 
 correctly return zero because it was called with a zero argument.
 Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a 
 compiler warning on Linux, but on AIX it prevents a VM crash later on because 
 the return value of malloc() will be casted to int which is especially bad if 
 that pointer was bigger than 32-bit.
 make/CompileJavaClasses.gmk
 
 Also use PollingWatchService on AIX.
 make/lib/NioLibraries.gmk
 src/aix/native/sun/nio/ch/AixNativeThread.c
 
 Put the implementation for the native methods of NativeThread into 
 AixNativeThread.c on AIX.
 src/solaris/native/sun/nio/ch/PollArrayWrapper.c
 src/solaris/native/sun/nio/ch/Net.c
 src/aix/classes/sun/nio/ch/AixPollPort.java
 src/aix/native/sun/nio/ch/AixPollPort.c
 src/aix/native/java/net/aix_close.c
 
 On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT, ...) 
 are defined to different values than on other operating systems. The problem 
 is however, that these constants are hardcoded as public final static members 
 of various, shared Java classes. We therefore have to map them from Java to 
 native every time before calling one of the native poll functions and back to 
 Java after the call on AIX in order to get the right semantics.
 src/share/classes/java/nio/file/CopyMoveHelper.java
 
 As discussed on the core-libs mailing list (see 
 http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/024119.html)
  it is not necessary to call Files.getFileAttributeView() with any 
 linkOptions because at that place we've already checked that the target file 
 can not be a symbolic link. This change makes the implementation more robust 
 on platforms which support symbolic links but do not support the O_NOFOLLOW 
 flag to the open system call. It also makes the JDK pass the 
 demo/zipfs/basic.sh test on AIX.
 src/share/classes/sun/nio/cs/ext/ExtendedCharsets.java
 
 Support compound text on AIX in the same way like on other Unix platforms.
 src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
 
 Define the correct attach provider for AIX.
 src/solaris/native/java/net/net_util_md.h
 src/solaris/native/sun/nio/ch/FileDispatcherImpl.c
 src/solaris/native/sun/nio/ch/ServerSocketChannelImpl.c
 
 AIX needs a workaround for I/O cancellation (see: 
 http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf1/close.htm).
  ..The close() subroutine is blocked until all subroutines which use the 
 file descriptor return to usr space. For example, when a thread is calling 
 close and another thread is calling select with the same file descriptor, the 
 close subroutine does not return until the select call returns To fix 
 this problem, we have to use the various NET_ wrappers which are declared in 
 

Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Staffan Larsen
Yes, that looks like a good solution.

/Staffan

On 15 jan 2014, at 17:34, Volker Simonis volker.simo...@gmail.com wrote:

 Hi Staffan,
 
 thanks for the review. Please find my comments inline:
 
 On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 Volker,
 
 I’ve look at the following files:
 
 src/share/native/sun/management/DiagnosticCommandImpl.c:
 nit: “legel” - “legal” (two times)
 In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if 
 you allow dcmd_info_array to become NULL, then 
 jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to 
 check that.
 
 Good catch. I actually had problems with malloc returning NULL in 
 'getDiagnosticCommandArgumentInfoArray()' and then changed all other 
 potentially dangerous locations which used the same pattern.
 
 However I think if the 'dcmd_info_array' has zero length it would be 
 perfectly fine to return a zero length array. So what about the following 
 solution:
 
   dcmdInfoCls = (*env)-FindClass(env,
   sun/management/DiagnosticCommandInfo);
   num_commands = (*env)-GetArrayLength(env, commands);
   if (num_commands = 0) {
   result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL);
   if (result == NULL) {
   JNU_ThrowOutOfMemoryError(env, 0);
   }
   else {
   return result;
   }
   }
   dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
   if (dcmd_info_array == NULL) {
   JNU_ThrowOutOfMemoryError(env, NULL);
   }
   jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
   result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 
 That seems easier and saves me from handling the exception.
 
 What do you think?
 
 src/solaris/native/sun/management/OperatingSystemImpl.c
 No comments.
 
 src/share/transport/socket/socketTransport.c
 No comments.
 
 src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
 No comments.
 
 
 Thanks,
 /Staffan
 
 
 
 On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote:
 
 Hi,
 
 could you please review the following changes for the ppc-aix-port 
 stage/stage-9 repositories (the changes are planned for integration into 
 ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):
 
 http://cr.openjdk.java.net/~simonis/webrevs/8031581/
 
 I've build and smoke tested without any problems on Linux/x86_64 and PPC64, 
 Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.
 
 With these changes (and together with the changes from 8028537: PPC64: 
 Updated jdk/test scripts to understand the AIX os and environment and 
 8031134 : PPC64: implement printing on AIX) our port passes all but the 
 following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 
 baseline from www.java.net/download/jdk8/testresults/testresults.html‎):
 
 java/net/Inet6Address/B6558853.java
 java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically)
 java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java
 java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically)
 java/nio/channels/Selector/RacyDeregister.java
 sun/security/krb5/auto/Unreachable.java (only on IPv6)
 
 Thank you and best regards,
 Volker
 
 
 Following a detailed description of the various changes:
 src/share/native/java/util/zip/zip_util.c
 src/share/native/sun/management/DiagnosticCommandImpl.c
 
 According to ISO C it is perfectly legal for malloc to return zero if called 
 with a zero argument. Fix various places where malloc can potentially 
 correctly return zero because it was called with a zero argument.
 Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a 
 compiler warning on Linux, but on AIX it prevents a VM crash later on 
 because the return value of malloc() will be casted to int which is 
 especially bad if that pointer was bigger than 32-bit.
 make/CompileJavaClasses.gmk
 
 Also use PollingWatchService on AIX.
 make/lib/NioLibraries.gmk
 src/aix/native/sun/nio/ch/AixNativeThread.c
 
 Put the implementation for the native methods of NativeThread into 
 AixNativeThread.c on AIX.
 src/solaris/native/sun/nio/ch/PollArrayWrapper.c
 src/solaris/native/sun/nio/ch/Net.c
 src/aix/classes/sun/nio/ch/AixPollPort.java
 src/aix/native/sun/nio/ch/AixPollPort.c
 src/aix/native/java/net/aix_close.c
 
 On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT, 
 ...) are defined to different values than on other operating systems. The 
 problem is however, that these constants are hardcoded as public final 
 static members of various, shared Java classes. We therefore have to map 
 them from Java to native every time before calling one of the native poll 
 functions and back to Java after the call on AIX in order to get the right 
 semantics.
 src/share/classes/java/nio/file/CopyMoveHelper.java
 
 As discussed on the core-libs mailing list (see 
 http

Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Staffan Larsen

On 15 jan 2014, at 18:27, Volker Simonis volker.simo...@gmail.com wrote:

 On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis
 volker.simo...@gmail.com wrote:
 Hi Staffan,
 
 thanks for the review. Please find my comments inline:
 
 On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com
 wrote:
 
 Volker,
 
 I’ve look at the following files:
 
 src/share/native/sun/management/DiagnosticCommandImpl.c:
 nit: “legel” - “legal” (two times)
 In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if
 you allow dcmd_info_array to become NULL, then
 jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to
 check that.
 
 
 Good catch. I actually had problems with malloc returning NULL in
 'getDiagnosticCommandArgumentInfoArray()' and then changed all other
 potentially dangerous locations which used the same pattern.
 
 However I think if the 'dcmd_info_array' has zero length it would be
 perfectly fine to return a zero length array. So what about the following
 solution:
 
  dcmdInfoCls = (*env)-FindClass(env,
  sun/management/DiagnosticCommandInfo);
  num_commands = (*env)-GetArrayLength(env, commands);
 
 Sorry, of course I wanted to say if (num_commands == 0) here!

I understood as much :-)

 
  if (num_commands = 0) {
  result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL);
  if (result == NULL) {
  JNU_ThrowOutOfMemoryError(env, 0);
  }
  else {
  return result;
  }
  }
  dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
  if (dcmd_info_array == NULL) {
  JNU_ThrowOutOfMemoryError(env, NULL);
  }
  jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
  result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 
 That seems easier and saves me from handling the exception.
 
 What do you think?
 
 src/solaris/native/sun/management/OperatingSystemImpl.c
 No comments.
 
 src/share/transport/socket/socketTransport.c
 No comments.
 
 
 src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
 No comments.
 
 
 Thanks,
 /Staffan
 
 
 
 On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote:
 
 Hi,
 
 could you please review the following changes for the ppc-aix-port
 stage/stage-9 repositories (the changes are planned for integration into
 ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):
 
 http://cr.openjdk.java.net/~simonis/webrevs/8031581/
 
 I've build and smoke tested without any problems on Linux/x86_64 and
 PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.
 
 With these changes (and together with the changes from 8028537: PPC64:
 Updated jdk/test scripts to understand the AIX os and environment and
 8031134 : PPC64: implement printing on AIX) our port passes all but the
 following 7 jtreg regression tests on AIX (compared to the Linux/x86_64
 baseline from www.java.net/download/jdk8/testresults/testresults.html‎):
 
 java/net/Inet6Address/B6558853.java
 java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically)
 java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java
 java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically)
 java/nio/channels/Selector/RacyDeregister.java
 sun/security/krb5/auto/Unreachable.java (only on IPv6)
 
 Thank you and best regards,
 Volker
 
 
 Following a detailed description of the various changes:
 
 src/share/native/java/util/zip/zip_util.c
 src/share/native/sun/management/DiagnosticCommandImpl.c
 
 According to ISO C it is perfectly legal for malloc to return zero if
 called with a zero argument. Fix various places where malloc can potentially
 correctly return zero because it was called with a zero argument.
 Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a
 compiler warning on Linux, but on AIX it prevents a VM crash later on
 because the return value of malloc() will be casted to int which is
 especially bad if that pointer was bigger than 32-bit.
 
 make/CompileJavaClasses.gmk
 
 Also use PollingWatchService on AIX.
 
 make/lib/NioLibraries.gmk
 src/aix/native/sun/nio/ch/AixNativeThread.c
 
 Put the implementation for the native methods of NativeThread into
 AixNativeThread.c on AIX.
 
 src/solaris/native/sun/nio/ch/PollArrayWrapper.c
 src/solaris/native/sun/nio/ch/Net.c
 src/aix/classes/sun/nio/ch/AixPollPort.java
 src/aix/native/sun/nio/ch/AixPollPort.c
 src/aix/native/java/net/aix_close.c
 
 On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT,
 ...) are defined to different values than on other operating systems. The
 problem is however, that these constants are hardcoded as public final
 static members of various, shared Java classes. We therefore have to map
 them from Java to native every time before calling one of the native poll
 functions and back to Java after the call on AIX in order to get the right
 semantics.
 
 src/share

Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Staffan Larsen

On 15 jan 2014, at 18:52, Volker Simonis volker.simo...@gmail.com wrote:

 
 
 On Wed, Jan 15, 2014 at 6:27 PM, Volker Simonis volker.simo...@gmail.com 
 wrote:
  On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis
  volker.simo...@gmail.com wrote:
  Hi Staffan,
 
  thanks for the review. Please find my comments inline:
 
  On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com
  wrote:
 
  Volker,
 
  I’ve look at the following files:
 
  src/share/native/sun/management/DiagnosticCommandImpl.c:
  nit: “legel” - “legal” (two times)
  In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if
  you allow dcmd_info_array to become NULL, then
  jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need 
  to
  check that.
 
 
  Good catch. I actually had problems with malloc returning NULL in
  'getDiagnosticCommandArgumentInfoArray()' and then changed all other
  potentially dangerous locations which used the same pattern.
 
  However I think if the 'dcmd_info_array' has zero length it would be
  perfectly fine to return a zero length array. So what about the following
  solution:
 
 
 Sorry for the noise - it seems I was a little indisposed during the last 
 mails:)
 So this is the simple change I'd like to propose for 
 Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo():
 
 
 @@ -117,19 +119,23 @@
return NULL;
}
num_commands = (*env)-GetArrayLength(env, commands);
 -  dcmd_info_array = (dcmdInfo*) malloc(num_commands *
 -   sizeof(dcmdInfo));
 +  dcmdInfoCls = (*env)-FindClass(env,
 +  sun/management/DiagnosticCommandInfo);
 +  result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 +  if (result == NULL) {
 +  JNU_ThrowOutOfMemoryError(env, 0);
 +  }
 +  if (num_commands == 0) {
 +  /* Handle the 'zero commands' case specially to avoid calling 
 'malloc()' */
 +  /* with a zero argument because that may legally return a NULL 
 pointer.  */
 +  return result;
 +  }
 +  dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
if (dcmd_info_array == NULL) {
JNU_ThrowOutOfMemoryError(env, NULL);
}
jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
 -  dcmdInfoCls = (*env)-FindClass(env,
 -  sun/management/DiagnosticCommandInfo);
 -  result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 -  if (result == NULL) {
 -  free(dcmd_info_array);
 -  JNU_ThrowOutOfMemoryError(env, 0);
 -  }
for (i=0; inum_commands; i++) {
args = getDiagnosticCommandArgumentInfoArray(env,
 
 (*env)-GetObjectArrayElement(env,commands,i),
 
 If the 'commands' input array is of zero length just return a zero length 
 array.
 OK?

Yes, this looks good (still :-) )

/Staffan


 
dcmdInfoCls = (*env)-FindClass(env,
sun/management/DiagnosticCommandInfo);
num_commands = (*env)-GetArrayLength(env, commands);
 
  Sorry, of course I wanted to say if (num_commands == 0) here!
 
if (num_commands = 0) {
result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL);
if (result == NULL) {
JNU_ThrowOutOfMemoryError(env, 0);
}
else {
return result;
}
}
dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
if (dcmd_info_array == NULL) {
JNU_ThrowOutOfMemoryError(env, NULL);
}
jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 
  That seems easier and saves me from handling the exception.
 
  What do you think?
 
  src/solaris/native/sun/management/OperatingSystemImpl.c
  No comments.
 
  src/share/transport/socket/socketTransport.c
  No comments.
 
 
  src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
  No comments.
 
 
  Thanks,
  /Staffan
 
 
 
  On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote:
 
  Hi,
 
  could you please review the following changes for the ppc-aix-port
  stage/stage-9 repositories (the changes are planned for integration into
  ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):
 
  http://cr.openjdk.java.net/~simonis/webrevs/8031581/
 
  I've build and smoke tested without any problems on Linux/x86_64 and
  PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.
 
  With these changes (and together with the changes from 8028537: PPC64:
  Updated jdk/test scripts to understand the AIX os and environment and
  8031134 : PPC64: implement printing on AIX) our port passes all but the
  following 7 jtreg regression tests on AIX (compared to the Linux/x86_64
  baseline from www.java.net/download/jdk8/testresults/testresults.html‎):
 
  java/net

Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-13 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 13 jan 2014, at 07:21, Tristan Yan tristan@oracle.com wrote:

 Hi All
 I add more trace output to track down possible reason of this failure. Please 
 help to review it again.
 
 http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.05/
 Thank you
 Tristan
 
 On 01/10/2014 07:20 AM, Tristan Yan wrote:
 Hi David
 I wasn't able to reproduce this failure either in local or in our same 
 binaries running(This is a continuous running with same JDK binaries). So 
 intention for this code change is bringing this test back;  add some debug 
 info and try to avoid possible issues in this test. I agree this code change 
 won't solve the failure happened. But this test was put into ProblemList two 
 years ago better move for this is move out it from ProblemList and trace 
 down the issue in our normal nightly.
 Thank you
 Tristan
 
 On 01/10/2014 06:35 AM, David Holmes wrote:
 On 9/01/2014 10:14 PM, Alan Bateman wrote:
 On 09/01/2014 11:27, David Holmes wrote:
 
 Okay I think I get it now. Both MonitorTest and WaitersTest use the
 Context class, so if both tests run in the same VM the second test
 will see the static total_turns_taken and turn in the state they
 were left from the first test - hence the second test will always
 fail. The bug report suggests making the tests othervm to avoid the
 problem but instead you have changed from using static state to
 instance state so that there is no interference.
 I haven't been following this one closely but I thought that jtreg
 created a class loader for each test (irrespective of mode) so I
 wouldn't expect statics to be an issue.
 
 That aside DemoRun forks off its own JVM to run a given test anyway!
 
 So I don't understand how the proposed fixes could actually be addressing 
 the hangs that are occurring. Even if the statics were being shared I don't 
 see how that leads to the failure mode in the bug report.
 
 David
 
 -Alan.
 
 



Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-10 Thread Staffan Larsen

On 10 jan 2014, at 09:34, Alan Bateman alan.bate...@oracle.com wrote:

 On 09/01/2014 23:20, Tristan Yan wrote:
 Hi David
 I wasn't able to reproduce this failure either in local or in our same 
 binaries running(This is a continuous running with same JDK binaries). So 
 intention for this code change is bringing this test back;  add some debug 
 info and try to avoid possible issues in this test. I agree this code change 
 won't solve the failure happened. But this test was put into ProblemList two 
 years ago better move for this is move out it from ProblemList and trace 
 down the issue in our normal nightly.
 If we can't duplicate it now, and the output from previously reported 
 failures (in 2011) is insufficient to diagnose it properly, then it seems 
 reasonable to add better output so as to improve our chances of understanding 
 if it fails again. So better output + removing from the exclude list seems 
 fine here. (cc'ing serviceability-dev as that is actually the mailing list 
 for the HPROF and JVM TI areas).

Sounds good to me,
/Staffan

 
 -Alan



Re: [8] WXP minor fixes for a cleaner compile of c code

2013-12-10 Thread Staffan Larsen
I see you were directed here from the build-dev list. Unfortunately these are 
core library fixes, not hotspot fixes. I’ve added core-libs and bcc:d 
hotspot-dev.

Thanks,
/Staffan

On 11 dec 2013, at 07:34, Francis ANDRE francis.andre.kampb...@orange.fr 
wrote:

 
 Hi
 
 Below are some warnings produced by the build of jdk8.
 
 Z:/JDK/jdk8/jdk/src/share/native/java/lang/Throwable.c(48) : warning C4028:
 paramétre formel 3 différent de la déclaration
 Z:/JDK/jdk8/jdk/src/windows/native/java/io/WinNTFileSystem_md.c(363) : warning
 C4101: 'pathlen': variable locale non référencée
 Z:/JDK/jdk8/jdk/src/windows/native/common/jdk_util_md.c(45) : warning C4101:
 'ret': variable locale non référencée
 Z:/JDK/jdk8/jdk/src/share/bin/java.c(1253) : warning C4101: 'result': variable
 locale nonréférencée
 Z:/JDK/jdk8/jdk/src/share/bin/parse_manifest.c(196) : warning C4244: 
 'fonction':
 conversion de 'jlong' en 'unsigned int', perte possible de données
 
 
 
 And here are the fixes
 
 diff --git a/src/share/bin/java.c b/src/share/bin/java.c
 --- a/src/share/bin/java.c
 +++ b/src/share/bin/java.c
 @@ -1250,7 +1250,6 @@
 GetApplicationClass(JNIEnv *env)
 {
 jmethodID mid;
 -jobject result;
 jclass cls = GetLauncherHelperClass(env);
 NULL_CHECK0(cls);
 NULL_CHECK0(mid = (*env)-GetStaticMethodID(env, cls,
 diff --git a/src/share/bin/parse_manifest.c b/src/share/bin/parse_manifest.c
 --- a/src/share/bin/parse_manifest.c
 +++ b/src/share/bin/parse_manifest.c
 @@ -193,7 +193,7 @@
 return (-1);
 if ((buffer = malloc(END_MAXLEN)) == NULL)
 return (-1);
 -if ((bytes = read(fd, buffer, len))  0) {
 +if ((bytes = read(fd, buffer, (size_t)len))  0) {
 free(buffer);
 return (-1);
 }
 diff --git a/src/share/native/java/lang/Throwable.c
 b/src/share/native/java/lang/Throwable.c
 --- a/src/share/native/java/lang/Throwable.c
 +++ b/src/share/native/java/lang/Throwable.c
 @@ -44,7 +44,7 @@
  * `this' so you can write 'throw e.fillInStackTrace();'
  */
 JNIEXPORT jobject JNICALL
 -Java_java_lang_Throwable_fillInStackTrace(JNIEnv *env, jobject throwable, int
 dummy)
 +Java_java_lang_Throwable_fillInStackTrace(JNIEnv *env, jobject throwable, 
 jint
 dummy)
 {
 JVM_FillInStackTrace(env, throwable);
 return throwable;
 diff --git a/src/windows/native/common/jdk_util_md.c
 b/src/windows/native/common/jdk_util_md.c
 --- a/src/windows/native/common/jdk_util_md.c
 +++ b/src/windows/native/common/jdk_util_md.c
 @@ -42,7 +42,6 @@
 JNIEXPORT HMODULE JDK_LoadSystemLibrary(const char* name) {
 HMODULE handle = NULL;
 char path[MAX_PATH];
 -int ret;
 
 if (GetSystemDirectory(path, sizeof(path)) != 0) {
 strcat(path, \\);
 diff --git a/src/windows/native/java/io/WinNTFileSystem_md.c
 b/src/windows/native/java/io/WinNTFileSystem_md.c
 --- a/src/windows/native/java/io/WinNTFileSystem_md.c
 +++ b/src/windows/native/java/io/WinNTFileSystem_md.c
 @@ -360,7 +360,6 @@
   jobject file)
 {
 jint rv = 0;
 -jint pathlen;
 
 WCHAR *pathbuf = fileToNTPath(env, file, ids.path);
 if (pathbuf == NULL)
 
 
 



hg: jdk8/tl/jdk: 6461635: [TESTBUG] BasicTests.sh test fails intermittently

2013-12-04 Thread staffan . larsen
Changeset: d30f49aa2d01
Author:sla
Date:  2013-12-03 17:06 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d30f49aa2d01

6461635: [TESTBUG] BasicTests.sh test fails intermittently
Summary: Transform dummy class instead of BigInteger to avoid complication by 
-Xshare. Ported from script to java.
Reviewed-by: alanb
Contributed-by: mattias.tobias...@oracle.com

! test/ProblemList.txt
- test/com/sun/tools/attach/AgentSetup.sh
! test/com/sun/tools/attach/Application.java
- test/com/sun/tools/attach/ApplicationSetup.sh
! test/com/sun/tools/attach/BasicTests.java
- test/com/sun/tools/attach/BasicTests.sh
- test/com/sun/tools/attach/CommonSetup.sh
! test/com/sun/tools/attach/PermissionTest.java
- test/com/sun/tools/attach/PermissionTests.sh
! test/com/sun/tools/attach/ProviderTest.java
- test/com/sun/tools/attach/ProviderTests.sh
! test/com/sun/tools/attach/RedefineAgent.java
+ test/com/sun/tools/attach/RedefineDummy.java
+ test/com/sun/tools/attach/RunnerUtil.java
! test/lib/testlibrary/jdk/testlibrary/ProcessThread.java
! test/lib/testlibrary/jdk/testlibrary/ProcessTools.java
! test/lib/testlibrary/jdk/testlibrary/Utils.java
! test/sun/tools/jstatd/JstatdTest.java



hg: jdk8/tl/jdk: 8028632: Update jdk/test/ProblemList.txt to reflect fix JDK-8024423

2013-11-21 Thread staffan . larsen
Changeset: fc9f24b9408e
Author:sla
Date:  2013-11-21 12:57 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/fc9f24b9408e

8028632: Update jdk/test/ProblemList.txt to reflect fix JDK-8024423
Summary: Removed 5 testcases from the ProblemList
Reviewed-by: sla
Contributed-by: balchandra.vai...@oracle.com

! test/ProblemList.txt



hg: jdk8/tl/jdk: 8023138: [TEST_BUG] java/lang/instrument/PremainClass/NoPremainAgent.sh fails intermittently

2013-11-18 Thread staffan . larsen
Changeset: 64a492bc0ba7
Author:sla
Date:  2013-11-14 12:35 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/64a492bc0ba7

8023138: [TEST_BUG] java/lang/instrument/PremainClass/NoPremainAgent.sh fails 
intermittently
Summary: Port tests for java/lang/instrument/PremainClass from script to java
Reviewed-by: sla
Contributed-by: mattias.tobias...@oracle.com

- test/java/lang/instrument/PremainClass/NoPremainAgent.sh
+ test/java/lang/instrument/PremainClass/NoPremainAgentTest.java
+ test/java/lang/instrument/PremainClass/PremainClassTest.java
- test/java/lang/instrument/PremainClass/PremainClassTest.sh
- test/java/lang/instrument/PremainClass/ZeroArgPremainAgent.sh
+ test/java/lang/instrument/PremainClass/ZeroArgPremainAgentTest.java
! test/lib/testlibrary/jdk/testlibrary/Utils.java



Re: Review Request for ProblemList.txt update

2013-11-18 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 19 Nov 2013, at 04:34, Mandy Chung mandy.ch...@oracle.com wrote:

 java/lang/management/ThreadMXBean/ThreadStateTest.java has been fixed in [1].
 
 This patch removes it from jdk/test/ProblemList.txt:
 
 $ hg diff ProblemList.txt
 diff --git a/test/ProblemList.txt b/test/ProblemList.txt
 --- a/test/ProblemList.txt
 +++ b/test/ProblemList.txt
 @@ -122,9 +122,6 @@
 
 # jdk_lang
 
 -# 6944188
 -java/lang/management/ThreadMXBean/ThreadStateTest.java  generic-all
 -
 # 7067973
 java/lang/management/MemoryMXBean/CollectionUsageThreshold.java generic-all
 
 thanks
 Mandy
 [1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/85fd2ae0a845
 



hg: jdk8/tl/jdk: 8015497: Take new fixes from hotspot/test/testlibrary to jdk/test/lib/testlibrary

2013-11-13 Thread staffan . larsen
Changeset: a42a416351b8
Author:ykantser
Date:  2013-11-13 11:46 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a42a416351b8

8015497: Take new fixes from hotspot/test/testlibrary to 
jdk/test/lib/testlibrary
Reviewed-by: sla

+ test/lib/testlibrary/AssertsTest.java
+ test/lib/testlibrary/OutputAnalyzerReportingTest.java
+ test/lib/testlibrary/jdk/testlibrary/InputArguments.java
! test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
- test/lib/testlibrary/jdk/testlibrary/JdkFinder.java
! test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java
! test/lib/testlibrary/jdk/testlibrary/ProcessTools.java
! test/sun/management/jmxremote/bootstrap/CustomLauncherTest.java



Re: Review request for 8028234: Remove unused methods in sun.misc.JavaAWTAccess

2013-11-13 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 12 Nov 2013, at 20:29, Mandy Chung mandy.ch...@oracle.com wrote:

 This is a simple code deletion in sun.misc.JavaAWTAccess and its 
 implementation class:
 
 Webrev:
 http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8028234/webrev.00/
 
 This patch removes the methods from sun.misc.JavaAWTAccess that are no longer 
 used.  The only dependency remaining from core-libs to AppContext is the 
 ability to obtain an opaque unique object to represent an applet context ifit 
 is running in an applet environment.
 
 thanks
 Mandy



hg: jdk8/tl/jdk: 8014506: Test of Jdp feature

2013-11-11 Thread staffan . larsen
Changeset: 0cacac7f5c37
Author:sla
Date:  2013-11-08 18:16 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0cacac7f5c37

8014506: Test of Jdp feature
Reviewed-by: sla
Contributed-by: Alex Schenkman alex.schenk...@oracle.com

+ test/sun/management/jdp/ClientConnection.java
+ test/sun/management/jdp/DynamicLauncher.java
! test/sun/management/jdp/JdpClient.java
+ test/sun/management/jdp/JdpDefaultsTest.java
! test/sun/management/jdp/JdpDoSomething.java
+ test/sun/management/jdp/JdpOffTest.java
+ test/sun/management/jdp/JdpOffTestCase.java
+ test/sun/management/jdp/JdpOnTestCase.java
+ test/sun/management/jdp/JdpSpecificAddressTest.java
! test/sun/management/jdp/JdpTest.sh
+ test/sun/management/jdp/JdpTestCase.java
+ test/sun/management/jdp/JdpTestUtil.java
+ test/sun/management/jdp/JdpTestUtilTest.java
! test/sun/management/jdp/JdpUnitTest.java
+ test/sun/management/jdp/PacketTest.java
+ test/sun/management/jdp/PortAlreadyInUseTest.java
+ test/sun/management/jdp/README



hg: jdk8/tl/jdk: 8027752: sun/tools/jstatd/TestJstatdExternalRegistry.java: java.lang.SecurityException: attempt to add a Permission to a readonly Permissions object

2013-11-08 Thread staffan . larsen
Changeset: 8a4405fb40ba
Author:ykantser
Date:  2013-11-07 16:55 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8a4405fb40ba

8027752: sun/tools/jstatd/TestJstatdExternalRegistry.java: 
java.lang.SecurityException: attempt to add a Permission to a readonly 
Permissions object
Reviewed-by: sla, jbachorik

! test/sun/tools/jstatd/JstatdTest.java



hg: jdk8/tl/jdk: 8027692: Remove java/lang/management/MemoryMXBean/LowMemoryTest2.sh from ProblemList.txt

2013-11-01 Thread staffan . larsen
Changeset: c35f6df5bce9
Author:sla
Date:  2013-11-01 10:08 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c35f6df5bce9

8027692: Remove java/lang/management/MemoryMXBean/LowMemoryTest2.sh from 
ProblemList.txt
Reviewed-by: stefank, alanb

! test/ProblemList.txt



hg: jdk8/tl/jdk: 8027705: com/sun/jdi/JdbMethodExitTest.sh fails when a background thread is generating events.

2013-11-01 Thread staffan . larsen
Changeset: c59ccad6eb72
Author:sla
Date:  2013-11-01 15:10 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c59ccad6eb72

8027705: com/sun/jdi/JdbMethodExitTest.sh fails when a background thread is 
generating events.
Reviewed-by: dcubed

! test/com/sun/jdi/JdbMethodExitTest.sh
! test/com/sun/jdi/ShellScaffold.sh



hg: jdk8/tl/jdk: 8027371: Add JDI tests for breakpointing and stepping in lambda code

2013-10-29 Thread staffan . larsen
Changeset: ecba02f6be31
Author:sla
Date:  2013-10-29 08:10 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ecba02f6be31

8027371: Add JDI tests for breakpointing and stepping in lambda code
Reviewed-by: mchung, sspitsyn

+ test/com/sun/jdi/LambdaBreakpointTest.java
+ test/com/sun/jdi/LambdaStepTest.java



hg: jdk8/tl/jdk: 8009681: TEST_BUG: MethodExitReturnValuesTest.java may fail when there are unexpected background threads

2013-10-24 Thread staffan . larsen
Changeset: e6bc0dca294b
Author:sla
Date:  2013-10-15 12:53 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e6bc0dca294b

8009681: TEST_BUG: MethodExitReturnValuesTest.java may fail when there are 
unexpected background threads
Reviewed-by: sla, allwin
Contributed-by: mikael.a...@oracle.com

! test/com/sun/jdi/MethodEntryExitEvents.java
! test/com/sun/jdi/MethodExitReturnValuesTest.java



hg: jdk8/tl/jdk: 8026789: Update test/java/lang/instrument/Re(transform|define)BigClass.sh test to use NMT for memory leak detection

2013-10-23 Thread staffan . larsen
Changeset: 8c20e9ef8709
Author:sla
Date:  2013-10-23 15:55 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8c20e9ef8709

8026789: Update test/java/lang/instrument/Re(transform|define)BigClass.sh test 
to use NMT for memory leak detection
Reviewed-by: dcubed

! test/ProblemList.txt
+ test/java/lang/instrument/NMTHelper.java
! test/java/lang/instrument/RedefineBigClass.sh
! test/java/lang/instrument/RedefineBigClassApp.java
! test/java/lang/instrument/RetransformBigClass.sh
! test/java/lang/instrument/RetransformBigClassApp.java



hg: jdk8/tl/jdk: 8026962: Put java/lang/management/ClassLoadingMXBean/LoadCounts.java into ProblemList.txt

2013-10-22 Thread staffan . larsen
Changeset: bb2fb6be8b2a
Author:ykantser
Date:  2013-10-22 10:57 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bb2fb6be8b2a

8026962: Put java/lang/management/ClassLoadingMXBean/LoadCounts.java into 
ProblemList.txt
Reviewed-by: sla, jbachorik

! test/ProblemList.txt



Re: RFR: 8009681: TEST_BUG: MethodExitReturnValuesTest.java fails with when there are unexpected background threads

2013-10-18 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 16 okt 2013, at 14:04, Mikael Auno mikael.a...@oracle.com wrote:

 This bug got a bit lost from my radar after vacation, but I've picked it
 again now. I've moved Arrays.asList() as suggested. In further testing of the 
 fix though, I found that the include list is not enough, as one of the 
 expected method exit events is from String.intern(), which might also be 
 called from background threads. To counter this, I added a thread filter to 
 the events. This also has the added benefit of speeding up the test 
 significantly (from 90 seconds to about 5 seconds) in the cases where there 
 are background threads interfering.
 
 Also added to this webrev is a fix for MethodEntryExitEvents.java which had 
 exactly the same problem and a similar test design as 
 MethodExitReturnValuesTest.java.
 
 The updated webrev is at 
 http://cr.openjdk.java.net/~allwin/auno/8009681/webrev.00/.
 
 Thanks,
 Mikael
 
 On 2013-05-28 08:46, Staffan Larsen wrote:
 Looks good.
 
 You could optimize it a bit by not doing the Arrays.asList() on every
 methodExit event.
 
 /Staffan
 
 On 17 apr 2013, at 15:03, Mikael Auno mikael.a...@oracle.com
 wrote:
 
 Hi, I'd like some reviews on
 http://cr.openjdk.java.net/~nloodin/8009681/webrev.01/ for
 JDK-8009681 (http://bugs.sun.com/view_bug.do?bug_id=8009681).
 
 The issue here is that when MethodExitReturnValuesTest hooks into
 MethodExit events through JDI it uses an exclude list to filter out
 classes from which it is not interested in these events. This is
 bound to break over and over again as new features are added to the
 JDK. I've changed the test to use an include list instead,
 containing only the handful of classes the test is actually
 interested in.
 
 Thanks,
  Mikael
 
 



hg: jdk8/tl/jdk: 8021897: EXCEPTION_ACCESS_VIOLATION on debugging String.contentEquals()

2013-10-18 Thread staffan . larsen
Changeset: 8479a48d9fd4
Author:sla
Date:  2013-10-18 11:52 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8479a48d9fd4

8021897: EXCEPTION_ACCESS_VIOLATION on debugging String.contentEquals()
Reviewed-by: alanb, sspitsyn

! src/share/back/outStream.c
+ test/com/sun/jdi/GetUninitializedStringValue.java



hg: jdk8/tl/jdk: 8025427: jstat tests fails on 32-bit platforms

2013-10-10 Thread staffan . larsen
Changeset: 6aa637dde16e
Author:sla
Date:  2013-10-10 09:38 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6aa637dde16e

8025427: jstat tests fails on 32-bit platforms
Reviewed-by: ehelin, dsamersoff, dholmes, sspitsyn

! src/share/classes/sun/tools/jstat/RowClosure.java
! test/ProblemList.txt
! test/sun/tools/jstat/gcCauseOutput1.awk
! test/sun/tools/jstat/lineCounts1.awk
! test/sun/tools/jstat/lineCounts2.awk
! test/sun/tools/jstat/lineCounts3.awk
! test/sun/tools/jstat/lineCounts4.awk
! test/sun/tools/jstat/timeStamp1.awk
! test/sun/tools/jstatd/jstatGcutilOutput1.awk



hg: jdk8/tl/jdk: 8014446: JT_JDK: Wrong detection of test result for test com/sun/jdi/NoLaunchOptionTest.java

2013-10-10 Thread staffan . larsen
Changeset: 998560cccefc
Author:allwin
Date:  2013-10-10 10:14 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/998560cccefc

8014446: JT_JDK: Wrong detection of test result for test 
com/sun/jdi/NoLaunchOptionTest.java
Reviewed-by: sla, mgronlun, dholmes, jbachorik, chegar

! test/com/sun/jdi/NoLaunchOptionTest.java



Re: RFR (S) 8025238: nsk/jvmti/scenarios/bcinstr/BI04/bi04t002 crashed with SIGSEGV

2013-10-04 Thread Staffan Larsen

On 4 okt 2013, at 00:49, Coleen Phillmore coleen.phillim...@oracle.com wrote:

 
 Thanks Dan -
 
 On 10/3/2013 4:28 PM, Daniel D. Daugherty wrote:
  open webrev at http://cr.openjdk.java.net/~coleenp/8025238_jdk
 
 test/java/lang/instrument/RedefineMethodInBacktrace.sh
No comments.
 
 test/java/lang/instrument/RedefineMethodInBacktraceApp.java
line 78: t.setDaemon(true);
Why make the target thread a daemon? Wouldn't it be a more
complete test to do Thread.join() of that thread just to
be sure that the target thread has finished (and is not
stuck)?
 
 Staffan, can you answer this?

You could do it either way. I don't have a strong opinion. It's possible that 
the deamonization is a leftover from one of my iterations of the test.

/Staffan


 
 Coleen
 
 
 test/java/lang/instrument/RedefineMethodInBacktraceTargetB.java
 test/java/lang/instrument/RedefineMethodInBacktraceTargetB_2.java
Nice sync logic with the test driver.
 
 Thumbs up.
 
 Dan
 
 
 On 10/3/13 12:02 PM, Coleen Phillimore wrote:
 Summary: Redefined class in stack trace may not be found by method_idnum so 
 handle null.
 
 This is a simple change.  I had another change to save the method name (as 
 u2) in the backtrace, but it's not worth the extra footprint in backtraces 
 for this rare case.
 
 The root problem was that we save method_idnum in the backtrace (u2) 
 instead of Method* to avoid Method* from being redefined and deallocated.  
 I made a change to InstanceKlass::method_from_idnum() to return null rather 
 than the last method in the list, which causes this crash.   Dan and I went 
 down the long rabbit-hole of why method_idnum is changed for obsolete 
 methods and we think there's some cleanup and potential bugs in this area.  
 But this is not that change.  I'll file another bug to continue this 
 investigation for jdk9 (or 8uN).
 
 Staffan created a test - am including core-libs for the review request.  
 Also tested with all of the vm testbase tests, mlvm tests, and 
 java/lang/instrument tests.
 
 open webrev at http://cr.openjdk.java.net/~coleenp/8025238/
 bug link https://bugs.openjdk.java.net/browse/JDK-8025238
 
 test case for jdk8 repository:
 
 open webrev at http://cr.openjdk.java.net/~coleenp/8025238_jdk
 
 Thanks,
 Coleen
 
 
 
 



hg: jdk8/tl/jdk: 8025829: Add java/lang/instrument/RetransformBigClass.sh to problemlist

2013-10-04 Thread staffan . larsen
Changeset: 77ba1e67707c
Author:allwin
Date:  2013-10-04 15:00 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/77ba1e67707c

8025829: Add java/lang/instrument/RetransformBigClass.sh to problemlist
Reviewed-by: sla, jbachorik

! test/ProblemList.txt



hg: jdk8/tl/jdk: 6696975: JTop plugin fails if connected readonly to target JVM

2013-10-02 Thread staffan . larsen
Changeset: 3bb89c509d59
Author:egahlin
Date:  2013-10-01 17:48 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3bb89c509d59

6696975: JTop plugin fails if connected readonly to target JVM
Reviewed-by: mchung, jbachorik, sla, sjiang

! src/share/demo/management/JTop/JTop.java



hg: jdk8/tl/jdk: 8023492: jfr.jar gets loaded even though it's not used

2013-09-30 Thread staffan . larsen
Changeset: cceaad499685
Author:sla
Date:  2013-09-30 12:58 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cceaad499685

8023492: jfr.jar gets loaded even though it's not used
Reviewed-by: erikj, mgronlun

! make/tools/src/build/tools/buildmetaindex/BuildMetaIndex.java



Re: RFR(L): 8024854: Basic changes and files to build the class library on AIX

2013-09-20 Thread Staffan Larsen
Volker,

The serviceability-related changes look ok to me (not a Reviewer).

/Staffan

On 16 sep 2013, at 21:30, Volker Simonis volker.simo...@gmail.com wrote:

 Resending this to more lists as requested by Alan Bateman with the kind 
 request to anybody to review the parts for which he feels responsible:)
 
 For those not up to date, this change is part of the ongoing PowerPC/AIX 
 Porting Project:
 http://openjdk.java.net/projects/ppc-aix-port
 https://wiki.openjdk.java.net/display/PPCAIXPort
 http://openjdk.java.net/jeps/175
 
 Please send reviews to all currently included recipients.
 
 Thank you and best regards,
 Volker
 
 
 -- Forwarded message --
 From: Volker Simonis 
 Date: Monday, September 16, 2013
 Subject: RFR(L): 8024854: Basic changes and files to build the class library 
 on AIX
 To: ppc-aix-port-...@openjdk.java.net ppc-aix-port-...@openjdk.java.net, 
 Java Core Libs core-libs-dev@openjdk.java.net
 
 
 Hi,
 
 could you please review the following webrev which contains the basic changes 
 and files needed in the 'jdk' repository in order to build the OpenJDK on AIX:
 
 http://cr.openjdk.java.net/~simonis/webrevs/8024854
 This change together with 8024265: Enable new build on AIX (jdk part) 
 allows it to configure and completely build the staging repository on AIX 5.3 
 and 7.1 with the following command:
 
 configure --with-boot-jdk=jdk-image --with-jvm-variants=core 
 --with-jvm-interpreter=cpp --with-cups-include=/opt/freeware/include 
 --x-includes=/opt/freeware/include
 
 Below you can find the changes and additions I've done, sorted by file. Most 
 of them are just additions which are only active during the AIX build anyway 
 or simple changes where AIX has been added to conditions which already check 
 for Linux and/or Solaris. The files with the biggest changes which you're 
 probably want to look on more thoroughly are 
 'src/solaris/native/java/net/NetworkInterface.c' and 
 'src/solaris/native/sun/nio/ch/Net.c' altough they shouldn't change anything 
 on the current OpenJDK platforms as well.
 
 Notice that there are still some files and some functionality missing from 
 the current change (notably NIO) but it still yields a running JDK which can 
 execute HelloWorld on the command line and as AWT application. I've 
 intentionally tried to keep this initial change as simple as possible (with 
 respect tot shared changes) in order to get it reviewed as fast as possible. 
 The missing parts can then be added later on, grouped by logical topics, to 
 simplify the review process.
 Thank you and best regards,
 
 Volker
 
 src/share/bin/jli_util.h
 
 Define JLI_Lseek on AIX.
 src/share/lib/security/java.security-aix
 
 Provide default java.security-aix for AIX.
 src/share/native/sun/awt/medialib/mlib_sys.c
 
 malloc always returns 8-byte aligned pointers on AIX as well.
 src/share/native/sun/awt/medialib/mlib_types.h
 
 Add AIX to the list of known platforms.
 src/share/native/sun/font/layout/KernTable.cpp
 
 Rename the macro DEBUG to DEBUG_KERN_TABLE because DEBUG is too common and 
 may be defined in other headers as well as on the command line and xlc bails 
 out on macro redefinitions with a different value.
 src/share/native/sun/security/ec/impl/ecc_impl.h
 
 Define required types and macros on AIX.
 src/solaris/back/exec_md.c
 
 AIX behaves like Linux in this case so check for it in the Linux branch.
 src/solaris/bin/java_md_solinux.c,
 src/solaris/bin/java_md_solinux.h
 
 On AIX LD_LIBRARY_PATH is called LIBPATH
 Always use LD_LIBRARY_PATH macro instead of using the string 
 LD_LIBRARY_PATH directly to cope with different library path names.
 Add jre/lib/arch/jli to the standard library search path on AIX because the 
 AIX linker doesn't support the -rpath option.
 Replace #ifdef __linux__ by #ifndef __solaris__ because in this case, AIX 
 behaves like Linux.
 src/solaris/classes/sun/awt/fontconfigs/aix.fontconfig.properties
 
 Provide basic fontconfig.propertiesfor AIX.
 src/solaris/classes/java/lang/UNIXProcess.java.aix,
 src/solaris/classes/sun/tools/attach/AixAttachProvider.java,
 src/solaris/classes/sun/tools/attach/AixVirtualMachine.java
 
 Provide AIX specific Java versions, mostly based on the corresponding Linux 
 versions.
 src/solaris/demo/jvmti/hprof/hprof_md.c
 
 Add AIX support. AIX mostly behaves like Linux, with the one exception that 
 it has no dladdr functionality.
 Implement dladdr functionality for AIX.
 src/solaris/native/com/sun/management/UnixOperatingSystem_md.c
 
 Adapt for AIX (i.e. use libperfstat on AIX to query OS memory).
 src/solaris/native/common/jdk_util_md.h
 
 Add AIX definitions of the ISNANF and ISNAND macros.
 src/solaris/native/java/io/io_util_md.c
 
 AIX behaves like Linux in this case so check for it in the Linux branch.
 src/solaris/native/java/net/NetworkInterface.c
 
 Add AIX support into the Linux branch because AIX mostly behaves like AIX for 
 IPv4.
 AIX needs a special function to enumerate IPv6 interfaces and to 

RFR(S): 7200277 [parfait] potential buffer overflow in npt/utf.c

2013-09-20 Thread Staffan Larsen
Please review this change to avoid a buffer overflow in npt/utf.c.

webrev: http://cr.openjdk.java.net/~sla/7200277/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-7200277

Thanks,
/Staffan


hg: jdk8/tl/jdk: 7200277: [parfait] potential buffer overflow in npt/utf.c

2013-09-20 Thread staffan . larsen
Changeset: 94cc251d0c45
Author:sla
Date:  2013-09-20 16:40 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/94cc251d0c45

7200277: [parfait] potential buffer overflow in npt/utf.c
Reviewed-by: dsamersoff, dcubed

! src/share/npt/utf.c



Re: RFR: 8014659: NPG: performance counters for compressed klass space

2013-09-12 Thread Staffan Larsen
Looks good (I'll take your word that the nasty awk scripts are correct...).

I think you should have included serviceability-dev on this review request.

/Staffan


On 10 sep 2013, at 18:56, Erik Helin erik.he...@oracle.com wrote:

 Hi all,
 
 this is the JDK part of the fix for 8014659 [0]. I've added the output
 of the compressed class space performance counters next to the metaspace
 output. I've also updated the jstat and jstatd tests to take the new
 output into account.
 
 Webrev:
 http://cr.openjdk.java.net/~ehelin/8014659/webrev.00/
 
 Testing:
 - jdk/test/sun/tools/jstat
 - jdk/test/sun/tools/jstatd
 
 Thanks,
 Erik
 
 [0]: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014659



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

2013-09-10 Thread Staffan Larsen
In SunCommandLineLauncher.java:

 198 if (home.length()  0) {
 199 String os_arch = System.getProperty(os.arch);
 200 if (SunOS.equals(System.getProperty(os.name))) {
 201 exePath = home + File.separator + bin + 
File.separator + exe;
 202 }
 203 } else {
 204 exePath = 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,
/Staffan


On 9 sep 2013, at 18:12, Kumar Srinivasan kumar.x.sriniva...@oracle.com wrote:

 Hi David,
 
 Hi Kumar,
 
 This is still dead code in 
 src/share/classes/com/sun/tools/jdi/SunCommandLineLauncher.java
 
 String os_arch = System.getProperty(os.arch);
 
 Ah, I will take care of it. Thanks for spotting this.
 
 Also:
 
 test/java/nio/channels/spi/SelectorProvider/inheritedChannel/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 David, Staffan, Alan,
 
 I have addressed all the issues pointed and some more I found while jprt
 testing.
 
 The updated webrev for jdk is here:
 http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.1/
 
 and the delta webrev since the last review webrev is here:
 http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.1/webrev.delta/index.html
  
 
 
 Thanks
 Kumar
 
 
 Hi Kumar,
 
 A few minor comments ...
 
 src/share/classes/com/sun/tools/jdi/SunCommandLineLauncher.java
 
 Seems to me this is all dead now:
 
 199 /*
 200  * A wrinkle in the environment:
 201  * 64-bit executables are stored under
 $JAVA_HOME/bin/os_arch
 202  * 32-bit executables are stored under
 $JAVA_HOME/bin
 203  */
 204 String os_arch = System.getProperty(os.arch);
 
 os_arch is no longer used and the comment no longer applicable.
 
 ---
 
 src/solaris/bin/java_md_solinux.c
 
 This seems to force DUAL_MODE off regardless of what the user may set
 it to:
 
  #ifdef __solaris__
 ! #  ifdef DUAL_MODE
 ! #undef DUAL_MODE
 ! #  endif
 
 why doesn't it just not define DUAL_MODE?
 
 ---
 
 test/demo/jvmti/DemoRun.java
 test/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-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.openjdk.java.net/~ksrini/8020552/webrev.jdk.0/
 
 The top forest changes are here:
 http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk8.0/
 
 
 Thanks
 Kumar
 
 
 
 



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

2013-09-07 Thread Staffan Larsen
This is a welcome change. I've looked at the serviceability test and the 
changes look good except:

test/demo/jvmti/DemoRun.java
test/sun/tools/jhat/HatRun.java
  - Looks like there are still some -d64 remnants that I wasn't expecting.

Thanks,
/Staffan

On 6 sep 2013, at 22:17, Kumar Srinivasan kumar.x.sriniva...@oracle.com wrote:

 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://mail.openjdk.java.net/pipermail/jdk8-dev/2013-September/003159.html 
 
 The jdk changes are here:
 http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk.0/
 
 The top forest changes are here:
 http://cr.openjdk.java.net/~ksrini/8020552/webrev.jdk8.0/
 I haven't studied the changes yet but I see you've updated 
 test/java/nio/channels/spi/SelectorProvider/inheritedChannel/run_tests.sh. I 
 don't think you need the changes at L42-48, instead you can just hg rm the 
 32-bit libraries that are in 
 test/java/nio/channels/spi/SelectorProvider/inheritedChannel/lib.
 
 Will do, I was wondering about those libraries.
 
 
 You might want to bring the changes to serviceability-dev because of the 
 change to the JDI launching connector and the JDI tests.
 
 cc'ed.
 
 Thanks
 
 Kumar
 
 
 -Alan.
 
 
 



hg: jdk8/tl/jdk: 7172176: java/jconsole test/sun/tools/jconsole/ImmutableResourceTest.sh failing

2013-09-02 Thread staffan . larsen
Changeset: a7d463f5a5b9
Author:egahlin
Date:  2013-09-02 16:03 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a7d463f5a5b9

7172176: java/jconsole test/sun/tools/jconsole/ImmutableResourceTest.sh failing
Reviewed-by: mchung, mfang

! src/share/classes/sun/tools/jconsole/Resources.java
! test/ProblemList.txt
- test/sun/tools/jconsole/ImmutableResourceTest.java
- test/sun/tools/jconsole/ImmutableResourceTest.sh
! test/sun/tools/jconsole/ResourceCheckTest.java
! test/sun/tools/jconsole/ResourceCheckTest.sh



hg: jdk8/tl/jdk: 8023786: (jdk) setjmp/longjmp changes the process signal mask on OS X

2013-08-29 Thread staffan . larsen
Changeset: 779ff9f3b2e3
Author:sla
Date:  2013-08-29 11:22 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/779ff9f3b2e3

8023786: (jdk) setjmp/longjmp changes the process signal mask on OS X
Reviewed-by: dholmes

! src/share/back/SDE.c
! src/share/native/common/check_code.c



  1   2   >