Re: Memory leak in com.sun.jndi.ldap.pool.PoolCleaner

2016-05-11 Thread Alan Bateman
On 11/05/2016 12:18, Mark Thomas wrote: : Sure. Done. Review ID: 9087338 I found it, and moved it to the JDK project in JIRA: https://bugs.openjdk.java.net/browse/JDK-8156824

Re: [9] RFR: 8145974: XMLStreamWriter produces invalid XML for surrogate pairs on OutputStreamWriter

2016-05-11 Thread huizhe wang
Hi Aleksej, The change looks good overall. It may be better to replace the name "writeCodePoint" with "writeCharRef" or "writeEscaped". Doing the "isSurrogatePair" check in place of the call for writeSurrogatePair may be more descriptive and readable as well, that is: +if (

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread Sundararajan Athijegannathan
+1 -Sundar On 5/11/2016 10:01 PM, shilpi.rast...@oracle.com wrote: > Hi All, > > Please review the updated webrev- > http://cr.openjdk.java.net/~srastogi/8149574/webrev.07/ > > Changed the anonymous class package with no package name. > > Regards, > Shilpi > > On 5/11/2016 8:22 PM, Paul Sandoz w

Re: RFR 8029891 : Deadlock detected in java/lang/ClassLoader/deadlock/GetResource.java - A Revival

2016-05-11 Thread David Holmes
Hi Brent, On 11/05/2016 7:56 AM, Brent Christian wrote: Hi! Welcome back to 8029891, "Deadlock detected in java/lang/ClassLoader/deadlock/GetResource.java". Our previous discussion is at [1]. Briefly, java.util.Properties isa Hashtable, which synchronizes on itself for any access. System.getP

Re: RFR(xs): 8156810 remove redundant sentence in SecurityManager.checkMemberAccess doc

2016-05-11 Thread Sean Mullan
Looks good. --Sean On 5/11/16 4:47 PM, Stuart Marks wrote: Hi all, Regarding this bug, https://bugs.openjdk.java.net/browse/JDK-8156810 I had recently "upgraded" the deprecation annotation of SecurityManager.checkMemberAccess() to include forRemoval=true. [1] [2] This included the addit

Re: RFR(xs): 8156810 remove redundant sentence in SecurityManager.checkMemberAccess doc

2016-05-11 Thread Joseph D. Darcy
Looks fine Stuart, -Joe On 5/11/2016 4:47 PM, Stuart Marks wrote: Hi all, Regarding this bug, https://bugs.openjdk.java.net/browse/JDK-8156810 I had recently "upgraded" the deprecation annotation of SecurityManager.checkMemberAccess() to include forRemoval=true. [1] [2] This included t

RFR(xs): 8156810 remove redundant sentence in SecurityManager.checkMemberAccess doc

2016-05-11 Thread Stuart Marks
Hi all, Regarding this bug, https://bugs.openjdk.java.net/browse/JDK-8156810 I had recently "upgraded" the deprecation annotation of SecurityManager.checkMemberAccess() to include forRemoval=true. [1] [2] This included the addition of some text about removal in a future version. Unfortun

[9] RFR: 8145974: XMLStreamWriter produces invalid XML for surrogate pairs on OutputStreamWriter

2016-05-11 Thread Aleks Efimov
Hello, Please, help to review the fix for XMLStreamWriter bug [1]: XMLStreamWriter incorrectly writes surrogate pairs into pair of invalid character references. For example: "\ud83d\ude0a" is transformed into "��". It should be one character reference "😊" instead. The proposed patch fixes the X

Re: MethodHandle for array length

2016-05-11 Thread Remi Forax
- Mail original - > De: "Uwe Schindler" > À: "Core-Libs-Dev" > Envoyé: Mercredi 11 Mai 2016 21:35:32 > Objet: MethodHandle for array length > > Hi, > Hi Uwe, > while working and trying the new JDK9 MethodHandles features like > MethodHandles#countedLoop, I recognized a API inconsisten

Re: RFR(m): 8140281 deprecate Optional.get()

2016-05-11 Thread Martin Buchholz
On Wed, May 11, 2016 at 2:04 PM, Stuart Marks wrote: > This works if you're willing to build once and use the same binary on > different JDK versions. Where you might run into trouble is if you want to > use the same source code and compile it using different JDK versions. Is > this what you're do

Re: RFR(m): 8139233u1 add initial compact immutable collection implementations

2016-05-11 Thread Stuart Marks
Hi, I agree with Stephen, using an enum here is overkill. :-) The way I think about this is that the collection implementations and the CollSer proxy are fairly closely coupled, so adding an enum here only makes this coupling more obscure. There's another wrinkle that I had slipped into the

