Re: Need Second reviewer! Re: RR(S): JDK-8023667 SA: ExceptionBlob and other C2 classes not available in client VM

2014-02-05 Thread David Holmes
Hi Dmitry, This looks okay to me. But can I suggest not starting a new email thread when you need to prompt for reviews as it makes this review disconnected from the original RFR email. (I would have replied to the original but I don't have a local copy to do that.) Thanks, David On 5/02/2

Re: code review round 0 for minor FDS makefile fix (8033714)

2014-02-05 Thread David Holmes
Hi Dan, Looks good to me. (I never run the install targets :( ) Thanks, David On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote: This code review request is going to three different aliases. Don't use Thunderbird's "reply to list" option since it will pick just _one_ of the _three_ lists. Gree

Re: code review round 0 for minor FDS makefile fix (8033714)

2014-02-05 Thread Daniel D. Daugherty
Ron, Thanks for the review. Dan On 2/5/14 7:04 PM, Ron Durbin wrote: Dan The changes look good Ron -Original Message- From: Daniel D. Daugherty Sent: Wednesday, February 05, 2014 4:21 PM To: hotspot-runtime-...@openjdk.java.net; serviceability-dev@openjdk.java.net; build-dev; Doug

RE: code review round 0 for minor FDS makefile fix (8033714)

2014-02-05 Thread Ron Durbin
Dan The changes look good Ron > -Original Message- > From: Daniel D. Daugherty > Sent: Wednesday, February 05, 2014 4:21 PM > To: hotspot-runtime-...@openjdk.java.net; > serviceability-dev@openjdk.java.net; build-dev; > Doug Simon; Tom Rodriguez > Subject: code review round 0 for minor

code review round 0 for minor FDS makefile fix (8033714)

2014-02-05 Thread Daniel D. Daugherty
This code review request is going to three different aliases. Don't use Thunderbird's "reply to list" option since it will pick just _one_ of the _three_ lists. Greetings, Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS) makefile fix our way. Here are the bug and webrev URLs:

Re: Why do we need both - export maps AND -fvisibility=hidden/__attribute__((visibility("default")))

2014-02-05 Thread Volker Simonis
On Wed, Feb 5, 2014 at 8:16 AM, David Holmes wrote: > On 5/02/2014 5:09 PM, Jeremy Manson wrote: >> >> Also, don't you statically link libstdc++ into Hotspot? > > > Normally yes. > That's bad - security-wise as well as from a usability perspective: Security-wise because the JDK will need to be r

Need Second reviewer! Re: RR(S): JDK-8023667 SA: ExceptionBlob and other C2 classes not available in client VM

2014-02-05 Thread Dmitry Samersoff
On 2014-02-04 17:46, Dmitry Samersoff wrote: > Staffan, > > You was right, only two classes is C2 specific. > > Here is updated webrev. > > http://cr.openjdk.java.net/~dsamersoff/JDK-8023667/webrev.02/ > > -Dmitry > > On 2014-02-03 16:13, Staffan Larsen wrote: >> >> On 3 feb 2014, at 11:59, Dm

Re: Review request for 7195249: Some jtreg tests use hard coded ports

2014-02-05 Thread Jaroslav Bachorik
Hi Taras, thanks for taking care of this. The changes look fine to me. One minor nit is unused imports of the library classes in "test/sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java". It does not use any of those classes as its base class "AbstractFilePermissionTest" doe

Re: Review request for 7195249: Some jtreg tests use hard coded ports

2014-02-05 Thread taras ledkov
Hi, So please take a look at the review against JDK9. The reviewed patch had not been integrated into JDK8. Port to JDK9 is identical. The difference: the ProcessTools.java has been already patched by Jaroslav. Webrev for jdk part: http://cr.openjdk.java.net/~anazarov/7195249/jdk/webrev.03/

Re: JDK-7090324: gclog rotation via external tool

2014-02-05 Thread Yasumasa Suenaga
Sorry, I forgot to paste URL of new webrev :-P http://cr.openjdk.java.net/~ysuenaga/JDK-7090324/webrev.04/ Yasumasa On 02/05/2014 09:09 PM, Yasumasa Suenaga wrote: Hi Erik, Thank you for reviewing again! I've updated new webrev. On 02/05/2014 07:40 PM, Erik Helin wrote: Hi Yasumasa, I've l

Re: JDK-7090324: gclog rotation via external tool

2014-02-05 Thread Yasumasa Suenaga
Hi Erik, Thank you for reviewing again! I've updated new webrev. On 02/05/2014 07:40 PM, Erik Helin wrote: Hi Yasumasa, I've looked through the latest patch, it is much better! I just have two comments: - ostream.hpp: Why did you add GCLogFileSize != 0 in should_rotate? The old check ju

Re: JDK-7090324: gclog rotation via external tool

2014-02-05 Thread Erik Helin
Hi Yasumasa, I've looked through the latest patch, it is much better! I just have two comments: - ostream.hpp: Why did you add GCLogFileSize != 0 in should_rotate? The old check just checked that _bytes_written > GCLogFileSize. - TestGCLogRotationViaJcmd.java: Could you use the helper c

Re: RFR(S): JDK-8029808 com/sun/jdi/ProcessAttachTest.sh times out

2014-02-05 Thread Staffan Larsen
Dmitry, Alan: Thanks! On 3 feb 2014, at 21:31, Dmitry Samersoff wrote: > Staffan, > > Looks good for me! > > -Dmitry > > On 2014-02-03 23:18, Staffan Larsen wrote: >> OK, my last try at this was very broken - I’m glad it didn’t pass the review. >> >> Here is a new try: http://cr.openjdk.java