Re: RFR(XXS): 8145212: ISO-8859-1 isn't properly handled as 'fastEncoding' in jni_util.c

2015-12-15 Thread Alan Bateman
On 15/12/2015 07:55, Volker Simonis wrote: Forwarded to core-libs-dev upon request. As I already got two reviews (thanks Roger and Martin) and as the fix for "8145015: jni_GetStringCritical asserts for empty strings" [1] has just been pushed to jdk9/hs-rt/hotspot I plan to push this one to jdk

Re: RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

2015-12-15 Thread Konstantin Shefov
Hi Christian Thanks for reviewing, I have changed indents as you asked: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.03 -Konstantin On 12/15/2015 06:23 AM, Christian Thalinger wrote: On Dec 11, 2015, at 1:54 AM, Konstantin Shefov mailto:konstantin.she...@oracle.com>> wrote: Hel

Re: Need help to understand TLS behavior

2015-12-15 Thread Thomas Stüfe
Hi Jeremy, David, I would like to understand the problem better and have some questions, maybe you could answer? - What is the difference between "static __thread int x" and "__thread int x" - one lives in the thread stacks, one does not? - What happens with existing threads if a library is loade

Re: Need help to understand TLS behavior

2015-12-15 Thread David Holmes
On 15/12/2015 7:25 PM, Thomas Stüfe wrote: Hi Jeremy, David, I would like to understand the problem better and have some questions, maybe you could answer? The "specification" for ELF based TLS as I understood it is the Ulrich Drepper document I referenced: ""ELF Handling for Thread Local Sto

RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Chris Hegarty
No new code here, just giving sun.misc.ProxyGenerator a more appropriate location, in jdk.internal.reflect ( since it contains the code to generate a dynamic proxy class for the java.lang.reflect.Proxy ). http://cr.openjdk.java.net/~chegar/proxyGeneratorWebrev/webrev/ Note: ProxyGenerator could

Re: RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Roger Riggs
Hi Chris, ok as is. But is that the only class in the jdk.internal.reflect package (or will it always be)? Roger On 12/15/2015 7:00 AM, Chris Hegarty wrote: No new code here, just giving sun.misc.ProxyGenerator a more appropriate location, in jdk.internal.reflect ( since it contains the co

Re: RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Alan Bateman
On 15/12/2015 12:00, Chris Hegarty wrote: No new code here, just giving sun.misc.ProxyGenerator a more appropriate location, in jdk.internal.reflect ( since it contains the code to generate a dynamic proxy class for the java.lang.reflect.Proxy ). http://cr.openjdk.java.net/~chegar/proxyGenera

Re: RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Chris Hegarty
On 15 Dec 2015, at 14:44, Roger Riggs wrote: > Hi Chris, > > ok as is. Thanks for looking at this Roger. > But is that the only class in the jdk.internal.reflect package (or will it > always be)? Nope. Several other classes from sun.reflect will likely wind up there too. -Chris. > Roger >

General question: sun package -> jdk package?

2015-12-15 Thread Paul Benedict
I have a general question prompted by the many classes moved from sun.* to jdk.*. Once JDK 9 delivers on the Module System and internals are no longer exposed, is it planned to eventually migrate away from the sun.* legacy namespace in later JDK versions? Cheers, Paul

Re: General question: sun package -> jdk package?

2015-12-15 Thread Chris Hegarty
Paul, I cannot address your general question, but I guess it might be motivated by some of my recent preparatory work for JEP 260 [1]. This JEP proposes to expose a small number of critical API’s that are in the sun.misc and sun.reflect namespace. Anything not deemed critical should be removed fro

Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-12-15 Thread Paul Benedict
David, this needs editing. * The cleaning function is invoked after the object it is cleaning up after it * becomes phantom reachable, so it is important that the references and values * it needs do not prevent the object from becoming phantom reachable. 1) The "after it" looks like a leftover fr

Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-12-15 Thread Roger Riggs
Hi David, Thanks for the comments and suggestions from recent emails, they have been applied to the webrev and javadoc. [1]http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/ [2]http://cr.openjdk.java.net/~rriggs/cleaner-doc/index.html Roger On 12/15/2015 1:59 AM, David Holmes wrote:

RFR: JDK-8115868: 32-bit JVM failed to start from a large network filesystem

2015-12-15 Thread Kumar Srinivasan
Hello, Please review fix for: JDK-8115868 The webrev is here: http://cr.openjdk.java.net/~ksrini/8115868/webrev.0/ The background: The launcher uses stat(2) to check for the existence of a file, unfortunately on 32-bit system with large file systems causes the inode storage to overflow causi

Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-12-15 Thread Paul Benedict
No, you didn't miss any. I didn't know you were using line breaks for readability or meant a new paragraph. No worries. You are right they don't break the paragraph in javadoc. Cheers, Paul On Tue, Dec 15, 2015 at 10:00 AM, Roger Riggs wrote: > Hi Paul, > > The credit/blame for the Cleaner doc

Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-12-15 Thread Roger Riggs
Hi Paul, The credit/blame for the Cleaner doc is mine. On 12/15/2015 10:25 AM, Paul Benedict wrote: David, this needs editing. * The cleaning function is invoked after the object it is cleaning up after it * becomes phantom reachable, so it is important that the references and values * it n

