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

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

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

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

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 ). >

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 >

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 @@ } /** - *

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

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

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 >

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 +++

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

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

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,

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

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

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

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

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

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

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

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

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

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

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

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 > wrote: On 15/12/2015 4:32 PM, Jeremy Manson wrote: David: What the spec says and what glibc does are two different

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

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

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

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 ).

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

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

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