Re: RFR: 8016725: TEST_BUG: java/lang/reflect/Method/DefaultMethodModeling.java failing intermittently

2013-11-04 Thread Joel Borggrén-Franck
I'll sponsor this. cheers /Joel On 31 okt 2013, at 01:33, Joseph Darcy joe.da...@oracle.com wrote: Hi Andreas, Approved; thanks, -Joe On 10/29/2013 3:26 AM, Andreas Lundblad wrote: Hi, Please review the fix for JDK-8016725 below. Description: DefaultMethodModeling.java and

Re: Assorted annotation optimizations

2013-11-04 Thread Joel Borggrén-Franck
Hi Peter, As always, thanks for doing this! On 2 nov 2013, at 22:58, Peter Levart peter.lev...@gmail.com wrote: Hi, I propose a set of straightforward optimizations of annotations in the field of minimizing garbage creation and execution performance. Here's a webrev containing the

RFR: JDK-8027796: Refactor Core Reflection for Type Annotations

2013-11-04 Thread Joel Borggrén-Franck
Hi Please review this small refactoring of the type annotations reflection code. This fix just makes the types final since the code wasn't designed for inheritance. webrev: http://cr.openjdk.java.net/~jfranck/8027796/webrev.00/ jbs: https://bugs.openjdk.java.net/browse/JDK-8027796 cheers

Re: hg: jdk8/tl/jdk: 7194897: JSR 292: Cannot create more than 16 instances of an anonymous class; ...

2013-11-05 Thread Joel Borggrén-Franck
Hi Peter, Narrowing this to core-libs-dev. On 5 nov 2013, at 09:25, Peter Levart peter.lev...@gmail.com wrote: Hi Robert, I think this fix is not complete. When one sets the system property sun.reflect.noInflation=true, reflection proxy is still attempted to be generated for anonymous

Re: hg: jdk8/tl/jdk: 7194897: JSR 292: Cannot create more than 16 instances of an anonymous class; ...

2013-11-05 Thread Joel Borggrén-Franck
On 5 nov 2013, at 10:51, Peter Levart peter.lev...@gmail.com wrote: On 11/05/2013 10:33 AM, Joel Borggrén-Franck wrote: I would also restructure the Method/Constructor accessor logic differently. The check for ReflectUtil.isVMAnonymousClass() can be performed just once

Re: RFR: JDK-8027796: Refactor Core Reflection for Type Annotations

2013-11-07 Thread Joel Borggrén-Franck
Thanks for the review Paul cheers /Joel On 2013-11-05, Paul Sandoz wrote: On Nov 4, 2013, at 8:36 PM, Joel Borggrén-Franck joel.fra...@oracle.com wrote: Hi Please review this small refactoring of the type annotations reflection code. This fix just makes the types final since

Re: JDK RFR to clean-up lint warnings in reflection implementation

2013-11-08 Thread Joel Borggrén-Franck
Looks good Joe. cheers /Joel On 8 nov 2013, at 20:40, Joe Darcy joe.da...@oracle.com wrote: Hello, Please review the simple patch below which addresses a handful of raw types lint warning in the core reflection implementation code. (If memory serves, this code dates back from a time

Re: RFR (XXS) 8027803: NPE in test infrastructure aka: test/sun/reflect/AnonymousNewInstance/ManyNewInstanceAnonTest.java fails

2013-11-10 Thread Joel Borggrén-Franck
Hi Robert, Looks good. cheers /Joel On 10 nov 2013, at 08:24, Robert Field robert.fi...@oracle.com wrote: Please review the fix for: https://bugs.openjdk.java.net/browse/JDK-8027803 Basically, the ClassFileInstaller test utility is getting a Null Pointer Exception when the

Re: RFR: JDK-8027413: Clarify javadoc for j.l.a.Target and j.l.a.ElementType

2013-11-20 Thread Joel Borggrén-Franck
Thanks for the review, change has been pushed. cheers /Joel On 19 Nov 2013, at 21:52, Joe Darcy joe.da...@oracle.com wrote: Hi Joel, The change looks good; approved to go back. Thanks, -Joe On 11/15/2013 04:26 AM, Joel Borggren-Franck wrote: Hi Please review this javadoc

RFR: JDK-8023278: Reflection API methods do not throw AnnotationFormatError in case of malformed Runtime[In]VisibleTypeAnnotations attribute

2013-11-21 Thread Joel Borggrén-Franck
Hi Please review this fix to the type annotation reflection parser. The wrong kind of exception was thrown in the case of malformed Runtime[In]VisibleTypeAnnotations . Also the parser needed to be fixed for sign issues in a handful of places. The test has tree broken classes encoded that

RFR: JDK-8029117: (reflect) clarify javadoc for getMethod(...) and getMethods()

2013-12-03 Thread Joel Borggrén-Franck
Hi Please review this javadoc fix for Class.getMethod(name, params) and Class.getMethods() when called on an interface. This fix aligns the javadoc with the long standing implementation to not return any implicitly declared object methods when calling getMethod(s) on an interface. Bug:

Re: The s.m.Unsafe representation in hotspot and method registration

2014-03-24 Thread Joel Borggrén-Franck
Hi Paul, I would guess this is because of the HSX model where we could use the same VM with different majors of the JDK. Would that make sense? cheers /Joel On 24 Mar 2014, at 17:42, Paul Sandoz paul.san...@oracle.com wrote: I started working on a patch to remove the monitor related methods

RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class

2014-05-07 Thread Joel Borggrén-Franck
Hi, Since Java 8 the methods Class.getMethod() and Class.getMethods() can in some circumstances return methods that are not members of the class. This is due to a spec change in 8 not due to a code change. For this to happen you need to have an interface hierarchy that looks something like:

Re: RFR: 5043030 (reflect) unnecessary object creation in reflection

2014-05-29 Thread Joel Borggrén-Franck
Hi, We need you send in the patches via the mailing list. Please reply to your mail with the diffs attached directly (not zipped for example). I’ll might have time to look at this next week, but it surprises me you need to patch hotspot. Allocations before we have inflated shouldn’t be a

RFR (S): JDK-8039916: AnnotatedType.getType() of a Executable parameters may return wrong type

2014-06-03 Thread Joel Borggrén-Franck
Hi Can I get a review for this small fix for https://bugs.openjdk.java.net/browse/JDK-8039916 Webrev here: http://cr.openjdk.java.net/~jfranck/8039916/webrev.01/ Since this is the second issue like this found recently I created a rather large test to see if there are any more lurking issues.

Re: RFR (S): JDK-8039916: AnnotatedType.getType() of a Executable parameters may return wrong type

2014-06-04 Thread Joel Borggrén-Franck
Thanks for the review Paul! I pushed the original, but I’ll experiment with your suggestion for the next set of tests. cheers /Joel On 4 jun 2014, at 12:34, Paul Sandoz paul.san...@oracle.com wrote: On Jun 4, 2014, at 12:25 PM, Paul Sandoz paul.san...@oracle.com wrote: You might consider

Re: RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class

2014-06-09 Thread Joel Borggrén-Franck
Hi Joe, IIRC name isn’t actually interned with String.intern() but in the VM:s Symbol table as a name representing a (Java) method. Should be equivalent, as long as we don’t start comparing it with == with interned Strings. As Andrej wrote, this is just refactoring of the previous code, to

Re: RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class

2014-06-10 Thread Joel Borggrén-Franck
Hi Andrej, On 9 jun 2014, at 21:56, Andrej Golovnin andrej.golov...@gmail.com wrote: Hi Joel, IIRC name isn’t actually interned with String.intern() but in the VM:s Symbol table as a name representing a (Java) method. Should be equivalent, as long as we don’t start comparing it with ==

Re: RFR: 5043030 (reflect) unnecessary object creation in reflection

2014-06-12 Thread Joel Borggrén-Franck
On 12 jun 2014, at 21:03, Andrej Golovnin andrej.golov...@gmail.com wrote: That is why we have tests :) You will have an easier time getting this accepted it you can show good code coverage of the fix in the current test suite for example. See if you can get jcov [1] up and running with

