Re: JDK 8 code review request for 8011800: Add java.util.Objects.requireNonNull(T, Supplier)

Hi Joe, This is similar to recent addition to j.u.l.Logger, which is ok. No matter how lambda construction is optimized, if it captures any non-constant arguments, new object will always be constructed (unless perhaps JIT could optimize this away using escape analysis). Myself I often find th

Re: [OpenJDK 2D-Dev] AAShapePipe concurrency & memory waste

Andrea, I am running benchmarks on my laptop (i7 - 2 core 2.8Ghz + HT => 4 virtual cpus) on linux 64 (fedora 14). Note: I always use cpufrequtils to set the cpu governor to performance and use fixed frequency = 2.8Ghz: [bourgesl@jmmc-laurent ~]$ uname -a Linux jmmc-laurent.obs.ujf-grenoble.fr 2.6.3

Re: sun.awt.X11 logs still using String + (waste)

Hi, Laurent. The changes in awt/swing area looks good. On 4/8/13 3:32 PM, Laurent Bourgès wrote: Anthony, here is an updated patch concerning both PlatformLogger and Logger 'bad' usages: http://jmmc.fr/~bourgesl/share/webrev-8010297.3/ It impacts now awt, swing, JMX, security, network, and apa

Re: RFR: 8004518 & 8010122 : Default methods on Map

Hi all, when I see all the extensions on interfaces via the new default construct, I still have the feeling, such entities should be seen and named as "normal" abstract classes. This would additionally allow protected and private members, which otherwise can be a cumbersome restriction. To be c

Re: [OpenJDK 2D-Dev] AAShapePipe concurrency & memory waste

Dear Jim, 2013/4/9 Jim Graham > > The allocations will always show up on a heap profiler, I don't know of > any way of having them not show up if they are stack allocated, but I don't > think that stack allocation is the issue here - small allocations come out > of a fast generation that costs a

Re: sun.awt.X11 logs still using String + (waste)

Anthony or Mandy, Could you ask JMX / Security groups for me to review my patch ? I am currently not registered to these mailing lists. Do you ask me to split the patch in two part: PlatformLogger on one side and Logger on the other side ? Laurent 2013/4/9 Mandy Chung > On 4/9/13 12:37 AM, L

Re: RFR: 8004518 & 8010122 : Default methods on Map

Hi again, please consider to add hash() to interface Map, as an additional performance opportunity, see: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6812862 It was rejected with: implement a wrapper class for the "key" to customize its hash() & equals() methods Unfortunately my earlier

Re: [OpenJDK 2D-Dev] AAShapePipe concurrency & memory waste

Hi Laurent, Could you disable tiered compilation for performance tests? Tiered compilation is usually a source of jitter in the results. Pass -XX:-TieredCompilation to the VM. Regards, Peter On 04/10/2013 10:58 AM, Laurent Bourgès wrote: Dear Jim, 2013/4/9 Jim Graham

Re: sun.awt.X11 logs still using String + (waste)

Laurent, I'm not subscribed to those mailing list, too. So you could send/forward your review request to the lists yourself - no difference here. Note that I tried sending your message to net-dev@ in the past, and even contacted the maintainer of the mailing list via a private email, but I neve

Re: [OpenJDK 2D-Dev] AAShapePipe concurrency & memory waste

Peter, 1/ I modified your TestRunner class to print total ops and perform explicit GC before runTests: http://jmmc.fr/~bourgesl/share/AAShapePipe/microbench/ 2/ I applied your advice but it does not change much: -XX:ClassMetaspaceSize=104857600 -XX:InitialHeapSize=134217728 -XX:MaxH

Re: RFR: 8004518 & 8010122 : Default methods on Map

Ulf, The discussions you refer to have been happening over a number of years. We are way past that point now. The key point is that default methods do not introduce multiple-inheritance of state, which is where the MI problems lie, and why we would not want to add MI and use abstract classes