Re: RFR(m): 8140281 deprecate Optional.get()

2016-05-11 Thread Stuart Marks
On 5/10/16 7:59 PM, Martin Buchholz wrote: Consider a library like guava classic. It currently supports jdk 6+, and there's a good chance that library will do that until its end of life, which is likely to be around the time when EVERYBODY is using jdk 8+, which is still many years away. Even

Re: Review request for JDK-8156680: jdeps implementation refresh

2016-05-11 Thread Mandy Chung
> On May 11, 2016, at 10:39 AM, Daniel Fuchs wrote: > > Hi Mandy, > > I had a look at the first webrev. > > Code reorganization in some of the files makes it difficult > to follow what is going on. I will try to import > the changes in my local workspace tomorrow and play with > it a bit. Tha

MethodHandle for array length

2016-05-11 Thread Uwe Schindler
Hi, while working and trying the new JDK9 MethodHandles features like MethodHandles#countedLoop, I recognized a API inconsistency problem with it. We also found this while implementing the "Painless" scripting language for Elasticsearch (see https://goo.gl/DbOzjC)! Painless is a very limited sc

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-11 Thread Ralph Goers
I am not at all familiar with how the stack is actually managed. I was hoping it was just an array and that the index into it was being kept track of. Since we know that we will only be adding more stack elements I would think that deferring use of that index wouldn’t be a problem. But since I h

Re: RFR 8029891 : Deadlock detected in java/lang/ClassLoader/deadlock/GetResource.java - A Revival

2016-05-11 Thread Brent Christian
On 5/10/16 2:56 PM, Brent Christian wrote: While good progress was made during the original code review, all of the overridden methods in Properties caused an explosion of unnecessary JavaDoc (see specdiff at [2]). With the recent fix of 8073100 (new "@hidden" JavaDoc tag), we can now avoid the

Re: Review request for JDK-8156680: jdeps implementation refresh

2016-05-11 Thread Daniel Fuchs
On 11/05/16 02:12, Mandy Chung wrote: JDK-8153481: tools/jdeps/modules/GenModuleInfo.java and ModuleTest.java fails intermittently Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153481/webrev.00/ Mandy second webrev above looks good. best regards, -- daniel

Re: Review request for JDK-8156680: jdeps implementation refresh

2016-05-11 Thread Daniel Fuchs
Hi Mandy, I had a look at the first webrev. Code reorganization in some of the files makes it difficult to follow what is going on. I will try to import the changes in my local workspace tomorrow and play with it a bit. I have two small comments so far: On 11/05/16 02:12, Mandy Chung wrote: J

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-11 Thread Daniel Fuchs
Hi, On 11/05/16 18:57, Ralph Goers wrote: But my point is that if obtaining the StackFrame and Class could be done so quickly that it wouldn’t add any noticeable overhead we could do that in every Logger.info, debug, etc. call. If we could just get the stack frame index so that we could obtain t

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-11 Thread Ralph Goers
OK, I will try to give that a try over the next few days and try to see how much overhead it incurs. Ralph > On May 11, 2016, at 9:43 AM, Mandy Chung wrote: > > >> On May 11, 2016, at 9:24 AM, Ralph Goers wrote: >> >> Currently StackWalker has a getCallerClass method, but not a >> getCalle

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-11 Thread Ralph Goers
It happens in situations like Foo:method1—>Logger.log—>DBAppender.append—>Hibernate.something—>Logger.log. It is actually pretty common when you create appenders that want to interact with third party libraries. For example, an Appender might use Netty to interact with sockets, Thrift to acce

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-11 Thread Mandy Chung
> On May 11, 2016, at 9:24 AM, Ralph Goers wrote: > > Currently StackWalker has a getCallerClass method, but not a > getCallerStackFrame. Having that method could also solve the problem, but > only if it is efficient enough that it could be called for every log event, > since when we would be

Re: RFR(m): 8140281 deprecate Optional.get()