RFR 8u20: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class

2014-06-16 Thread Joel Borggrén-Franck
Hi, Can I get a review for this almost trivial 8u20 back port. The 9 fix reviewed here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-June/027173.html applies cleanly except one hunk fails due to a non-backported change updating a for to a for each: $ cat

RFR: JDK-8044629: (reflect) Constructor.getAnnotatedReceiverType() returns wrong value

2014-06-17 Thread Joel Borggrén-Franck
Hi, Can I get a review for this fix and javadoc clarification for https://bugs.openjdk.java.net/browse/JDK-8044629 The problem is with potentially annotated receiver parameters, they only exist for inner class constructors, so this fix makes sure that a ctor is for an inner class or returns

Re: RFR [8051382] Optimize java.lang.reflect.Modifier.toString()

2014-08-13 Thread Joel Borggrén-Franck
Hi Ivan, The first version looks good. I think this is desirable even if it is a tiny bit slower. cheers /Joel On 4 aug 2014, at 13:27, Ivan Gerasimov ivan.gerasi...@oracle.com wrote: Hi Martin! Sorry for the pause, I had to take a break. Thank you for your StringJoiner rework! I

RFR 8u: JDK-8044629: (reflect) Constructor.getAnnotatedReceiverType() returns wrong value

2014-08-20 Thread Joel Borggrén-Franck
Hi, On 17 jun 2014, at 18:52, Joel Borggrén-Franck joel.fra...@oracle.com wrote: Hi, Can I get a review for this fix and javadoc clarification for https://bugs.openjdk.java.net/browse/JDK-8044629 The problem is with potentially annotated receiver parameters, they only exist for inner

Re: RFR: 5043030 (reflect) unnecessary object creation in reflection

2014-08-29 Thread Joel Borggrén-Franck
Hi Andrej, On 22 jun 2014, at 00:00, Andrej Golovnin andrej.golov...@gmail.com wrote: Hi Joel, sorry for late response. I was too busy with other things. Likewise! I have changed the test TestMethodReflectValueOf as you suggested and I have changed the summary of both tests too. You

Re: RFR: 5043030 (reflect) unnecessary object creation in reflection

2014-09-09 Thread Joel Borggrén-Franck
Hi Andrej, Can you resend the latest patch attached to a mail to this list? cheers /Joel On 2014-08-29, Andrej Golovnin wrote: Hi Joel, I have changed the test TestMethodReflectValueOf as you suggested and I have changed the summary of both tests too. You will find the changes in the

Re: RFR: 5043030 (reflect) unnecessary object creation in reflection

2014-09-11 Thread Joel Borggrén-Franck
Hi, I pushed this two days ago. Thank you Andrej for the contribution! cheers /Joel On 9 sep 2014, at 11:21, Joel Borggrén-Franck joel.fra...@oracle.com wrote: Hi Andrej, Can you resend the latest patch attached to a mail to this list? cheers /Joel On 2014-08-29, Andrej Golovnin