hg: jdk8/tl/langtools: 8011432: javac, compiler regression iterable + captured type

Changeset: a4be2c2fe0a1 Author:vromero Date: 2013-04-10 12:31 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/a4be2c2fe0a1 8011432: javac, compiler regression iterable + captured type Reviewed-by: mcimadamore ! src/share/classes/com/sun/tools/javac/comp/Lower.java ! tes

Re: JDK 8 code review request for 8011800: Add java.util.Objects.requireNonNull(T, Supplier)

On 09/04/2013 22:12, Joe Darcy wrote: Hello, Please review my changes for 8011800: Add java.util.Objects.requireNonNull(T, Supplier) http://cr.openjdk.java.net/~darcy/8011800.0/ which add a new method to java.util.Objects to take a Supplier rather than a String. Patch inline below.

Re: JDK 8 code review request for 8011800: Add java.util.Objects.requireNonNull(T, Supplier)

On 04/09/2013 11:12 PM, Joe Darcy wrote: Hello, Please review my changes for 8011800: Add java.util.Objects.requireNonNull(T, Supplier) http://cr.openjdk.java.net/~darcy/8011800.0/ which add a new method to java.util.Objects to take a Supplier rather than a String. Patch inline belo

Re: Proxy.isProxyClass scalability

Hi Alan, I have prepared new webrev of the patch rebased to current tip of jdk8/tl repo: https://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy/webrev.04/index.html I noticed there were quite a few changes to the j.l.r.Proxy in the meanwhile regarding new security checks. But @CallerSen

RFR JDK-8011426: java.util collection Spliterator implementations

Hi, Following up from JDK-8010096 [1] here is a webrev for spliterator implementations of collection classes in java.util: http://cr.openjdk.java.net/~psandoz/lambda/spliterator/jdk-8011426/webrev/ This is dependent on [1]. -- Note that for some reason the webrev script creates the jdk chan

RFR JDK-8011427: java.util.concurrent collection Spliterator implementations

Hi, Following up from JDK-8010096 [1] here is a webrev for spliterator implementations of collection classes in java.util.concurrent. More precisely it represents updates from JSR 166 for collection classes that implement spliterators: http://cr.openjdk.java.net/~psandoz/lambda/spliterator/j

Re: RFR JDK-8011427: java.util.concurrent collection Spliterator implementations

On 04/10/2013 03:56 PM, Paul Sandoz wrote: Hi, Following up from JDK-8010096 [1] here is a webrev for spliterator implementations of collection classes in java.util.concurrent. More precisely it represents updates from JSR 166 for collection classes that implement spliterators: http://cr.

Re: Proxy.isProxyClass scalability

On 4/10/2013 5:35 AM, Peter Levart wrote: Hi Alan, I have prepared new webrev of the patch rebased to current tip of jdk8/tl repo: https://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy/webrev.04/index.html I noticed there were quite a few changes to the j.l.r.Proxy in the meanwhil

Re: RFR: 8004518 & 8010122 : Default methods on Map

Thanks David, Am 10.04.2013 13:32, schrieb David Holmes: Ulf, The discussions you refer to have been happening over a number of years. We are way past that point now. Yes, it's a pity, that I haven't followed those discussions early enough. Theoretically, Java 8 is not final now, but I unde

Re: RFR-8008118

Greetings: There is a new webrev image available for your inspection that fixes all identified bugs: http://cr.openjdk.java.net/~jzavgren/8008118/webrev.07/ Your comments are welcome. Thanks! John On 03/29/2013 09:59 AM, chris...@z

RFR: 8010279 - Comparators should throw NPE on null argument

Hi, The bug is reported on lambda repo, part of the fix involved code already integrated into TL/master, thus need help on review and commit. Webrev at: http://cr.openjdk.java.net/~henryjen/tl/8010279.0/webrev/ Simple changes in the code, with a little restructure of jtreg test to fit convention

Re: RFR: 8010279 - Comparators should throw NPE on null argument

