March 2013 - core-libs-dev

Re: RFR: 8001647: In-place methods on Collection/List
by Akhil Arora 21 Apr '13

21 Apr '13

Updated with yours and Alan's comments - http://cr.openjdk.java.net/~akhil/8001647.2/webrev/ - removed null check for removeSet - cache this.size in removeAll, replaceAll (for ArrayList, Vector and CopyOnWriteArrayList) - calculate removeCount instead of BitCount.cardinality() - removed unnecessary @library from test support classes Catching IndexOOB and throwing CME in forEach/removeAll/replaceAll seems unecessary... did you have a use-case in mind where an IndexOOB can occur with these methods? Thanks On 12/08/2012 05:11 AM, Arne Siegel wrote: > ArrayList.java, line 1171: > final boolean anyToRemove = (removeSet != null) && !removeSet.isEmpty(); > As removeSet will never be null, this line can be simplified. This is a left-over from the > preceding implementation (see b67). > > ArrayList.java, method forEach optimizes the loop by reducing the number of heap accesses: > final int size = this.size; > for (int i=0; modCount == expectedModCount && i < size; i++) { > ... > This trick might also be introduced to methods removeAll and replaceAll. > > In the ListIterator implementation of ArrayList, there are various appearances of the idiom: > try { > ... > } catch (IndexOutOfBoundsException ex) { > throw new ConcurrentModificationException(); > } > This technique might also be used in forEach, removeAll, and replaceAll (though not likely to > be of any importance). > > Regards > Arne Siegel >

5 16

Code review request: JDK-8008670 (partial java.util.stream implementation)
by Brian Goetz 18 Apr '13

18 Apr '13

At: http://cr.openjdk.java.net/~briangoetz/jdk-8008670/webrev/ there is a webrev of a subset of the implementation of java.util.stream, for review. These are all new files. None of these are public classes; they are internal interfaces for the Stream library, as well as some implementation classes. This is about half the classes in the Streams package. (The webrev includes changes to Map, but those are being reviewed separately, so just ignore those.) We're asking people to focus their review on both the quality of API specification for these internal APIs, and the quality of implementation of the specified classes. It may not be obvious how some of these work until you've seen the rest of it, but do what you can.

5 13

JDK 8 code review request for 8005042 Add Method.isDefault to core reflection
by Joe Darcy 13 Apr '13

13 Apr '13

Hello, Please review these changes to add core reflection support to recognize default methods: 8005042 Add Method.isDefault to core reflection http://cr.openjdk.java.net/~darcy/8005042.0/ Thanks, -Joe

9 21

Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK
by Mandy Chung 11 Apr '13

11 Apr '13

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]. Webrev at: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/7198429/webrev.00/ While it touches many files, the fix is simple and straight-forward for review. This fix annotates all methods that call Reflection.getCallerClass() method with @sun.reflect.CallerSensitive annotation so that it enables the VM to reliably enforce that methods looking up its immediate caller class are marked as caller-sensitive. The JVM will set a new caller-sensitive bit when resolving a MemberName and java.lang.invoke.MethodHandleNatives.isCallerSensitive is upgraded to query it directly. The hand-maintained method list in MethodHandleNatives is removed. A couple things to mention: 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. 2. NashornScriptEngineFactory.getAppClassLoader() The change is to workaround the issue until 8009783 is resolved. The current implementation walks the stack to find the classloader of the user context that NashornScriptEngine is running on which is fragile. Also other script engine implementations may require similiar capability. 8009783 has been filed to revisit the scripting API to pass the user "context" to the script engine rather than relying the implementation to find it magically. Thanks Mandy [1] http://openjdk.java.net/jeps/176 [2] http://mail.openjdk.java.net/pipermail/hotspot-dev/2013-March/008915.html

7 22

RFR JDK-7143928 : (coll) Optimize for Empty ArrayList and HashMap
by Mike Duigou 11 Apr '13

11 Apr '13

