Re: RFR: Optimize StringBuilder.append(null)

2013-03-28 Thread Martin Buchholz
On Wed, Mar 27, 2013 at 4:36 PM, Remi Forax fo...@univ-mlv.fr wrote: On 03/27/2013 03:01 AM, Martin Buchholz wrote: Hi Martin, the code looks good, I've just noticed that you declare the char array value final in appendNull but it's not something that is usually done in the java.lang

Re: RFR: Optimize StringBuilder.append(null)

2013-03-28 Thread Laurent Bourgès
Maybe the local var count could be renamed _count or c to avoid name conflict with the member count and make the code more readable / obvious. PS: I like final vars. Cheers Laurent Le 28 mars 2013 07:31, Martin Buchholz marti...@google.com a écrit : On Wed, Mar 27, 2013 at 4:36 PM, Remi Forax

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

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 mandy.ch...@oracle.com 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

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

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 HashMapInteger, Level 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 in

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMapInteger, Level 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

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMapInteger, Level 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

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

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

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 !

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

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMapInteger, Level 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

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

Re: 8010309 : PlatformLogger: isLoggable performance / waste due to HashMapInteger, Level 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

Re: hg: jdk8/tl/jdk: 8001642: Add OptionalT, 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

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

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

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

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

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 value

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 !

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 mandy.ch...@oracle.com 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 mandy.ch...@oracle.com wrote:

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.

Re: RFR : JDK-8001642 : Add OptionalT, 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 fo...@univ-mlv.fr wrote: Google's Guava, which is a popular library, defines a class named Optional, but allow to store null unlike the current proposed

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

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*

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

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: http://cr.openjdk.java.net/~dfuchs/JDK-8010495/jdk8/webrev.01/ 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

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:

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 philip.r...@oracle.com mailto:philip.r...@oracle.com wrote: Maintaining a pool of objects might be an appropriate thing for

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

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 Thu,

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 the

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:

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 mandy.ch...@oracle.com 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

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 marti...@google.comwrote: strsep is nice, but not standard

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 vita...@gmail.comwrote: Enum and