On 10/04/2013 19:00, Henry Jen wrote: Hi, The bug is reported on lambda repo, part of the fix involved code already integrated into TL/master, thus need help on review and commit. Webrev at: http://cr.openjdk.java.net/~henryjen/tl/8010279.0/webrev/ Simple changes in the code, with a little res

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

On Apr 9 2013, at 19:56 , Martin Buchholz wrote: > Mike, thanks. > > I don't see anything wrong in this version, although the ongoing > complexification and special-case-ification (with attendant risk of bugs) of > ArrayList and HashMap, the two most didactically important classes, continue >

Re: RFR: 8010279 - Comparators should throw NPE on null argument

Looks OK to me. Mike On Apr 10 2013, at 11:00 , Henry Jen wrote: > Hi, > > The bug is reported on lambda repo, part of the fix involved code > already integrated into TL/master, thus need help on review and commit. > > Webrev at: http://cr.openjdk.java.net/~henryjen/tl/8010279.0/webrev/ > > S

Re: RFR-8008118

I am happy with the latest webrev - ship it! Thanks, John! On Wed, Apr 10, 2013 at 10:54 AM, John Zavgren wrote: > Greetings: > > There is a new webrev image available for your inspection that fixes all > identified bugs: > http://cr.openjdk.java.net/~jzavgren/8008118/webrev.07/ > > Your comme

Re: RFR: 8004518 & 8010122 : Default methods on Map

On 04/10/2013 07:19 PM, Ulf Zibis wrote: Thanks David, Am 10.04.2013 13:32, schrieb David Holmes: Ulf, The discussions you refer to have been happening over a number of years. We are way past that point now. Yes, it's a pity, that I haven't followed those discussions early enough. Theoreti

Re: RFR: 8010279 - Comparators should throw NPE on null argument

On 04/10/2013 08:20 PM, Mike Duigou wrote: Looks OK to me. Mike Ok for me too. Rémi On Apr 10 2013, at 11:00 , Henry Jen wrote: Hi, The bug is reported on lambda repo, part of the fix involved code already integrated into TL/master, thus need help on review and commit. Webrev at: http:

Re: [OpenJDK 2D-Dev] AAShapePipe concurrency & memory waste