2016-05-11 Thread Peter Levart
On 05/11/2016 06:32 PM, Martin Buchholz wrote: On Wed, May 11, 2016 at 9:17 AM, Peter Levart wrote: Hi Martin, On 05/11/2016 05:59 PM, Martin Buchholz wrote: On Wed, May 11, 2016 at 12:05 AM, Peter Levart wrote: Couldn't this new @Deprecated.since annotation attribute play a role here?

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-11 Thread Daniel Fuchs
Hi Ralph, On 11/05/16 18:14, Ralph Goers wrote: You are correct that most of the time it would be faster to start from frame 0. However, the problem we have with walking the stack from frame 0 is that it is possible to have a situation like Foo::method1—>Logger.log—>BarAppender.append—>Foo::m

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread shilpi.rast...@oracle.com
Hi All, Please review the updated webrev- http://cr.openjdk.java.net/~srastogi/8149574/webrev.07/ Changed the anonymous class package with no package name. Regards, Shilpi On 5/11/2016 8:22 PM, Paul Sandoz wrote: Hi Shilpi, What makes me slightly queasy about the constant pool patching of

Re: RFR(m): 8140281 deprecate Optional.get()

2016-05-11 Thread Martin Buchholz
On Wed, May 11, 2016 at 9:17 AM, Peter Levart wrote: > Hi Martin, > > > > On 05/11/2016 05:59 PM, Martin Buchholz wrote: >> >> On Wed, May 11, 2016 at 12:05 AM, Peter Levart >> wrote: >>> >>> Couldn't this new @Deprecated.since annotation attribute play a role >>> here? >>> For example, if you ar

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-11 Thread Ralph Goers
Currently StackWalker has a getCallerClass method, but not a getCallerStackFrame. Having that method could also solve the problem, but only if it is efficient enough that it could be called for every log event, since when we would be calling it we wouldn’t even know if the user required it. In

Re: RFR(m): 8140281 deprecate Optional.get()

2016-05-11 Thread Peter Levart
Hi Martin, On 05/11/2016 05:59 PM, Martin Buchholz wrote: On Wed, May 11, 2016 at 12:05 AM, Peter Levart wrote: Couldn't this new @Deprecated.since annotation attribute play a role here? For example, if you are building with JDK 9 javac, but specify -release 8 option, then you don't get depre

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-11 Thread Ralph Goers
Yes, we need the StakWalker API for other parts of our code where we need the Class so we can determine what ClassLoader to use. For creating a LogEvent with location information we primarily need the class name, method name and line number. However, the “extended” Throwable also returns the na

Re: RFR(m): 8140281 deprecate Optional.get()