Hello all; This is a review for optimization work that came out of internal analysis of Oracle's Java applications. It's based upon analysis that shows that in large applications as much as 10% of maps and lists are initialized but never receive any entries. A smaller number spend a large proportion of their lifetime empty. We've found similar results across other workloads as well. This patch is not a substitute for pre-sizing your collections and maps--doing so will *always* have better results. This patch extends HashMap and ArrayList to provide special handling for newly created instances that avoids creating the backing array until needed. There is a very small additional cost for detecting when to inflate the map or list that is measurable in interpreted tests but disappears in JITed code. http://cr.openjdk.java.net/~mduigou/JDK-7143928/0/webrev/ We expect that should this code prove successful in Java 8 it will be backported to Java 7 updates. The unit test may appear to be somewhat unrelated. It was created after resolving a bug in an early version of this patch to detect the issue encountered (LinkedHashMap.init() was not being called in readObject() when the map was empty). Mike

12 43

RFR: Project files for Solaris Studio / NetBeans
by Jesper Wilhelmsson 11 Apr '13

11 Apr '13

Hi, Sorry for cross posting, but I think this could be useful for several areas. I would like to add Solaris Studio / NetBeans project files for the entire OpenJDK project. To clarify: One project that contains the entire OpenJDK. With the new build infrastructure in JDK 8 building the entire OpenJDK is fairly fast and even though I personally mostly work in the HotSpot tree, I tend to always clone and build the entire JDK forest. I find this to have several benefits. Webrev: http://cr.openjdk.java.net/~jwilhelm/7074926/webrev/ The configuration in this project is currently Mac only. Linux and Solaris configurations are also planned. The webrev is made from the jdk8/build repository which is where I think a change like this should go in. Let me know if you think something else. To use this project (once pushed): 1. Clone your favorite repository hg clone http://hg.openjdk.java.net/hsx/hotspot-gc 2. Get the whole forest cd hotspot-gc sh get_source.sh 3. Configure sh configure 4. Open Solaris studio / NetBeans and load the project. The project in located in the common directory. Thanks, /Jesper

6 16

Re: RFR-8008118
by John Zavgren 11 Apr '13

11 Apr '13

Greetings: I made modifications as per Martin's suggestions: 1.) free pathv on "abort". 2.) allocate memory for storing the "cwd" string, and then copy it into the memory location (to make sure that the contents of the pathv[] array don't refer to memory that's from the stack of the procedure that created it.) Thanks! John http://cr.openjdk.java.net/~jzavgren/8008118/webrev.06/ ----- Original Message ----- From: martinrb(a)google.com To: christos(a)zoulas.com Cc: john.zavgren(a)oracle.com, core-libs-dev(a)openjdk.java.net Sent: Friday, March 22, 2013 10:19:24 AM GMT -05:00 US/Canada Eastern Subject: Re: RFR-8008118 On Thu, Mar 21, 2013 at 12:11 PM, Christos Zoulas < christos(a)zoulas.com > wrote: On Mar 21, 11:36am, john.zavgren(a)oracle.com (John Zavgren) wrote: -- Subject: Re: RFR-8008118 | All: | | How does this look? | 1.) I reverted the for statement formatting change. Not exactly. Now it is indented incorrectly. Agreed. Really revert. | 2.) I removed the goto statement and "inlined" some code instead. I prefer to write simpler code that works with both signed and unsigned indexes: + while (i > 0) + if (pathv[--i] != cwd) + free(pathv[i]); + Christos' suggestion looks pretty good. As Mark noted, need to add missing free of pathv itself. | 3.) I checked to make sure that we're not freeing memory that we didn't act= | ually allocate. (Path vector elements that are empty.) You are still using the "./" string literal constant in the code so you'll end up freeing it (the compiler will prolly merge the two instances and you'll get lucky but it is semantically incorrect). Agreed, pathv[i] = cwd;

6 24

RFR: Optimize StringBuilder.append(null)
by Martin Buchholz 08 Apr '13

08 Apr '13