Re: RFR 4823133: RandomAccessFile.length() is not thread-safe

2015-12-15 Thread Roger Riggs
Hi Yvom, Minor comments: src/java.base/share/native/libjava/RandomAccessFile.c: - "length fail" might be clearer as "GetLength failed" src/java.base/unix/native/libjava/io_util_md.c: - Please add a comment before the define of FILE_OFFSET_BITS to indicate where it is used and why it is ther

Re: RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Mandy Chung
> On Dec 15, 2015, at 4:00 AM, Chris Hegarty wrote: > > No new code here, just giving sun.misc.ProxyGenerator a more appropriate > location, in jdk.internal.reflect ( since it contains the code to generate a > dynamic > proxy class for the java.lang.reflect.Proxy ). > > http://cr.openjdk.java.

Re: RFR 4823133: RandomAccessFile.length() is not thread-safe

2015-12-15 Thread Martin Buchholz
_FILE_OFFSET_BITS is generally an all-or-nothing thing, because it affects interoperability between translation units. It would be good to convert all of the JDK build to use -D_FILE_OFFSET_BITS=64, but that would be a big job. So traditionally the JDK has instead used the functions made availabl

Re: RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

2015-12-15 Thread Christian Thalinger
> On Dec 14, 2015, at 11:11 PM, Konstantin Shefov > wrote: > > Hi Christian > > Thanks for reviewing, I have changed indents as you asked: > > http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.03 > Thanks. I’m still not comfo

Review Request for JDK-8145430: Fix typo in StackWalker javadoc