On Wed, Apr 10, 2013 at 10:14 AM, Laurent Bourgès wrote: > Andrea, > I am running benchmarks on my laptop (i7 - 2 core 2.8Ghz + HT => 4 virtual > cpus) on linux 64 (fedora 14). > Note: I always use cpufrequtils to set the cpu governor to performance and > use fixed frequency = 2.8Ghz: > [bourgesl

Re: RFR-8008118

On Apr 10, 1:54pm, john.zavg...@oracle.com (John Zavgren) wrote: -- Subject: Re: RFR-8008118 | Your comments are welcome. 1. We did we switch from NEW() to xmalloc()? Why is the xmalloc cast needed? 2. I would not declare pathv "const char **", but "char **", and then cast the return if neede

Re: RFR-8008118

On Wed, Apr 10, 2013 at 11:24 AM, Christos Zoulas wrote: > On Apr 10, 1:54pm, john.zavg...@oracle.com (John Zavgren) wrote: > -- Subject: Re: RFR-8008118 > > | Your comments are welcome. > > 1. We did we switch from NEW() to xmalloc()? Why is the xmalloc cast > needed? > NEW is for allocating ho

Re: [OpenJDK 2D-Dev] AAShapePipe concurrency & memory waste

Hi, Laurent. I am not an expert here but just my 50 cents. This optimization shall take place only if it is really hotspot. But if it is a really hotspot - probably it would be better to remove these array/object allocation at all and use plane bytes? I see that some methods which take it as arg

Re: RFR: 8004518 & 8010122 : Default methods on Map

Am 10.04.2013 20:24, schrieb Remi Forax: interface + default methods are conceptually what is known as traits(*), you can see them as interface + method with code or as abstract class without state, it's the same thing. Thanks, that's what I wanted to make sure. Now, if you want traits in Ja

Re: [OpenJDK 2D-Dev] AAShapePipe concurrency & memory waste

Sergey, I am not an expert here but just my 50 cents. > This optimization shall take place only if it is really hotspot. But if it > is a really hotspot - probably it would be better to remove these > array/object allocation at all and use plane bytes? > Java2D calls AAShapePipe for each shape (l

Re: RFR-8008118

On Apr 10, 11:54am, marti...@google.com (Martin Buchholz) wrote: -- Subject: Re: RFR-8008118 | > 1. We did we switch from NEW() to xmalloc()? Why is the xmalloc cast | > needed? | | NEW is for allocating homogeneous arrays, but here the memory block is | being used for both chars and pointers. I

Re: RFR-8008118

Christos, I think you may have overlooked the subtlety that pathv is *not* a const pointer - it just looks like one. cat cast.c && echo --- && gcc -Wall -Wcast-qual cast.c #include #include int main(int argc, char *argv[]) { const char **p = (const char **) malloc(5); char *q = (char *

Re: JDK 8 code review request for 8011800: Add java.util.Objects.requireNonNull(T, Supplier)

Hello, On 04/10/2013 05:01 AM, Alan Bateman wrote: On 09/04/2013 22:12, Joe Darcy wrote: Hello, Please review my changes for 8011800: Add java.util.Objects.requireNonNull(T, Supplier) http://cr.openjdk.java.net/~darcy/8011800.0/ which add a new method to java.util.Objects to take a

Re: RFR : 8010948 : Add conversion functional interfaces

I have added two @see to every interface, the reverse function and the function converting from the other primitive type to the same type. ie. in IntToDoubleFunction I add @see for LongToDoubleFunction and DoubleToIntFunction. Mike On Apr 9 2013, at 16:14 , Paul Benedict wrote: > Mike, > > It

Re: RFR-8008118

On Apr 10, 1:44pm, marti...@google.com (Martin Buchholz) wrote: -- Subject: Re: RFR-8008118 | Christos, I think you may have overlooked the subtlety that pathv is *not* | a const pointer - it just looks like one. | Yes, indeed; you are right! christos

Re: Incorrect arguments is passed to sun.misc.Perf#createLong

Hi Yasumasa, I'm adding core-libs and bcc serviceability-dev to move this thread to core-libs for sun.misc.PerfCounter discussion. On 4/9/2013 4:50 AM, Yasu wrote: Hi, I'm trying to create entry from Java program to hsperfdata through sun.misc.PerfCounter . First of all, sun.misc.PerfCou

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

I'm willing to accept John as an authority on hotspot optimization. I'm surprised that null checks aren't more close to free in part because recent jsr166 code has been introducing more explicit null checks, sometimes in code where the reference being checked is "known" not to be null. Martin On

hg: jdk8/tl/jdk: 8010948: Add conversion functional interfaces

Changeset: 863da64214e8 Author:mduigou Date: 2013-04-10 14:06 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/863da64214e8 8010948: Add conversion functional interfaces Reviewed-by: mduigou, dholmes Contributed-by: Brian Goetz + src/share/classes/java/util/function/DoubleToI

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

On 04/10/13 17:26, Martin Buchholz wrote: I'm willing to accept John as an authority on hotspot optimization. I'm surprised that null checks aren't more close to free in part because recent jsr166 code has been introducing more explicit null checks, sometimes in code where the reference being che

Re: [OpenJDK 2D-Dev] AAShapePipe concurrency & memory waste

On 4/10/13 11:46 PM, Laurent Bourgès wrote: I see that some methods which take it as argument doesn't use them. And most of the time we pass AATileGenerator and abox[] to the same methods, so it could be merged? For now I did not want to modify the AAShapePipe signatures: abox[] is filled by AA

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

On 04/10/2013 11:26 PM, Martin Buchholz wrote: I'm willing to accept John as an authority on hotspot optimization. I'm surprised that null checks aren't more close to free in part because recent jsr166 code has been introducing more explicit null checks, sometimes in code where the reference bein

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

The null check jumps should be taken care of by branch prediction; as long as they're predictable, penalty on OOO CPU is minimal. So modern CPUs don't like mispredicted branches I'd say, not just any jump. On Apr 10, 2013 7:23 PM, "Remi Forax" wrote: > On 04/10/2013 11:26 PM, Martin Buchholz wro

hg: jdk8/tl/jdk: 8011930: Long.parseLong(String, int) has inaccurate limit on radix for using 'L'

Changeset: b0458dd21ba6 Author:darcy Date: 2013-04-10 16:38 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b0458dd21ba6 8011930: Long.parseLong(String, int) has inaccurate limit on radix for using 'L' Reviewed-by: smarks ! src/share/classes/java/lang/Long.java

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

Whether or not we use null or an empty array as a sentinel value, we will need to make a special check for empty on most operations, and that will not be predictable if empty collection operations are common. The empty collection optimization is a strange sort of compression algorithm.

Re: [OpenJDK 2D-Dev] AAShapePipe concurrency & memory waste

I'm pretty familiar with all of this code and there aren't any places that save the tile array that I remember. The embedded code that Pisces was taken from had some caching of alpha arrays, but we didn't use or keep that when we converted it for use in the JDK... It occurs to me that since y

Add getChars to CharSequence

I've often wished that CharSequence had getChars methods, as many of the concrete implementations already do. In jdk8 with default methods, this is possible! This will make some of the String code a little nicer and more efficient. Here's a preliminary patch in this direction, that overlaps with s

Re: Add getChars to CharSequence

Seems like a nice extension to me. Mike On Apr 10 2013, at 17:40 , Martin Buchholz wrote: > I've often wished that CharSequence had getChars methods, as many of the > concrete implementations already do. > In jdk8 with default methods, this is possible! > This will make some of the String code

Re: Re: Incorrect arguments is passed to sun.misc.Perf#createLong

Hi Mandy, On 4:59, Mandy Chung wrote: Hi Yasumasa, I'm adding core-libs and bcc serviceability-dev to move this thread to core-libs for sun.misc.PerfCounter discussion. On 4/9/2013 4:50 AM, Yasu wrote: Hi, I'm trying to create entry from Java program to hsperfdata through sun.misc.PerfCoun

Re: RFR: 8004518 & 8010122 : Default methods on Map

On Apr 8 2013, at 14:09 , Peter Levart wrote: > Hi Mike, > > It's unfortunate that getOrDefault() is specified so that it can't be > implemented atomically in terms of existent (non-default) ConcurrentMap > methods, so platform ConcurrentMap implementations will have atomic > implementation a

Re: JDK 8 code review request for 8011800: Add java.util.Objects.requireNonNull(T, Supplier)

On 4/10/2013 5:02 AM, Remi Forax wrote: On 04/09/2013 11:12 PM, Joe Darcy wrote: Hello, Please review my changes for 8011800: Add java.util.Objects.requireNonNull(T, Supplier) http://cr.openjdk.java.net/~darcy/8011800.0/ which add a new method to java.util.Objects to take a Supplier

hg: jdk8/tl/jdk: 8005460: [findbugs] Probably returned array should be cloned

Changeset: 6f8e1428f7c3 Author:weijun Date: 2013-04-11 10:58 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6f8e1428f7c3 8005460: [findbugs] Probably returned array should be cloned Reviewed-by: xuelei ! src/share/classes/sun/security/krb5/PrincipalName.java + test/sun/secur

hg: jdk8/tl/jdk: 2 new changesets

Changeset: 0ab22e58d151 Author:weijun Date: 2013-04-11 11:09 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0ab22e58d151 8011867: Accept unknown PKCS #9 attributes Reviewed-by: vinnie ! src/share/classes/sun/security/pkcs/PKCS9Attribute.java + test/sun/security/pkcs/pkcs9/Un

Re: RFR: 8004518 & 8010122 : Default methods on Map

On Apr 8 2013, at 17:08 , Ulf Zibis wrote: > Hi Mike, > > Comments for j.u.Map: > > To my savour the variant belongs to the left hand side of a comparison, e.g.: > if (v = get(key) != null) Yeah, you caught me. Almost everyone hates these "Yoda conditions" (http://wiert.me/2010/05/25/yoda-c

hg: jdk8/tl/jdk: 8009638: Wrong comment for PL in LocaleISOData, 1989 forward Poland is Republic of Poland

Changeset: 006a7a576fe9 Author:peytoia Date: 2013-04-11 12:22 +0900 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/006a7a576fe9 8009638: Wrong comment for PL in LocaleISOData, 1989 forward Poland is Republic of Poland Reviewed-by: okutsu ! src/share/classes/java/util/LocaleISODat

RFR 8011805: Update sun.tools.java class file reading/writing support to include the new constant pool entries (including invokedynamic)

Currently blocking lambda library pushes. Internal class reader used by rmic does not support new constant pool constant types: CONSTANT_METHODHANDLE = 15; CONSTANT_METHODTYPE = 16; CONSTANT_INVOKEDYNAMIC= 18; Please review the fix for CR: http://bugs.sun.

Re: RFR 8011805: Update sun.tools.java class file reading/writing support to include the new constant pool entries (including invokedynamic)

- License on test is the wrong one. Tests don't have classpath exemption. - The initial block comment with @test doesn't have the normal leading * on each line. - The commented out part mentioning testWrite should just be removed along with unreferenced code. Looks otherwise fine to me. Mike

Re: RFR: 8004518 & 8010122 : Default methods on Map

On Apr 8 2013, at 19:22 , David Holmes wrote: > Hi Mike, > > Looking only at Map itself for now. > > On 9/04/2013 4:07 AM, Mike Duigou wrote: >> Hello all; >> >> This is a combined review for the new default methods on the java.util.Map >> interface being added for the JSR-335 lambda librarie

Re: RFR: 8004518 & 8010122 : Default methods on Map

Hi Mike, Comments inline and trimmed ... On 11/04/2013 2:41 PM, Mike Duigou wrote: On Apr 8 2013, at 19:22 , David Holmes wrote: (where has our style guide gone? I can't find it on internal or external wikis :( ) This one? http://www.oracle.com/technetwork/java/codeconv-138413.html I am un

Re: RFR: 8004518 & 8010122 : Default methods on Map

On Apr 10 2013, at 22:06 , David Holmes wrote: > Hi Mike, > > Comments inline and trimmed ... Additional trimming applied. > On 11/04/2013 2:41 PM, Mike Duigou wrote: >> On Apr 8 2013, at 19:22 , David Holmes wrote: 8004518: Add in-place operations to Map forEach() replaceAll(

Re: RFR: 8004518 & 8010122 : Default methods on Map

I've posted an updated webrev with the review comments I have received. http://cr.openjdk.java.net/~mduigou/JDK-8010122/1/webrev/ One important point to consider: - The current implementations of compute, computeIfPresent, computeIfAbsent, merge are implemented so that they can work correctly

Re: RFR: 8004518 & 8010122 : Default methods on Map

On 04/11/2013 06:41 AM, Mike Duigou wrote: General style issues: - spaces after keyword ie "if (x == null)" not "if(x == null)" Fixed. I am sorry this keeps coming up. I am loathe to run an automatic formatter on any JDK code. Hi Mike, I find IDEA's feature very useful. It can reformat j

Re: RFR: 8004518 & 8010122 : Default methods on Map

On 04/11/2013 07:06 AM, David Holmes wrote: I find the spec for these rather confusing from a concurrency perspective - this non-concurrent interface seems to be trying to say too much about how a concurrent interface should specify behaviour. Why does it need to say: * In concurrent contex