[8u40] RFR: JDK-8058632: Revert JDK-8054984 from 8u40

2014-09-18 Thread Joel Borggrén-Franck
Hi, Can I get a review for this anti-patch for JDK-8054984 (which is a backport of JDK-8044629 (reflect) Constructor.getAnnotatedReceiverType() returns wrong value” to 8u). The fix shouldn’t have been back ported, so this change reverts it in 8u only. Webrev:

Re: [8u40] RFR: JDK-8058632: Revert JDK-8054984 from 8u40

2014-09-18 Thread Joel Borggrén-Franck
Thanks for the review Staffan! cheers /Joel On 2014-09-18, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 18 sep 2014, at 11:00, Joel Borggrén-Franck joel.fra...@oracle.com wrote: Hi, Can I get a review for this anti-patch for JDK-8054984 (which is a backport of JDK

Re: CloneNotSupportedException should extends RuntimeException not Exception

2012-10-15 Thread Joel Borggrén-Franck
On 10/15/2012 12:34 AM, David Holmes wrote: Remi, This ship has sailed you can't recall it. CloneNotSupportedException is a checked exception and needs to remain so for source and binary compatibility. I see how this is source incompatible and also behaviorally incompatible in a few

RFR: 8004699 : Add type annotation storage to Constructor, Field and Method

2012-12-17 Thread Joel Borggrén-Franck
Here is the first in a series of changes to add type annotation support to reflection. http://cr.openjdk.java.net/~jfranck/8004699/webrev.00/ While this adds overhead all instances of Field, Constructor, and Method we need this change in order to coordinate with the changes going in to HotSpot

Re: RFR: 8004699 : Add type annotation storage to Constructor, Field and Method

2012-12-17 Thread Joel Borggrén-Franck
On Dec 17, 2012, at 4:47 PM, Alan Bateman alan.bate...@oracle.com wrote: On 17/12/2012 15:41, Joel Borggrén-Franck wrote: Here is the first in a series of changes to add type annotation support to reflection. http://cr.openjdk.java.net/~jfranck/8004699/webrev.00/ I assume this should

Re: RFR: 8004699 : Add type annotation storage to Constructor, Field and Method

2012-12-17 Thread Joel Borggrén-Franck
On Dec 17, 2012, at 5:00 PM, Peter Levart peter.lev...@gmail.com wrote: Hi Joel, 82 // This is set by the vm at Method creation 83 private byte[] type_annotations; Wouldn't it be better to initialize this field in the constructor? You could create an overloaded

Re: RFR: 8004699 : Add type annotation storage to Constructor, Field and Method

2012-12-18 Thread Joel Borggrén-Franck
On 12/18/2012 02:58 AM, David Holmes wrote: On 18/12/2012 3:06 AM, Joel Borggrén-Franck wrote: On Dec 17, 2012, at 5:00 PM, Peter Levartpeter.lev...@gmail.com wrote: Hi Joel, 82 // This is set by the vm at Method creation 83 private byte[] type_annotations

Re: Review request:Updated JDK-8004728 Implement core reflection API for parameter reflection

2012-12-20 Thread Joel Borggrén-Franck
Hi Peter, Eric and and others, Thanks for your comments, On 12/20/2012 09:09 AM, Peter Levart wrote: Hi Eric and others, I'd also like to rise an internal design concern regarding construction of Parameter objects. Currently raw reflection data for Executable objects is obtained from the VM

Re: 8006344: Broken javadoc link in javax.lang.model.element.Element

2013-01-15 Thread Joel Borggrén-Franck
Thanks for catching this, Looks good. cheers /Joel On 15 jan 2013, at 18:59, Chris Hegarty chris.hega...@oracle.com wrote: Minor oversight in the changes from 7193719: Support repeating annotations in javax.lang.model.

Request for review: 8004698: Implement Core Reflection for Type Annotations

2013-01-17 Thread Joel Borggrén-Franck
Hi, Here is a webrev for core reflection support for type annotations: http://cr.openjdk.java.net/~jfranck/8004698/webrev.00/ This code is based on the tl/jdk repository, but in order to run the tests you need a javac with type annotations support and also a recent copy of the VM that

Re: Request for review: 8004698: Implement Core Reflection for Type Annotations

2013-01-22 Thread Joel Borggrén-Franck
05:23 PM, Joel Borggrén-Franck wrote: Hi, Here is a webrev for core reflection support for type annotations: http://cr.openjdk.java.net/~jfranck/8004698/webrev.00/ This code is based on the tl/jdk repository, but in order to run the tests you need a javac with type annotations support and also

Re: Request for review: 8004698: Implement Core Reflection for Type Annotations

2013-01-22 Thread Joel Borggrén-Franck
on Field/Method/Constructor/(TypeVar). Thank you again for your work on this! cheers /Joel Regards, Peter On Jan 17, 2013 5:24 PM, Joel Borggrén-Franck joel.fra...@oracle.com mailto:joel.fra...@oracle.com wrote: Hi, Here is a webrev for core reflection support for type annotations

Re: Request for review: 8004698: Implement Core Reflection for Type Annotations

2013-01-22 Thread Joel Borggrén-Franck
Hi, On Jan 22, 2013, at 6:01 PM, Peter Levart peter.lev...@gmail.com wrote: On 01/22/2013 01:47 PM, Joel Borggrén-Franck wrote: Hi Peter, Thanks for your comments, see inline, On 01/19/2013 06:11 PM, Peter Levart wrote: I see, there is a dilema how to cache type annotations

Re: Code review request for 8007113 Upgrade AnnotatedElement.isAnnotionPresent to be a default method

2013-02-01 Thread Joel Borggrén-Franck
Hi Joe, On 02/01/2013 04:21 AM, Joe Darcy wrote: Hello, I'd like to upgrade the existing AnnotatedElement.isAnnotationPresent interface method to a default method: 8007113 Upgrade AnnotatedElement.isAnnotionPresent to be a default method http://cr.openjdk.java.net/~darcy/8007113.0/

Re: Code review request for 8007113 Upgrade AnnotatedElement.isAnnotionPresent to be a default method

2013-02-01 Thread Joel Borggrén-Franck
Looks good (but I'm not a reviewer-kind of reviewer). cheers /Joel On Feb 1, 2013, at 10:28 PM, Joe Darcy joe.da...@oracle.com wrote: On 02/01/2013 11:16 AM, Joe Darcy wrote: Hi Joel, On 2/1/2013 1:47 AM, Joel Borggrén-Franck wrote: Hi Joe, On 02/01/2013 04:21 AM, Joe Darcy wrote

RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()

2013-03-06 Thread Joel Borggrén-Franck
Hi, Can I get a review of this small fix for issue 8007808: Missing method: Executable.getAnnotatedReturnType() http://cr.openjdk.java.net/~jfranck/8007808/webrev.00/ When we added Core Reflection support for type annotations this method got left out on Executable, but were included on

Re: RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()

2013-03-06 Thread Joel Borggrén-Franck
Thanks for the reviews Joe and Remi cheers /Joel On 6 mar 2013, at 17:15, Remi Forax fo...@univ-mlv.fr wrote: Hi Joel, looks good for me too :) Rémi On 03/06/2013 04:35 PM, Joe Darcy wrote: Hi Joel, Look good; approved. -Joe On 3/6/2013 6:37 AM, Joel Borggrén-Franck wrote: Hi

Re: JDK 8 RFR for JDK-7185456 : (ann) Optimize Annotation handling in java/sun.reflect.* code for small number of annotationsC

2013-03-27 Thread Joel Borggrén-Franck
Hi Joe, Looks good to me. I would include Andrej's suggestion of removing one of the members.put(...). cheers /Joel On 03/26/2013 11:43 PM, Joe Darcy wrote: Hello, Please review this refactoring of how annotations objects are created: JDK-7185456 : (ann) Optimize Annotation handling

Re: JDK 8 RFR for JDK-7185456 : (ann) Optimize Annotation handling in java/sun.reflect.* code for small number of annotationsC

2013-03-27 Thread Joel Borggrén-Franck
Hi Peter, On 03/27/2013 01:31 PM, Peter Levart wrote: I also don't know whether having LinkedHashMap instead of plain HashMap is necessary, since it is initialized with defaults from plain HashMap (which is hashCode % capacity ordered) and only some of defaults are overridden in parsing loop

Re: JDK 8 RFR for JDK-7185456 : (ann) Optimize Annotation handling in java/sun.reflect.* code for small number of annotationsC

2013-03-27 Thread Joel Borggrén-Franck
On 03/27/2013 03:00 PM, Peter Levart wrote: On 03/27/2013 02:00 PM, Joel Borggrén-Franck wrote: Hi Peter, On 03/27/2013 01:31 PM, Peter Levart wrote: I also don't know whether having LinkedHashMap instead of plain HashMap is necessary, since it is initialized with defaults from plain

Re: JDK 8 code review request for JDK-8004979 java.lang.reflect.Modifier.toString should include default

2013-03-28 Thread Joel Borggrén-Franck
Hi Jeroen, See http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cf0049037deb I'm not sure if this is in a promoted build yet. cheers /Joel On 28 mar 2013, at 11:38, Jeroen Frijters jer...@sumatra.nl wrote: Hi Joe, I notice that Method.isDefault() returns true for static interface methods. I

RFR 8012681: Commit for JDK-8012656 breaks tl build Was: can't build current jdk8/tl tip

2013-04-19 Thread Joel Borggrén-Franck
Hi On 04/19/2013 09:30 AM, Alan Bateman wrote: On 19/04/2013 08:08, Peter Levart wrote: Hi, I get the following compile-time error: /home/peter/work/hg/jdk8-tl/langtools/src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java:264: error: illegal start of expression if

Re: RFR 8012681: Commit for JDK-8012656 breaks tl build Was: can't build current jdk8/tl tip

2013-04-19 Thread Joel Borggrén-Franck
Fix pushed, thanks for the reviews. Peter, I missed your patch. My fix should probably have been attributed to you, sorry! cheers /Joel On 04/19/2013 10:49 AM, Joel Borggrén-Franck wrote: Hi On 04/19/2013 09:30 AM, Alan Bateman wrote: On 19/04/2013 08:08, Peter Levart wrote: Hi, I

RFR 8007073: Implement Core Reflection for Type Annotations on parameters

2013-05-03 Thread Joel Borggrén-Franck
Hello all, Here is an update to core reflection for type annotations adding support for reflecting over annotated type use on parameters. http://cr.openjdk.java.net/~jfranck/8007073/webrev.00/ Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8007073 cheers /Joel

Re: JDK 8 code review request for 8014836: Have GenericDeclaration extend AnnotatedElement

2013-05-21 Thread Joel Borggrén-Franck
Hi Joe, I applied the patch and built a jdk, looks good to me. (Not a Reviewer kind of reviewer tough.) cheers /Joel On 20 maj 2013, at 23:10, Joe Darcy joe.da...@oracle.com wrote: Hello, Please review the patch below which implements 8014836: Have GenericDeclaration extend

Re: 7122142 : (ann) Race condition between isAnnotationPresent and getAnnotations

2013-07-03 Thread Joel Borggrén-Franck
Hi Peter, As Alan said, a big thanks for looking into this. I have been toying with a slightly different approach to breaking the infinite recursion by pre-construct AnnotationType instances for Retention and Inherited. While cleaner in some places others became uglier. I'm fine with this

Re: 7122142 : (ann) Race condition between isAnnotationPresent and getAnnotations

2013-07-04 Thread Joel Borggrén-Franck
Hi Peter, On 4 jul 2013, at 16:40, Peter Levart peter.lev...@gmail.com wrote: Answers inline... There's another usage of AnnotationParser.parseAnnotation in TypeAnnotationParser that will need to be updated (I only noticed it when I grabbed your patch to try it). I rather restored

Re: 7122142 : (ann) Race condition between isAnnotationPresent and getAnnotations

2013-07-05 Thread Joel Borggrén-Franck
Hi Peter, Thanks for the quick update! While I have looked over the changes to j.l.Class and the cas in AnnotationType I don't think I'm qualified to review that. (FWIW it looked fine to me but my jmm isn't swapped in at the moment so I won't pretend to know the interactions between volatile

Re: RFR 7122142 : (ann) Race condition between isAnnotationPresent and getAnnotations

2013-07-15 Thread Joel Borggrén-Franck
7122142 : (ann) Race condition between isAnnotationPresent and getAnnotations* *Date: *8 juli 2013 22:54:12 CEST *To: *Joel Borggrén-Franck joel.fra...@oracle.com mailto:joel.fra...@oracle.com *Cc: *Alan Bateman alan.bate...@oracle.com mailto:alan.bate...@oracle.com, core-libs-dev@openjdk.java.net

Re: Code Review Request: More tests for 7184826: (reflect) Add support for Project Lambda concepts in core reflection

2013-07-25 Thread Joel Borggrén-Franck
Hi Dan, Thanks for looking at this, On Jul 23, 2013, at 11:12 PM, Dan Smith daniel.sm...@oracle.com wrote: Hi. Per a request from Joel, I've taken a look at DefaultStaticTestData. I don't really have the full context here, but I'm assuming that the annotations get translated into tests

Re: Code Review Request: More tests for 7184826: (reflect) Add support for Project Lambda concepts in core reflection

2013-07-25 Thread Joel Borggrén-Franck
Hi Amy, On Jul 24, 2013, at 3:30 AM, Amy Lu amy...@oracle.com wrote: Thank you Dan ! Please see my comments inline... On 7/24/13 5:12 AM, Dan Smith wrote: Hi. Per a request from Joel, I've taken a look at DefaultStaticTestData. I don't really have the full context here, but I'm

Re: RFR JDK-8011940 : java.lang.Class.getAnnotations() always enters synchronized method

2013-08-13 Thread Joel Borggrén-Franck
Hi all, Comments inline, On 12 aug 2013, at 15:19, Paul Sandoz paul.san...@oracle.com wrote: On Aug 12, 2013, at 2:40 PM, Peter Levart peter.lev...@gmail.com wrote: On 08/12/2013 12:54 PM, Joel Borggren-Franck wrote: I realize the interaction between probably any reflective operation and a

Re: RFR: 8022343: j.l.Class.getAnnotatedSuperclass() doesn't return null in some cases

2013-08-21 Thread Joel Borggrén-Franck
Hi Joe, Paul I rewrote the test in Paul's style without using testNG. http://cr.openjdk.java.net/~jfranck/8022343/webrev.01/ Please review. cheers /Joel On 2013-08-19, Joe Darcy wrote: Hi Joel, I agree the code looks fine. However, I concur with the general sentiment of Paul test

Re: RFR: 8022343: j.l.Class.getAnnotatedSuperclass() doesn't return null in some cases

2013-08-23 Thread Joel Borggrén-Franck
++; System.out.println(toTest + .getAnnotatedSuperclass() returns: + res + , was non-null); } } if (failed != 0) throw new RuntimeException(Test failed, check log for details); } -Joe On 8/21/2013 5:04 AM, Joel Borggrén-Franck

