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

2013-03-28 Thread Joe Darcy
Hello, Please review these changes to add support for the "default" modifier to the output of Method.to[Generic]String: 8004979 java.lang.reflect.Modifier.toString should include "default" http://cr.openjdk.java.net/~darcy/8004979.0/ Patch also included below. Thanks, -Joe diff -r

Re: RFR JDK-8010267 & JDK-8010268 : Makefile maintenance for test targets

2013-03-28 Thread Erik Joelsson
Both of these look good to me, but you still need a jdk8 reviewer. /Erik On 2013-03-27 17:27, Mike Duigou wrote: I still need a review for both of these changes. Mike On Mar 18 2013, at 22:48 , Mike Duigou wrote: A two small changes to review: If approved I will commit to TL (or someone el

Re: Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK

2013-03-28 Thread John Rose
On Mar 27, 2013, at 10:35 AM, Mandy Chung wrote: > 1. I am working on a fix for 8007035 that proposes to deprecate > SecurityManager.checkMemberAccess method as it requires the caller’s frame to > be at a stack depth of four, which is fragile and difficult to enforce. Where you test c=smgr.get

Re: RFR (M): 7198429: need checked categorization of caller-sensitive methods in the JDK

2013-03-28 Thread Peter Levart
On 03/28/2013 04:34 AM, Mandy Chung wrote: Hi Peter, On 3/21/2013 11:47 AM, Peter Levart wrote: ... I have seen a utility that uses it to establish the context (package and ClassLoader) of where to start searching for resources for GUI components construction. And a utility that wraps Class

hg: jdk8/tl/jdk: 8010991: Enable test/javax/script/GetInterfaceTest.java again

2013-03-28 Thread sundararajan . athijegannathan
Changeset: 811c771acf65 Author:sundar Date: 2013-03-28 14:36 +0530 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/811c771acf65 8010991: Enable test/javax/script/GetInterfaceTest.java again Reviewed-by: lagergren, hannesw ! test/javax/script/GetInterfaceTest.java

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap leads to Integer allocations (boxing)

2013-03-28 Thread Laurent Bourgès
Dear Mandy & Peter, I will test your patch asap (+ benchmark) Few comments below: > Webrev at: >http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8010309/webrev.00/ > > Please let me know if you don't agree with these changes or you find any > performance issue. > > I have retained the part i

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap leads to Integer allocations (boxing)

2013-03-28 Thread Laurent Bourgès
Sorry for my previous comment: JavaLoggerProxy static initialization is not called (false argument); I made the test: JavaLoggerProxy: static initialization... LoggingSupport.isAvailable: true java.lang.Throwable at sun.util.logging.PlatformLogger$JavaLoggerProxy.(PlatformLogger.java:562)

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap leads to Integer allocations (boxing)

2013-03-28 Thread Peter Levart
On 03/28/2013 04:02 AM, Mandy Chung wrote: Hi Peter, It looks better. I want to wrap up this patch and push the changeset tomorrow. I have made slight modification and clean up some comments and the test. See my comments inlined below. Webrev at: http://cr.openjdk.java.net/~mchung/jdk8/

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap leads to Integer allocations (boxing)

2013-03-28 Thread Peter Levart
On 03/28/2013 10:19 AM, Laurent Bourgès wrote: the following method in JavaLoggerProxy also caught my eye: void doLog(Level level, String msg, Object... params) { if (!isLoggable(level)) { return; } // only pas

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

2013-03-28 Thread Jeroen Frijters
Hi Joe, I notice that Method.isDefault() returns true for static interface methods. I assume that is not correct. Regards, Jeroen > -Original Message- > From: core-libs-dev-boun...@openjdk.java.net [mailto:core-libs-dev- > boun...@openjdk.java.net] On Behalf Of Joe Darcy > Sent: Thursda

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 wrote: > Hi Joe, > > I notice that Method.isDefault() returns true for static interface methods. I > assume that is

Re: RFR JDK-8003245

2013-03-28 Thread Florian Weimer
On 03/27/2013 05:30 PM, John Zavgren wrote: Yes, the uninitialized memory will be accessed in some cases, for example: Okay, in this case, the changes look okay to me (although returning structures by value is somewhat unusual, but that's obviously a pre-existing issue). -- Florian Weimer /

hg: jdk8/tl/langtools: 2 new changesets

2013-03-28 Thread maurizio . cimadamore
Changeset: 7bebe17ff323 Author:mcimadamore Date: 2013-03-28 11:38 + URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/7bebe17ff323 8010469: Bad assertion in LambdaToMethod Summary: Add assertion in LambdaToMethod.serializedLambdaName Reviewed-by: jjg ! src/share/classes/com

Re: RFR JDK-7143928 : (coll) Optimize for Empty ArrayList and HashMap

2013-03-28 Thread Doug Lea
On 03/27/13 12:17, Martin Buchholz wrote: But you can support any requested initial size if stored in the size field when list is empty. In other words: Mike, please do not add a field if your goal is to save space! Also, I hope you or the performance testing folks have extensive and accurate e

Re: RFR JDK-7143928 : (coll) Optimize for Empty ArrayList and HashMap

2013-03-28 Thread Peter Levart
Hi Mike, Have you considered the possibility to re-constitute the initial capacity from threshold and loadFactor? Regards, Peter On 03/27/2013 05:28 PM, Mike Duigou wrote: I started looking at crafty reuse of size but found too many direct references to size to attempt getting this right in

hg: jdk8/tl/jdk: 8010125: keytool -importkeystore could create a pkcs12 keystore with different storepass and keypass

2013-03-28 Thread weijun . wang
Changeset: a87fac00915e Author:weijun Date: 2013-03-28 20:27 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a87fac00915e 8010125: keytool -importkeystore could create a pkcs12 keystore with different storepass and keypass Reviewed-by: vinnie ! src/share/classes/sun/security

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap leads to Integer allocations (boxing)

2013-03-28 Thread Laurent Bourgès
My benchmaks are also OK: run: -XX:+AggressiveOpts -XX:ClassMetaspaceSize=104857600 -XX:InitialHeapSize=268435456 -XX:MaxHeapSize=268435456 -XX:+PrintCommandLineFlags -XX:-PrintFlagsFinal -XX:+UseCompressedKlassPointers -XX:+UseCompressedOops -XX:+UseParallelGC mars 28, 2013 1:53:57 PM test.Platfor

Re: RFR JDK-8010267 & JDK-8010268 : Makefile maintenance for test targets

2013-03-28 Thread Tim Bell
Mike - Looks good to me. Tim On 03/28/13 01:11, Erik Joelsson wrote: Both of these look good to me, but you still need a jdk8 reviewer. /Erik On 2013-03-27 17:27, Mike Duigou wrote: I still need a review for both of these changes. Mike On Mar 18 2013, at 22:48 , Mike Duigou wrote: A two

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMap leads to Integer allocations (boxing)

2013-03-28 Thread Mandy Chung
Hi Peter, Laurent, Thanks for the performance run. I'll push the changeset once I check the test result. On 3/28/2013 2:52 AM, Peter Levart wrote: ...just a nit! In the test: private static void checkPlatformLoggerLevelMapping(Level level) { // map the given level to PlatformLo

Re: hg: jdk8/tl/jdk: 8001642: Add Optional, OptionalDouble, OptionalInt, OptionalLong

2013-03-28 Thread Brian Goetz
> Has the optional classes been verified to serialize/deserialize correctly? > They are not serializable. > Finally, are these utilities critical to some other part JDK 8 that they were > pushed out now as opposed to JDK 9? > > They are part of the libraries being added by JSR-335 / Project L

RFR JDK-8010096 : Initial java.util.Spliterator putback

2013-03-28 Thread Paul Sandoz
Hi, http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8010096 Webrev: http://cr.openjdk.java.net/~psandoz/lambda/spliterator/jdk-8010096/webrev/ Spec diff: http://cr.openjdk.java.net/~psandoz/lambda/spliterator/jdk-8010096/specdiff/overview-summary.html Relevant JavaDoc generated from lambda

Re: RFR: Optimize StringBuilder.append(null)

2013-03-28 Thread Martin Buchholz
I took Laurent's suggestion of renaming count to c, even though the result looks like a value judgement on a competing programming environment: value[c++] = 'n'; value[c++] = 'u'; value[c++] = 'l'; value[c++] = 'l'; On Wed, Mar 27, 2013 at 11:42 PM, Laurent Bourg

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

2013-03-28 Thread Joe Darcy
Hi Jeroen and Joel, Static methods on interfaces and *not* default methods and that distinction is made my the changeset Joel references. The changeset is not in the master repository yet, but should be there soon. Thanks, -Joe On 03/28/2013 03:55 AM, Joel Borggrén-Franck wrote: Hi Jeroen,

Re: RFR JDK-7143928 : (coll) Optimize for Empty ArrayList and HashMap

2013-03-28 Thread Mike Duigou
We have heard back from the performance folks that 85% of empty lists are created at default size. The proposed patch is going to be revised to do the inflation trick only for default sized maps which will eliminate the need for a new field. Something creative involving the use of the existing s

Re: RFR: Optimize StringBuilder.append(null)

2013-03-28 Thread Jim Gish
Thanks, Martin, We've all heard of code "smells" -- I hereby dub this a code "chuckle" :-) The change looks good to me otherwise as well. Thanks, Jim On 03/28/2013 12:05 PM, Martin Buchholz wrote: I took Laurent's suggestion of renaming count to c, even though the result looks like a valu

hg: jdk8/tl/langtools: 8006346: doclint should make allowance for headers generated by standard doclet

2013-03-28 Thread jonathan . gibbons
Changeset: 991f11e13598 Author:jjg Date: 2013-03-28 10:49 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/991f11e13598 8006346: doclint should make allowance for headers generated by standard doclet Reviewed-by: mcimadamore ! src/share/classes/com/sun/tools/doclint/Chec

Re: Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK

2013-03-28 Thread Christian Thalinger
On Mar 27, 2013, at 8:01 PM, Mandy Chung wrote: > Thanks for the review. I forgot to mention that Chris contributed the > initial patch (thanks). > > On 3/27/2013 1:13 PM, Christian Thalinger wrote: >> On Mar 27, 2013, at 10:35 AM, Mandy Chung wrote: >> >>> This is the JDK change for JEP 17

Re: RFR-8008118

2013-03-28 Thread Martin Buchholz
My apologies for unrelenting perfectionism - I noticed that we can: - colocate the pathv and path in one malloc'ed chunk - can discard xstrdup - can iterate through path with only one char* pointer. Making this code even cleaner, more concise and efficient. http://cr.openjdk.java.net/~martin/web

Re: RFR : JDK-8001642 : Add Optional, OptionalDouble, OptionalInt, OptionalLong

2013-03-28 Thread Kevin Bourrillion
I do NOT wish to restart this discussion; I just noticed a falsehood that was never exposed: On Wed, Mar 6, 2013 at 1:47 AM, Remi Forax wrote: Google's Guava, which is a popular library, defines a class named Optional, > but allow to store null unlike the current proposed implementation, this >

Re: Spliterator flags as enum (was Initial java.util.Spliterator putback)

2013-03-28 Thread Doug Lea
On 03/28/13 13:18, Tim Peierls wrote: I can't find a discussion of why Spliterator flags are ints rather than enum. We started out with enums on (my) initial Spliterator side vs control flags internal to streams. The we had to somehow mesh these to work together. On the stream side, you need to

Re: Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK

2013-03-28 Thread Lance Andersen - Oracle
Hi Mandy the DriverManager change looks fine. Best Lance On Mar 27, 2013, at 1:35 PM, Mandy Chung wrote: > This is the JDK change for JEP 176: JEP 176: Mechanical Checking of > Caller-Sensitive Methods [1]. Christian has posted the webrev for the > hotspot VM change a couple weeks ago [2]. >

Re: Spliterator flags as enum (was Initial java.util.Spliterator putback)

2013-03-28 Thread Doug Lea
On 03/28/13 14:52, Joshua Bloch wrote: Doug, I don't get it. You can set and unset flags on your own EnumSet. Why isn't that sufficient? There are a lot of problems. First, even though most spliterators will return the same set of characteristics each time, you can't just create one static on

Re: RFR-8008118

2013-03-28 Thread Christos Zoulas
On Mar 28, 11:12am, marti...@google.com (Martin Buchholz) wrote: -- Subject: Re: RFR-8008118 | My apologies for unrelenting perfectionism - I noticed that we can: | - colocate the pathv and path in one malloc'ed chunk | - can discard xstrdup | - can iterate through path with only one char* pointer

Re: [OpenJDK 2D-Dev] sun.java2D.pisces big memory usage (waste ?)

2013-03-28 Thread Phil Race
Maintaining a pool of objects might be an appropriate thing for an applications, but its a lot trickier for the platform as the application's usage pattern or intent is largely unknown. Weak references or soft references might be of use but weak references usually go away even at the next increm

Re: JDK-8010495: Update JAXP NetBeans project - add support for generating javadoc

2013-03-28 Thread Daniel Fuchs
Hi, Please find below a revised patch: I had oversimplified the changes in project.xml. It seems you need to declare a source folder of type 'java' as well as a compilation-unit in order for 'Find Usage' to work properly in the ed

hg: jdk8/tl/jdk: 8010309: Improve PlatformLogger.isLoggable performance by direct mapping from an integer to Level

2013-03-28 Thread mandy . chung
Changeset: e433ed08b733 Author:mchung Date: 2013-03-28 13:14 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e433ed08b733 8010309: Improve PlatformLogger.isLoggable performance by direct mapping from an integer to Level Reviewed-by: mchung Contributed-by: peter.lev...@gmail.c

Re: [OpenJDK 2D-Dev] sun.java2D.pisces big memory usage (waste ?)

2013-03-28 Thread Phil Race
B'ccing discuss and core-libs as I realise this is mostly spam for those lists. On 3/28/2013 1:40 PM, Andrea Aime wrote: On Thu, Mar 28, 2013 at 8:38 PM, Phil Race > wrote: Maintaining a pool of objects might be an appropriate thing for an applications,

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

2013-03-28 Thread Mike Duigou
Technically the changes look fine but I am a little concerned by the position of default in the order placement. The emerging practice is to put default first immediately following the access modifier. Counter to this is the correct preference for using the "blessed modifier order". Can we move

Re: Spliterator flags as enum (was Initial java.util.Spliterator putback)

2013-03-28 Thread Vitaly Davidovich
Enum and EnumSet *are* good, but the problem is the overhead of representation. If all you want is 32 bits to play with, you pay exactly 4 bytes in price if using int. With EnumSet, you pay object/heap overhead - on x64, 4 bytes vs 24; you pay GC price if you have lots of objects with embedded En

Re: RFR-8008118

2013-03-28 Thread Martin Buchholz
strsep is nice, but not standard enough to use in the JDK. OTOH, strcspn is already used in the JDK, but doesn't really buy much: for (i = 0; i < count; i++) { int len = strcspn(p, ":"); pathv[i] = (len == 0) ? "." : p; p[len] = '\0'; p += len + 1; } On

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

2013-03-28 Thread Joe Darcy
Hi Mike, Sure; I've checked with the latest lambda draft and changed the code to output any default modifier after any public / private / protected modifiers. The main change is in the printModifiersIfNonzero method: void printModifiersIfNonzero(StringBuilder sb, int mask, boolean isDefa

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

2013-03-28 Thread Mike Duigou
Thanks for making the change. It looks good to me. Mike On Mar 28 2013, at 17:59 , Joe Darcy wrote: > Hi Mike, > > Sure; I've checked with the latest lambda draft and changed the code to > output any default modifier after any public / private / protected modifiers. > The main change is in th

Review Request 8007035: Deprecate SecurityManager.checkMemberAccess

2013-03-28 Thread Mandy Chung
Sean, John, Joe, Can you review this fix todeprecatesthe |SecurityManager.checkMemberAccess| method as proposed in http://openjdk.java.net/jeps/176? Webrev at: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8007035/webrev.00 Specdiff: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8007035/spe

Re: Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK

2013-03-28 Thread Mandy Chung
On 3/28/2013 1:54 AM, John Rose wrote: On Mar 27, 2013, at 10:35 AM, Mandy Chung wrote: 1. I am working on a fix for 8007035 that proposes to deprecate SecurityManager.checkMemberAccess method as it requires the caller’s frame to be at a stack depth of four, which is fragile and difficult t

Re: RFR-8008118

2013-03-28 Thread Martin Buchholz
Latest webrev does it this way: for (i = 0; i < count; i++) { char *q = p + strcspn(p, ":"); pathv[i] = (p == q) ? "." : p; *q = '\0'; p = q + 1; } On Thu, Mar 28, 2013 at 5:07 PM, Martin Buchholz wrote: > strsep is nice, but not standard enough to use i

Re: Spliterator flags as enum (was Initial java.util.Spliterator putback)

2013-03-28 Thread Paul Benedict
I think the use of EnumSet in a public API is superior to bit flags. Worrying about the number of bytes here is not important since they will all end up being garbage collected when the stream processing ends. On Thu, Mar 28, 2013 at 6:56 PM, Vitaly Davidovich wrote: > Enum and EnumSet *are* goo