RFR (S) 8136500: Integer/Long getChars and stringSize should be more idiomatic

2015-11-20 Thread Aleksey Shipilev
Hi, We have discovered this both in Compact Strings and Indy String Concat work, but it deserves to be treated separately. Integer/Long getChars code seems to be very old (Josh Bloch estimated circa 1994) and written under the assumption no compiler is here to help us. Fast-forward 20 years, and

Re: [RFR]: 8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

2015-11-20 Thread huizhe wang
On 11/19/2015 12:45 PM, Alan Bateman wrote: On 19/11/2015 18:36, Daniel Fuchs wrote: Hi Miran, I would expect this to fail horribly - you may not have the right to create anything under $JAVA_HOME/conf - it may even be a read-only file system... And I would be very uneasy as this may have s

Re: RFR(L): JDK-8046936 : JEP 270: Reserved Stack Areas for Critical Sections

2015-11-20 Thread Karen Kinnear
Frederic, Code review for web revs you sent out. Code looks good. I am not as familiar with the compiler code. I realize you need to check in at least a subset of the java.util.concurrent changes for the test to work, so maybe I should not have asked Doug about his preference there. Sorry. I a

Re: RFR(L): JDK-8046936 : JEP 270: Reserved Stack Areas for Critical Sections

2015-11-20 Thread Karen Kinnear
Totally appreciate the suggestion that the java.util.concurrent modifications be done by folks with more domain expertise. Would you have us incorporate the initial minimal set of j.u.c. updates or none at all? thanks, Karen > On Nov 11, 2015, at 8:09 PM, Doug Lea wrote: > > On 11/10/2015 12:

Re: RFR(L): JDK-8046936 : JEP 270: Reserved Stack Areas for Critical Sections

2015-11-20 Thread Karen Kinnear
One other example is an unlock that requires three steps - update state, update owner and unpark successor. thanks, Karen > On Nov 19, 2015, at 9:25 PM, David Holmes wrote: > > On 20/11/2015 4:10 AM, Doug Lea wrote: >> On 11/16/2015 08:00 PM, David Holmes wrote: I've tried that :-) The am

Re: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions

2015-11-20 Thread Alexander Fomin
Hi Jason On 20.11.2015 17:47, Jason Mehrens wrote: Alexander, Why not just cache the last exception in the formatter and use getTail to clear it and report it? It might be a good solution, but according to spec Formatter#getTail() method is intended to return the tail string for a set

Re: Code Review for JEP 259: Stack-Walking API

2015-11-20 Thread Mandy Chung
> On Nov 20, 2015, at 1:59 AM, serguei.spit...@oracle.com wrote: > > The 'int bci' is not used above. This is weird. Daniel caught that and I took that line out already. http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.03-delta/hotspot/src/share/vm/classfile/javaClasses.cpp.sdiff.html A

Re: Code Review for JEP 259: Stack-Walking API

2015-11-20 Thread Daniel Fuchs
On 20/11/15 03:13, Mandy Chung wrote: Cross-posting to core-libs-dev. Delta webrev incorporating feedback from Coleen and Brent and also a fix that Daniel made: http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.03-delta/ Full webrev: http://cr.openjdk.java.net/~mchung/jdk9/jep259/w

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

2015-11-20 Thread Attila Szegedi
On Nov 20, 2015, at 4:10 PM, Alan Bateman wrote: > > On 19/11/2015 23:15, Attila Szegedi wrote: >> Please review JDK-8141338 "Move jdk.internal.dynalink package to >> jdk.dynalink" for . This >> is basically the implementation step for integrati

Re: [RFR]: 8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

2015-11-20 Thread Miroslav Kos
On 20/11/15 15:25, Daniel Fuchs wrote: On 20/11/15 09:43, Miroslav Kos wrote: Locally, it worked for me, but I understand that it's dengerous - global state is evil ... I removed all the scenarios modifying JDK/conf/ dir - I just commented out those parts so if necessary, it can be uncommented a

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

2015-11-20 Thread Alan Bateman
On 19/11/2015 23:15, Attila Szegedi wrote: Please review JDK-8141338 "Move jdk.internal.dynalink package to jdk.dynalink" for . This is basically the implementation step for integrating JEP 276. This changeset will introduce a new public API tha