Re: RFR: 8022343: j.l.Class.getAnnotatedSuperclass() doesn't return null in some cases

2013-08-26 Thread Joel Borggrén-Franck
Joe, Paul, Vicente, Thanks for the reviews! cheers /Joel On 16 aug 2013, at 14:17, Joel Borggren-Franck joel.fra...@oracle.com wrote: Hi Please review this small fix for a type annotation reflection issue. The javadoc spec for Class.getAnnotatedSuperclass says: * If this Class

Re: RFR: 5047859 : (reflect) Class.getField can't find String[].length

2013-08-28 Thread Joel Borggrén-Franck
Hi Mandy, Thanks for your comments, On 2013-08-26, Mandy Chung wrote: Joel, The spec of the getFields and getDeclaredFields() methods both states this: This method returns an array of length 0 if the class or interface declares no fields, or if this|Class| object represents a

Re: RFR: 5047859 : (reflect) Class.getField can't find String[].length

2013-08-28 Thread Joel Borggrén-Franck
Hi David, On Aug 27, 2013, at 5:21 AM, David Holmes david.hol...@oracle.com wrote: Hi Joel, On 26/08/2013 10:39 PM, Joel Borggren-Franck wrote: Hi, Please review doc fix and test for http://bugs.sun.com/view_bug.do?bug_id=5047859

