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

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.

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

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

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

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

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

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

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

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

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

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

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

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?

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

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:

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

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

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

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

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

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

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

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

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

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

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

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

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

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.

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

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

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

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

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

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:

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

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

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

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.

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

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

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

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

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

> 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: Code Review for JEP 259: Stack-Walking API

> 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

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

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: [RFR]: 8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

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

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

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:

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

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

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

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

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

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: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

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