Oops: I forgot to include core-libs-dev Forwarded conversation Subject: RFR: Optimize StringBuilder.append(null) ------------------------ From: *Martin Buchholz* <martinrb(a)google.com> Date: Tue, Mar 26, 2013 at 1:54 PM To: Xueming Shen <xueming.shen(a)oracle.com>, Masayoshi Okutsu < masayoshi.okutsu(a)oracle.com>, Ulf Zibis <Ulf.Zibis(a)cosoco.de>, Mike Duigou < mike.duigou(a)oracle.com> Hi! It's me again, fiddlling with StringBuilder. One of our users noticed that a lot of CPU time was being spent in StringBuilder.append(null). We can apply the previously proven optimization that improves it by a factor of 2 on microbenchmarks. http://cr.openjdk.java.net/~martin/webrevs/openjdk8/StringBuilder.appendNul… ---------- From: *Mike Duigou* <mike.duigou(a)oracle.com> Date: Tue, Mar 26, 2013 at 1:58 PM To: Martin Buchholz <martinrb(a)google.com> Cc: Xueming Shen <xueming.shen(a)oracle.com>, Masayoshi Okutsu < masayoshi.okutsu(a)oracle.com>, Ulf Zibis <Ulf.Zibis(a)cosoco.de> This looks fine to me. Do you need a bug number? Mike ---------- From: *Martin Buchholz* <martinrb(a)google.com> Date: Tue, Mar 26, 2013 at 2:02 PM To: Mike Duigou <mike.duigou(a)oracle.com> Cc: Xueming Shen <xueming.shen(a)oracle.com>, Masayoshi Okutsu < masayoshi.okutsu(a)oracle.com>, Ulf Zibis <Ulf.Zibis(a)cosoco.de> On Tue, Mar 26, 2013 at 1:58 PM, Mike Duigou <mike.duigou(a)oracle.com> wrote: > This looks fine to me. Do you need a bug number? > > Yes, I do (thanks!) ---------- From: *Mike Duigou* <mike.duigou(a)oracle.com> Date: Tue, Mar 26, 2013 at 2:12 PM To: Martin Buchholz <martinrb(a)google.com> Cc: Xueming Shen <xueming.shen(a)oracle.com>, Masayoshi Okutsu < masayoshi.okutsu(a)oracle.com>, Ulf Zibis <Ulf.Zibis(a)cosoco.de>, Jim Gish < jim.gish(a)oracle.com> I have created: JDK-8010849: (str) Optimize StringBuilder.append(null) (yes we know that requiring an Oracle engineer create the bug is a problem. SOON!) Mike ---------- From: *Mike Duigou* <mike.duigou(a)oracle.com> Date: Tue, Mar 26, 2013 at 6:57 PM To: Martin Buchholz <martinrb(a)google.com> Hi Martin; I hadn't originally noticed but can you make sure to send a RFR through core-libs-dev for this issue? Mike

4 8

JAX-WS update coming soon
by Alan Bateman 08 Apr '13

08 Apr '13

Just a heads-up that there is a JAX-WS update coming for jdk8. Miroslav Kos will be sending a webrev soon with the changes that update what we have in jdk8 from 2.2.7-b09 to 2.2.9-b13922. The patch file is likely be >100k lines so impossible to do a complete review. The tentative plan is to push this via jdk8/tl/jaxws and any help with the review would be appreciated. -Alan.

2 2

RFR 8005696: Add CompletableFuture - JEP 155
by Chris Hegarty 04 Apr '13

04 Apr '13

As part of JEP 155 (Concurrency Updates) we are proposing to add CompletableFuture. A Future that may be explicitly completed (setting its value and status), and may include dependent functions and actions that trigger upon its completion. Specdiff: http://cr.openjdk.java.net/~chegar/8005696/specdiff/java/util/concurrent/pa… Webrev: http://cr.openjdk.java.net/~chegar/8005696/webrev.00/webrev/ CompletableFuture, as usual, is coming from Doug Lea, with assistance from members of the former JCP JSR-166 Expert Group. I have already informally reviewed this. This is an official call for review before finalizing the API for inclusion in jdk8. Note: I still need to make some additions to the test for better coverage, but these should not be a blocker for this review. -Chris.

3 10