Re: RFR: 5047859 : (reflect) Class.getField can't find String[].length

2013-09-06 Thread Joel Borggrén-Franck
Thanks for the reviews everyone! cheers /Joel On Sep 4, 2013, at 3:25 PM, Joel Borggren-Franck joel.fra...@oracle.com wrote: Hi all, Thanks for the comments. New webrev here: http://cr.openjdk.java.net/~jfranck/5047859/webrev.03/ I have also created a specdiff to make it easier to

Re: RFR: 4987375: (reflect) Class.get{Declared}Method{s} does not return clone() for array type

2013-09-06 Thread Joel Borggrén-Franck
Hi Mandy, On Sep 5, 2013, at 9:34 PM, Mandy Chung mandy.ch...@oracle.com wrote: On 9/4/13 6:55 AM, Joel Borggren-Franck wrote: Hi, Please review fix for: http://bugs.sun.com/view_bug.do?bug_id=4987375 Webrev: http://cr.openjdk.java.net/~jfranck/4987375/webrev.01/ Specdiff:

Re: RFR JDK-8011940 : java.lang.Class.getAnnotations() always enters synchronized method

2013-09-09 Thread Joel Borggrén-Franck
and resembles declaration order) - behaviour of class annotation caching when class is redefined Regards, Peter On 08/13/2013 09:52 AM, Joel Borggrén-Franck wrote: Hi, Comments inline, On 12 aug 2013, at 14:40, Peter Levart peter.lev...@gmail.com wrote: On 08/12/2013 12:54 PM, Joel