Re: [RFR]: 8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

2015-11-20 Thread Daniel Fuchs
On 20/11/15 09:43, Miroslav Kos wrote: Locally, it worked for me, but I understand that it's dengerous - global state is evil ... I removed all the scenarios modifying JDK/conf/ dir - I just commented out those parts so if necessary, it can be uncommented and run locally. http://cr.openjdk.java.n

Re: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions

2015-11-20 Thread Jason Mehrens
Alexander, I see your point. It is also out of spec for Handler.setFormatter to actually call Formatter.setErrorManager. What if there are subclasses of formatter that exist today with a setErrorManager method? Also not all handlers have one formatter or actually call super.setFormatter sinc

Re: Code Review for JEP 259: Stack-Walking API

2015-11-20 Thread serguei.spit...@oracle.com
On 11/20/15 08:39, Mandy Chung wrote: On Nov 20, 2015, at 1:59 AM, serguei.spit...@oracle.com wrote: The 'int bci' is not used above. This is weird. Daniel caught that and I took that line out already. http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.03-delta/hotspot/src/share/vm/class

Re: RFR JDK-8143330: No appropriate CCC request for two new protected methods added in JDK9b93 by JDK-8141132

2015-11-20 Thread Xueming Shen
I missed the override version in StringBuffer.java, thanks Tobias for catching it. http://cr.openjdk.java.net/~sherman/8143553 https://bugs.openjdk.java.net/browse/JDK-8143553 I did re-generate the api doc, but still missed it :-( Thanks, Sherman On 11/19/2015 12:50 PM, Xueming Shen wrote:

JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions

2015-11-20 Thread Alexander Fomin
Hi, please, review this patch to report errors in java.util.logging.Formatter#formatMessage(). Bug: https://bugs.openjdk.java.net/browse/JDK-8137005 Webrev: http://cr.openjdk.java.net/~dfuchs/alexander/8137005/webrev.00 Summary: j.u.logging.Formatter#formatMessage() swallows exceptions th

Re: RFR: JDK-8140364: JEP 264 Platform Logger API and Service Implementation

2015-11-20 Thread Mandy Chung
> On Nov 20, 2015, at 9:35 AM, Daniel Fuchs wrote: > > Yet a small update integrating your latest editorial > suggestions and feedback from the CCC. > > Also tweaked BootstrapLogger. > > I pushed the changes to the sandbox. > http://hg.openjdk.java.net/jdk9/sandbox/jdk/shortlog/JDK-8046565-bra

Re: [RFR]: 8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

2015-11-20 Thread Lance Andersen
Hi Miran, On Nov 20, 2015, at 10:29 AM, Miroslav Kos wrote: > On 20/11/15 15:25, Daniel Fuchs wrote: >> On 20/11/15 09:43, Miroslav Kos wrote: >>> Locally, it worked for me, but I understand that it's dengerous - global >>> state is evil ... I removed all the scenarios modifying JDK/conf/ dir

Re: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions

2015-11-20 Thread Jason Mehrens
Right. I know attachments get trashed sometimes so hopefully this hacked up code in-lines correctly: = private Exception formatThrown; private volatile Handler lastHandler; private void reportError(Handler h, Exception ex) { if (h == null)

Re: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions

2015-11-20 Thread Alexander Fomin
On 20.11.2015 20:04, Daniel Fuchs wrote: On 20/11/15 17:55, Jason Mehrens wrote: Hi Daniel, Well I'm sure the authors of the unit tests wrote code that never leaks the handlers they have created right? :) If urgency or frequency of the reporting is required then capture the handler in get

Re: RFR: JDK-8140364: JEP 264 Platform Logger API and Service Implementation

2015-11-20 Thread Daniel Fuchs
On 18/11/15 02:56, Mandy Chung wrote: On Nov 3, 2015, at 12:12 PM, Daniel Fuchs wrote: Hi Mandy, I have pushed an update that adds the diagnosability tweaks you asked me for in LoggerFinderLoader. I also added a couple of new tests. No changes in the API specification. : http://cr.openjdk.j

Re: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions

2015-11-20 Thread Daniel Fuchs
On 20/11/15 17:55, Jason Mehrens wrote: Hi Daniel, Well I'm sure the authors of the unit tests wrote code that never leaks the handlers they have created right? :) If urgency or frequency of the reporting is required then capture the handler in getHead as formatter state. The write code to r

Re: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions

2015-11-20 Thread Jason Mehrens
Hi Daniel, Well I'm sure the authors of the unit tests wrote code that never leaks the handlers they have created right? :) If urgency or frequency of the reporting is required then capture the handler in getHead as formatter state. The write code to report the exception under all possible st

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

2015-11-20 Thread Attila Szegedi
Thanks for pointing out these, Mandy; I will make the changes you suggested. Attila. > On Nov 20, 2015, at 6:10 AM, Mandy Chung wrote: > > jdk.scripting.nashorn is loaded by the extension class loader. Is > jdk.dynalink expected to be loaded by the ext. class loader? > > You need to edit thi

Re: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions

2015-11-20 Thread Daniel Fuchs
On 20/11/15 15:47, Jason Mehrens wrote: Alexander, Why not just cache the last exception in the formatter and use getTail to clear it and report it? Since formatter is in the same package as Handler you will have elevated access to the error manager through Handler.reportError. That also ma

Re: Code Review for JEP 259: Stack-Walking API

2015-11-20 Thread serguei.spit...@oracle.com
Somehow some of the formatting in my reply is gone. I'm trying to fix it below... Thanks, Serguei On 11/20/15 01:59, serguei.spit...@oracle.com wrote: Hi Mandy, It looks pretty good to me. At least, I do not see any obvious issues. Just some minor comments below. webrev.03/hotspot/src/s

Re: RFR(L): JDK-8046936 : JEP 270: Reserved Stack Areas for Critical Sections

2015-11-20 Thread Frederic Parain
David, On 17/11/2015 02:00, David Holmes wrote: Another issue is that writing a rule for javac that is not overpessimistic is also an intractable problem. The pattern "atomic operation followed by method invocation to complete status update" is too general to be the trigger of the annotation. 1)

Re: RFR JDK-8143330: No appropriate CCC request for two new protected methods added in JDK9b93 by JDK-8141132

2015-11-20 Thread Tobias Hartmann
Hi Sherman, On 19.11.2015 21:27, Xueming Shen wrote: > Hi > > Please help review the change for 8143330. > > Issue: https://bugs.openjdk.java.net/browse/JDK-8143330 > webrev: http://cr.openjdk.java.net/~sherman/8143330/webrev What about this 733 protected synchronized void getBytes(byte ds

Re: Code Review for JEP 259: Stack-Walking API

2015-11-20 Thread serguei.spit...@oracle.com
Hi Mandy, It looks pretty good to me. At least, I do not see any obvious issues. Just some minor comments below. webrev.03/hotspot/src/share/vm/classfile/javaClasses.cpp 2128 Symbol* java_lang_StackFrameInfo::get_file_name(Handle stackFrame, InstanceKlass* holder) { 2129 if (MemberNameInS

Re: RFR(L): 8139885: implement JEP 274: enhanced method handles

2015-11-20 Thread Paul Sandoz
> On 19 Nov 2015, at 22:16, Michael Haupt wrote: > > Hi John, Paul, > > thanks for your reviews - they have helped me polish the code and > documentation some more and add some more tests. The result is at > http://cr.openjdk.java.net/~mhaupt/8139885/webrev.02 >

Re: [RFR]: 8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

2015-11-20 Thread Miroslav Kos
Locally, it worked for me, but I understand that it's dengerous - global state is evil ... I removed all the scenarios modifying JDK/conf/ dir - I just commented out those parts so if necessary, it can be uncommented and run locally. http://cr.openjdk.java.net/~mkos/8131334/jdk.02/ Thanks Mira

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

2015-11-20 Thread Mandy Chung
jdk.scripting.nashorn is loaded by the extension class loader. Is jdk.dynalink expected to be loaded by the ext. class loader? You need to edit this file to include the new module: jdk/make/src/classes/build/tools/module/ext.modules This is an interim file to map modules to class loader and