2016-05-11 Thread Martin Buchholz
On Wed, May 11, 2016 at 12:05 AM, Peter Levart wrote: > Couldn't this new @Deprecated.since annotation attribute play a role here? > For example, if you are building with JDK 9 javac, but specify -release 8 > option, then you don't get deprecation warning for methods annotated with > @Deprecated(s

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread Rémi Forax
yes, adding @Hidden solves the first item of my list, but nevertheless changing the behavior of defineAnonymousClass does not solve the other items. That said, i hijack this thread because i have not noticed that defineAnonymousClass behavior was changed. I should have started another thread ab

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread Paul Sandoz
Hi Shilpi, What makes me slightly queasy about the constant pool patching of T is it’s quite fragile and it produces a class with a slightly inconsistent state regarding the InnerClasses structures. It’s probably mostly harmless but still bothers me. One solution is to combine ASM with constan

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-11 Thread Daniel Fuchs
Hi Ralph, On 10/05/16 19:40, Ralph Goers wrote: The benchmark logs events using Log4j 2’s asynchronous loggers. In the process of doing that it captures the location information using the code below: // LOG4J2-1029 new Throwable().getStackTrace is faster than Thread.currentThread().getStackT

Re: RFR[9]:java/lang/invoke/VarargsArrayTest.java miss othervm for main/bootclasspath mode

2016-05-11 Thread shilpi.rast...@oracle.com
Thank You Michael for comments! It was fixed for VarargsArrayTest.java but not fixed for java/lang/invoke/CustomizedLambdaFormTest.java. Boris Molodenkov added a comment -2016-05-02 02:37-Restricted toConfidential One mo

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread Michael Haupt
Hi Shilpi, thank you for preparing these patches. They both implement solution (A), which is to patch the package name. As it happens, most of the complexity in the solutions is due to the desire to make the solution tidy in terms of association of the anonymous class with the host class' packa

Re: RFR[9]:java/lang/invoke/VarargsArrayTest.java miss othervm for main/bootclasspath mode

2016-05-11 Thread Michael Haupt
Hi Shilpi, not a review - the bug mentions that this issue is fixed in Jake already and that the bug should be closed once the fix from Jake propagates to dev. What is the status of that? Best, Michael > Am 11.05.2016 um 13:24 schrieb shilpi.rast...@oracle.com: > > Hi All, > > Please review

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread Vladimir Ivanov
Let me clarify: both proposed patches move invoker class out of java.lang.invoke package, but add @Hidden on invoke_V instead. So, JVM should not list it in stack traces and you don't have to filter it out on your side. Moreover, I think the absence of @Hidden on j.l.i.MethodHandleImpl.T.inv

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread forax
- Mail original - > De: "Vladimir Ivanov" > À: "Remi Forax" , "shilpi rastogi" > > Cc: core-libs-dev@openjdk.java.net, "John Rose" , > "Michael Haupt" , > "paul sandoz" , "Da Vinci Machine Project" > > Envoyé: Mercredi 11 Mai 2016 14:50:25 > Objet: Re: RFR[9]:Fix java/lang/invoke/Meth

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread Vladimir Ivanov
Remi, I'm curious why doesn't @Hidden on the invoker method solve your problem? Best regards, Vladimir Ivanov On 5/11/16 3:44 PM, Remi Forax wrote: Hi all, changing the behavior of defineAnonymousClass in 9 is huge burden for me and i believe for anybody that maintains a dynamic language runt

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread Remi Forax
Hi all, changing the behavior of defineAnonymousClass in 9 is huge burden for me and i believe for anybody that maintains a dynamic language runtime. As an implementer, being able to choose the package of an anonymous class is an important feature. I use to choose carefully the package name for:

RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

2016-05-11 Thread shilpi.rast...@oracle.com
Hi All, Please review the following- https://bugs.openjdk.java.net/browse/JDK-8149574 Solution: Changed anonymous class package name with the package name of its host class. Two approaches to solve this- 1. Parse .class and get the class name index form constant pool and patch it with new

RFR[9]:java/lang/invoke/VarargsArrayTest.java miss othervm for main/bootclasspath mode

2016-05-11 Thread shilpi.rast...@oracle.com
Hi All, Please review one small fix for https://bugs.openjdk.java.net/browse/JDK-8155791 http://cr.openjdk.java.net/~srastogi/8155791/webrev.00/ Thanks, Shilpi

Re: Memory leak in com.sun.jndi.ldap.pool.PoolCleaner

2016-05-11 Thread Mark Thomas
On 11/05/2016 08:18, Alan Bateman wrote: > On 10/05/2016 23:05, Mark Thomas wrote: >> Hi, >> >> While working my way through Tomcat's memory leak protection / detection >> / fixing code, I have found an issue that remains unresolved in the >> latest JDK 9 source. >> >> The PoolCleaner thread starte

RFR: Implement RandomAccess spliterator

2016-05-11 Thread Ito, Hiroshi
Hi, Please kindly review the attached patch for RandomAccessSpliterator implementation. Currently default implementation of spliterator is an IteratorSpliterator which is not optimal for RandomAccess data structures (besides ArrayList and Vector). This patch is to provide a default RandomAcces

Re: RFR:JDK-8155823: Add date-time patterns 'v' and 'vvvv'

2016-05-11 Thread Stephen Colebourne
This seems fine by me. Stephen On 10 May 2016 at 19:25, nadeesh tv wrote: > Hi, > > Stephen, Roger Thanks for the comments. > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8155823/webrev.04/ > > Apart from the fix, made a correction in the java doc of ZoneRules.java > > Than

Re: RFR (JAXP) 8152912: SAX XMLReaderFactory needs to be ServiceLoader compliant

2016-05-11 Thread huizhe wang
On 5/10/2016 4:21 PM, Stuart Marks wrote: On 5/9/16 2:59 PM, huizhe wang wrote: This looks much better to me. I just wonder about the @Deprecated("5") vs @Deprecated("1.5") Yes, the compiler accepted it just fine and produced javadocs similar to that of XMLReaderFactory where since="9". H

Re: Memory leak in com.sun.jndi.ldap.pool.PoolCleaner

2016-05-11 Thread Alan Bateman
On 10/05/2016 23:05, Mark Thomas wrote: Hi, While working my way through Tomcat's memory leak protection / detection / fixing code, I have found an issue that remains unresolved in the latest JDK 9 source. The PoolCleaner thread started by the LdapPoolManager when the idle timeout is positive d

Re: RFR(m): 8140281 deprecate Optional.get()

2016-05-11 Thread Peter Levart
On 05/11/2016 02:44 AM, Stuart Marks wrote: On 4/28/16 7:23 PM, Martin Buchholz wrote: No opinion on Optional.get() itself, but here's an opinion on the high cost of deprecation. If you deprecate the API today, and I have a library that is already using the API, then I should add a @SuppressW