RFR: 8009411 : getMethods should not inherit static methods from interfaces

2013-09-09 Thread Joel Borggrén-Franck
Hi Pleaser review fix for 8009411 : getMethods should not inherit static methods from interfaces The issue is that since we added static methods to interfaces those have erroneously been reflected in getMethods of implementing classes. This fix filters out static interface methods from

Re: RFR: 8009411 : getMethods should not inherit static methods from interfaces

2013-09-09 Thread Joel Borggrén-Franck
On 09/09/2013 07:00 PM, Joel Borggrén-Franck wrote: Hi Pleaser review fix for 8009411 : getMethods should not inherit static methods from interfaces The issue is that since we added static methods to interfaces those have erroneously been reflected in getMethods of implementing classes

Re: RFR: 8009411 : getMethods should not inherit static methods from interfaces

2013-09-09 Thread Joel Borggrén-Franck
On 9 sep 2013, at 19:00, Joel Borggrén-Franck joel.fra...@oracle.com wrote: The issue is that since we added static methods to interfaces those have erroneously been reflected in getMethods of implementing classes. This fix filters out static interface methods from superinterfaces when

Re: RFR: 4987375: (reflect) Class.get{Declared}Method{s} does not return clone() for array type

2013-09-11 Thread Joel Borggrén-Franck
Thanks everyone for the reviews and suggestions for improvements. cheers /Joel On Sep 4, 2013, at 3:55 PM, Joel Borggren-Franck joel.fra...@oracle.com wrote: Hi, Please review fix for: http://bugs.sun.com/view_bug.do?bug_id=4987375 Webrev:

Re: RFR: 8009411 : getMethods should not inherit static methods from interfaces

2013-09-12 Thread Joel Borggrén-Franck
review cheers /Joel On 2013-09-09, Joel Borggrén-Franck wrote: Hi Pleaser review fix for 8009411 : getMethods should not inherit static methods from interfaces The issue is that since we added static methods to interfaces those have erroneously been reflected in getMethods

Re: static vs. default interface methods and inheritance VM/javac issues

2013-09-13 Thread Joel Borggrén-Franck
Hi, Thanks Karen and Peter. FWIW my interpretation is that javac is doing the right thing. The bytecode generated is an invokeinterface DefaultStaticMethodTest$C.m(). This looks consistent with the other bytecodes generated for calls to methods of anonymous classes implementing an interface.

Re: RFR: 8009411 : getMethods should not inherit static methods from interfaces

2013-09-13 Thread Joel Borggrén-Franck
Hi Peter, Interesting case, thanks for the testing. On Sep 13, 2013, at 1:15 PM, Peter Levart peter.lev...@gmail.com wrote: On 09/13/2013 12:18 PM, Peter Levart wrote: The C.class.getMethods() returns a 1 element array containing A.m(), but C.class.getMethod(m) throws NoSuchMethodException.

Re: RFR: 8009411 : getMethods should not inherit static methods from interfaces

2013-09-13 Thread Joel Borggrén-Franck
On Sep 13, 2013, at 3:18 PM, Peter Levart peter.lev...@gmail.com wrote: On 09/13/2013 02:55 PM, Joel Borggrén-Franck wrote: Hi Peter, Interesting case, thanks for the testing. On Sep 13, 2013, at 1:15 PM, Peter Levart peter.lev...@gmail.com wrote: On 09/13/2013 12:18 PM, Peter Levart

Re: JDK-8020981: Update methods of java.lang.reflect.Parameter to throw correct exceptions