2015-12-15 Thread Mandy Chung
diff --git a/src/java.base/share/classes/java/lang/StackWalker.java b/src/java.base/share/classes/java/lang/StackWalker.java --- a/src/java.base/share/classes/java/lang/StackWalker.java +++ b/src/java.base/share/classes/java/lang/StackWalker.java @@ -304,8 +304,8 @@ } /** - * Retur

Re: Review Request for JDK-8145430: Fix typo in StackWalker javadoc

2015-12-15 Thread Daniel Fuchs
On 15/12/15 19:18, Mandy Chung wrote: diff --git a/src/java.base/share/classes/java/lang/StackWalker.java b/src/java.base/share/classes/java/lang/StackWalker.java --- a/src/java.base/share/classes/java/lang/StackWalker.java +++ b/src/java.base/share/classes/java/lang/StackWalker.java @@ -304,8 +

Re: Review Request for JDK-8145430: Fix typo in StackWalker javadoc

2015-12-15 Thread joe darcy
+1 On 12/15/2015 10:29 AM, Daniel Fuchs wrote: On 15/12/15 19:18, Mandy Chung wrote: diff --git a/src/java.base/share/classes/java/lang/StackWalker.java b/src/java.base/share/classes/java/lang/StackWalker.java --- a/src/java.base/share/classes/java/lang/StackWalker.java +++ b/src/java.base/sha

Re: RFR: JDK-8115868: 32-bit JVM failed to start from a large network filesystem

2015-12-15 Thread Henry Jen
Changes looks reasonable to me. Cheers, Henry > On Dec 15, 2015, at 7:54 AM, Kumar Srinivasan > wrote: > > Hello, > > Please review fix for: JDK-8115868 > > The webrev is here: > http://cr.openjdk.java.net/~ksrini/8115868/webrev.0/ > > The background: > The launcher uses stat(2) to check f

Re: RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Chris Hegarty
On 15 Dec 2015, at 17:15, Mandy Chung wrote: > >> On Dec 15, 2015, at 4:00 AM, Chris Hegarty wrote: >> >> No new code here, just giving sun.misc.ProxyGenerator a more appropriate >> location, in jdk.internal.reflect ( since it contains the code to generate a >> dynamic >> proxy class for the

Re: RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Roger Riggs
+1 On 12/15/2015 1:41 PM, Chris Hegarty wrote: On 15 Dec 2015, at 17:15, Mandy Chung wrote: On Dec 15, 2015, at 4:00 AM, Chris Hegarty wrote: No new code here, just giving sun.misc.ProxyGenerator a more appropriate location, in jdk.internal.reflect ( since it contains the code to generate a

Re: General question: sun package -> jdk package?

2015-12-15 Thread Joel Borggrén-Franck
Hi Chris, I'm somewhat surprised to see ReflectionFactory on that list. Can you share more details around its external use? cheers /Joel On Tue, 15 Dec 2015 at 16:15 Chris Hegarty wrote: > Paul, > > I cannot address your general question, but I guess it might be motivated > by some of my recen

Re: RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Mandy Chung
> On Dec 15, 2015, at 10:41 AM, Chris Hegarty wrote: > > > Webrev updated in-place. Thank you for moving it to java.lang.reflect. Formatting nit: since you make generateProxyClass method package-private, line 323 & 335-336 needs to be adjusted to align with the line above. Thanks for renami

Re: RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Chris Hegarty
On 15 Dec 2015, at 19:26, Mandy Chung wrote: >> On Dec 15, 2015, at 10:41 AM, Chris Hegarty wrote: >> >> >> Webrev updated in-place. > > Thank you for moving it to java.lang.reflect. > > Formatting nit: since you make generateProxyClass method package-private, > line 323 & 335-336 needs to

Re: General question: sun package -> jdk package?

2015-12-15 Thread mark . reinhold
2015/12/15 7:09 -0800, Paul Benedict : > I have a general question prompted by the many classes moved from sun.* to > jdk.*. Once JDK 9 delivers on the Module System and internals are no longer > exposed, is it planned to eventually migrate away from the sun.* legacy > namespace in later JDK versio

Re: RFR: JDK-8115868: 32-bit JVM failed to start from a large network filesystem

2015-12-15 Thread Martin Buchholz
Note that the semantics of stat and access may be subtly different, and that there are many calls to stat in the JDK sources, and they may all be broken on 32-bit systems. I just wrote elsewhere: _FILE_OFFSET_BITS is generally an all-or-nothing thing, because it affects interoperability between t

Re: General question: sun package -> jdk package?

2015-12-15 Thread Paul Benedict
I only asked out of curiosity. There seemed to be a migration trend, but I didn't know what the intent was. I understand a name is just a name, but I personally do like seeing the vestiges of "sun" being replaced with something more universal. Cheers, Paul On Tue, Dec 15, 2015 at 1:48 PM, wrote:

Re: RFR [9] Moved sun.misc.ProxyGenerator to jdk.internal.reflect

2015-12-15 Thread Alan Bateman
On 15/12/2015 18:41, Chris Hegarty wrote: : I would prefer it moving to java.lang.reflect as package-private class in java.lang.reflect since we are moving it. I have no objection. Webrev updated in-place. java.lang.reflect with package access is okay with me too. -Alan

Re: RFR: JDK-8115868: 32-bit JVM failed to start from a large network filesystem

2015-12-15 Thread Martin Buchholz
OK, now doing an actual review ... this does look like a clear improvement!

Re: Need help to understand TLS behavior

2015-12-15 Thread Martin Buchholz
On Mon, Dec 14, 2015 at 8:44 PM, David Holmes wrote: > On 15/12/2015 6:53 AM, Martin Buchholz wrote: >> >> Thread local storage is trouble. >> >> java stack sizes should be in _addition_ to any OS overhead, which >> includes TLS. > > > TLS shouldn't be coming out the stack of the thread AFAIK - I

Re: RFR: JDK-8115868: 32-bit JVM failed to start from a large network filesystem

2015-12-15 Thread Bernd Eckenfels
Hello, I always like it when access() is used instead of stat() magic. I noticed that the new ProgramExists in java_md_common.c does not anymore reject directories (which are typically executable). Not sure it this matters or is intentional, it is a change in semantic. Is there an exising typo i

Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-12-15 Thread Peter Levart
Hi Roger, Just one thing about implementation: Since the type exposed to user is Cleaner.Cleanable that has only a single method clean(), it would be good if the implementation class (CleanerImpl.PhantomCleanableRef) overrode CleanerImpl.PhantomCleanable.clear() method and threw UnsupportedO

Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-12-15 Thread Roger Riggs
Hi Peter, That will break up clearing the ref when the Cleanable is explicitly cleaned. Reference.clear() needs to be called from Cleanable.clean(). it might be nice to block that but to do so we'd need to go back to separate objects for the Reference and the Cleanable and we worked hard to g

Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-12-15 Thread Peter Levart
On 12/15/2015 11:48 PM, Roger Riggs wrote: Hi Peter, That will break up clearing the ref when the Cleanable is explicitly cleaned. Reference.clear() needs to be called from Cleanable.clean(). From PhantomCleanable (the superclass of PhantomCleanableRef): 253 @Override 254

Re: Need help to understand TLS behavior

2015-12-15 Thread David Holmes
On 16/12/2015 6:23 AM, Martin Buchholz wrote: On Mon, Dec 14, 2015 at 8:44 PM, David Holmes wrote: On 15/12/2015 6:53 AM, Martin Buchholz wrote: Thread local storage is trouble. java stack sizes should be in _addition_ to any OS overhead, which includes TLS. TLS shouldn't be coming out th

Re: Need help to understand TLS behavior

2015-12-15 Thread David Holmes
On 16/12/2015 8:03 AM, Jeremy Manson wrote: On Mon, Dec 14, 2015 at 11:25 PM, David Holmes mailto:david.hol...@oracle.com>> wrote: On 15/12/2015 4:32 PM, Jeremy Manson wrote: David: What the spec says and what glibc does are two different things: https://sourcew