2013-09-13 Thread Joel Borggrén-Franck
Hi Eric, IIRC we don't check in classfiles into the repo. I'm not sure how we handle testing of broken class-files in jdk, but ASM might be an option, or storing the class file as an embedded byte array in the test. cheers /Joel On Sep 13, 2013, at 3:40 PM, Eric McCorkle

Re: JDK-8020981: Update methods of java.lang.reflect.Parameter to throw correct exceptions

2013-09-13 Thread Joel Borggrén-Franck
the test I've provided with the class files and see that it works. On 09/13/13 09:55, Joel Borggrén-Franck wrote: Hi Eric, IIRC we don't check in classfiles into the repo. I'm not sure how we handle testing of broken class-files in jdk, but ASM might be an option, or storing the class file

Re: RFR JDK-8011940 : java.lang.Class.getAnnotations() always enters synchronized method

2013-09-18 Thread Joel Borggrén-Franck
/09/2013 06:57 PM, Peter Levart wrote: Hi Joel, Thanks for reviewing. On 09/09/2013 04:25 PM, Joel Borggrén-Franck wrote: Hi Peter, Thanks for this, please add a @bug 8011940 tag to your test. IMO you don't need a re-review for that. Otherwise looks good. I'll do that. I just didn't

Re: Review request for JDK-8021398: j.l.r.Parameter.getAnnotatedType().getType() for not annotated use of type returns null

2013-10-01 Thread Joel Borggrén-Franck
Looks good. cheers /Joel On Oct 1, 2013, at 8:18 PM, Eric McCorkle eric.mccor...@oracle.com wrote: I forgot to hg add the test. I've addressed your and others' comments, and refreshed the webrev. Please review. On 10/01/13 04:06, Joel Borggren-Franck wrote: Hi Eric, Thanks for fixing

Re: RFR: 7044282: (reflect) Class.forName and Array.newInstance are inconsistent regarding multidimensional arrays

2013-10-11 Thread Joel Borggrén-Franck
Thanks for the reviews, fix has been pushed. cheers /Joel On 2013-10-10, Joe Darcy wrote: Looks fine; thanks, -Joe On 10/10/2013 5:56 AM, Paul Sandoz wrote: On Oct 10, 2013, at 2:46 PM, Joel Borggren-Franck joel.fra...@oracle.com wrote: Hi, Joe, Paul, agreed the test could be

Re: RFR: 8009411 : getMethods should not inherit static methods from interfaces

2013-10-21 Thread Joel Borggrén-Franck
in the search for methods in getMethod() , and are filtered out from getMethods() calls. Tests run: jdk_lang (including the updated testing with this commit) jdk_beans1, jdk_beans2, jdk_beans3 please review cheers /Joel On 26 sep 2013, at 22:53, Joel Borggrén-Franck joel.fra...@oracle.com wrote: Hi

Re: RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected

2013-10-22 Thread Joel Borggrén-Franck
Hi Andreas, A few nits: Class.java: import java.util.Collection; +import java.util.Collections; import java.util.HashSet; unused import. AnnotationSupport.java: +/** + * Equivalent to calling {@code getDirectlyAndIndirectlyPresentAnnotations( + * annotations, annoClass,

Re: RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected

2013-10-22 Thread Joel Borggrén-Franck
enough information to reconstruct a sound set of inherited and/or declared repeating annotations in all situations. But I'd like to 1st see the specification before showing you some examples where problems arise. Regards, Peter On 10/22/2013 12:21 PM, Joel Borggrén-Franck wrote: Hi

Re: RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected

2013-10-22 Thread Joel Borggrén-Franck
, but they should not, right? Class B3 should show the same annotations as B2 and class C3 should show the same annotations as class C2, I think. Regards, Peter On 10/22/2013 01:03 PM, Joel Borggrén-Franck wrote: Hi Peter, Spec is here: http://cr.openjdk.java.net/~abuckley

RFR: 8023651: j.l.r.Constructor.getAnnotatedReceiverType() and j.l.r.Constructor.getAnnotatedReturnType() for inner classes return incorrect result

2013-10-24 Thread Joel Borggrén-Franck
Hi, Please review this fix for reflection for type annotations. The problem was that the reflection side expected a different encoding of nested (static) inner classes. Also the type of the receiver parameter for a inner class constructor should be the outer this. I have added some test cases

Re: JDK 8 (initial) RFR for JDK-8005294 : Consider default methods for additions to AnnotatedElement

2013-10-24 Thread Joel Borggrén-Franck
Hi Peter, Joe, On 24 okt 2013, at 22:10, Peter Levart peter.lev...@gmail.com wrote: Hi Joe, I see two problems with the implementation in AnnotatedElementSupport. The first is the treatment of declared-only annotations where the code returns either directly present or in-directly present

Re: JDK 8 (initial) RFR for JDK-8005294 : Consider default methods for additions to AnnotatedElement

2013-10-24 Thread Joel Borggrén-Franck
Hi Joe, I think this a desirable change. Some design questions inline, Also please note the related work going on here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-October/022590.html On 24 okt 2013, at 15:31, Joe Darcy joe.da...@oracle.com wrote: Hello, Please review my

Re: RFR: 8027170: Annotations declared on super-super-class should be overridden by super-class

2013-10-24 Thread Joel Borggrén-Franck
Looks good Andreas. I can sponsor this. cheers /Joel On 24 okt 2013, at 17:18, Andreas Lundblad andreas.lundb...@oracle.com wrote: Hi, Please review the fix for JDK-8027170 below. Description: Class.getAnnotationsByType did not differentiate between annotations in superclass and

Re: JDK 8 RFR, redux, for JDK-8005294 : Consider default methods for additions to AnnotatedElement

2013-10-29 Thread Joel Borggrén-Franck
Hi Joe, Peter, On 29 okt 2013, at 07:09, Joe Darcy joe.da...@oracle.com wrote: Your comments, along with some spec refinements from the other OpenJDK list, are reflected in the next iteration of the webrev: http://cr.openjdk.java.net/~darcy/8005294.5/ Thanks, 259 if

Re: JDK 8 RFR, redux, for JDK-8005294 : Consider default methods for additions to AnnotatedElement

2013-10-30 Thread Joel Borggrén-Franck
Hi, On 30 okt 2013, at 18:15, Joe Darcy joe.da...@oracle.com wrote: On 10/30/2013 09:56 AM, Mandy Chung wrote: On 10/30/2013 9:41 AM, Joe Darcy wrote: Good suggestion; I've made the recommended change: http://cr.openjdk.java.net/~darcy/8005294.7/ I looked through the change and it

Re: Loading classes with many methods is very expensive

2014-10-23 Thread Joel Borggrén-Franck
Hi Martin, On 23 okt 2014, at 00:53, Martin Buchholz marti...@google.com wrote: Here at Google we have both kinds of scalability problems - loading classes from a classpath with 10,000 jars, and loading a single class file packed with the maximal number of methods. This message is about the

Re: Loading classes with many methods is very expensive

2014-10-23 Thread Joel Borggrén-Franck
On 23 Oct 2014, at 17:44, Peter Levart peter.lev...@gmail.com wrote: Is there a test that validates correctness of getMethods() or at least a set of interfaces and/or classes to exercise the algorithm and compare it to a different implementation? There is :)

Re: Loading classes with many methods is very expensive

2014-10-27 Thread Joel Borggrén-Franck
Hi Peter, As always, thanks for doing this! It has been on my todolist for a while but never quite bubbling up to the top. I don’t have time to look att his right now, but I expect to have some free time next week, but i have two short comments First, I have been thinking about moving

Re: Request for review/advice from langtools teamwas Re: Covariant overrides on the Buffer Hierarchy redux

2014-10-28 Thread Joel Borggrén-Franck
Hi Paul, Sorry for the delay. So if I understand this correctly, we get 4 warnings (and because of -Werror a build failure) in langtools when compiling vs Jdk 9, but need the casts because we bootstrap with Jdk 8. Looks good to me but I would prefer if you filed a bug on me for Jdk 10 for

Re: Loading classes with many methods is very expensive

2014-10-29 Thread Joel Borggrén-Franck
() algorithm stops searching for and consolidating any methods in (super)interfaces. Do you agree that this is a bug? Regards, Peter On 10/27/2014 02:45 PM, Joel Borggrén-Franck wrote: Hi Peter, As always, thanks for doing this! It has been on my todolist for a while but never quite

Re: inconsistency between Class.getMethod and Class.getMethods Was: Loading classes with many methods is very expensive

2014-10-29 Thread Joel Borggrén-Franck
Hi Peter, On 29 Oct 2014, at 18:16, Peter Levart peter.lev...@gmail.com wrote: On 10/29/2014 02:08 PM, Joel Borggrén-Franck wrote: Hi Peter, I’m not entirely convinced this is a bug. The lookup order for getMethod has for a long time been walk up superclasses and return what you find

Re: Review request for 8055063: Parameter#toString() fails w/ AIOOBE for ctr of inner class w/ generic type

2014-11-05 Thread Joel Borggrén-Franck
Hi Eric, I think caching the real parameter type array on Executable is the wrong thing to do considering we can have a lot of instances of Executable but fairly few will be examined for parameters and the vast majority of those will never have this issue. Also, the tests are a bit minimal, I

Re: RFR: 8062771: Core reflection should use final fields whenever possible

2014-11-06 Thread Joel Borggrén-Franck
Hi, I’m having a hard time following this thread, which webrev has been updated, for which release, fixing which issue? Martin, as far as I can see you are the only one of us who has replied to this thread who is a jdk7u committer (or reviewer). I think the assumption is that generic

Re: RFR: 8062771: Core reflection should use final fields whenever possible

2014-11-07 Thread Joel Borggrén-Franck
Hi Martin, Thanks for the clarification. On 6 nov 2014, at 20:51, Martin Buchholz marti...@google.com wrote: Hi Joel, On Thu, Nov 6, 2014 at 2:48 AM, Joel Borggrén-Franck joel.fra...@oracle.com wrote: Hi, I’m having a hard time following this thread, which webrev has been updated

Re: RFR: 8062771: Core reflection should use final fields whenever possible

2014-11-10 Thread Joel Borggrén-Franck
://bugs.openjdk.java.net/browse/JDK-8064391 I volunteer to be your reviewer for the backports. On Fri, Nov 7, 2014 at 1:36 PM, Joel Borggrén-Franck joel.fra...@oracle.com wrote: Hi Martin, Thanks for the clarification. On 6 nov 2014, at 20:51, Martin Buchholz marti...@google.com wrote

Re: Review request for 8055063: Parameter#toString() fails w/ AIOOBE for ctr of inner class w/ generic type

2014-11-10 Thread Joel Borggrén-Franck
the webrev. On 11/05/14 10:10, Joel Borggrén-Franck wrote: Hi Eric, I think caching the real parameter type array on Executable is the wrong thing to do considering we can have a lot of instances of Executable but fairly few will be examined for parameters and the vast majority

  1   2   >