February 2014 - core-libs-dev

Adding a link() Method to Method, Redux
by Jeremy Manson 06 Feb '14

06 Feb '14

Hi folks, ( I sent this around in 2009, and it kind of got dropped. We're forward porting our patches to JDK8 now, so I've come across it again. It would be great not to have to keep forward porting it, and using reflection to invoke it. Original thread is here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-August/002319.html Original method follows... ) I found myself in need of a method that determines whether a given native method is linked, and links it if it is present. My use case prevents me from calling the method directly and catching an UnsatisfiedLinkError, because the method has side effects. This is not the first time I've needed such a method, so I implemented it and dropped it into java.lang.reflect.Method. I thought I would ask if it was something that should be added to the platform. The prototype looks like this: /** * Attempts to link this method, per Section 5.6 of the Java * Virtual Machine Specification. Returns {@code true} if this is * a Java method, or if it is a native method and the Java Virtual * Machine succeeds in loading an appropriate native-language * definition of it; returns {@code false} otherwise. * * @return {@code true} if this is a Java method, or if it is a * native method and the Java Virtual Machine succeeds in * loading an appropriate native-language definition of it; returns * {@code false} otherwise. * */ public boolean tryLink(); This is basically useful in any case where you would otherwise catch UnsatisfiedLinkError, but can't invoke the method. For example, if you have a binary that lives on the system, but you aren't sure whether that shared library provides the implementation you want. (Another point in the design space would be to have it called link(), return void, and throw an UnsatisfiedLinkError if it fails. It is also worth pointing out that the JVMS calls it "binding", not "linking", so there is that possibility as well.) What do you all think? It seems equally as useful as the ability to catch UnsatisfiedLinkError from native method invocation. (2014 update: Alan thought I should add a versioning method to my native library. This is less direct, of course. Thoughts? Yes / no / maybe?) Jeremy

1 0

RFR: 8033590: java.util.Comparator thenComparing has unnecessary type restriction
by Henry Jen 06 Feb '14

06 Feb '14

Hi, Please review a simple bug fix that loosen the generic type, the original proposed CCC for JDK-8009736 and webrev/specdiff has a discrepancy, I am not sure if another CCC is needed. https://bugs.openjdk.java.net/browse/JDK-8033590 http://cr.openjdk.java.net/~henryjen/tl/8033590.0/webrev/ Cheers, Henry

2 1

hg: jdk8/tl/jdk: 8033590: java.util.Comparator::thenComparing has unnecessary type restriction
by henry.jen＠oracle.com 06 Feb '14

06 Feb '14

Changeset: 7534523b4174 Author: henryjen Date: 2014-02-06 10:30 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7534523b4174 8033590: java.util.Comparator::thenComparing has unnecessary type restriction Reviewed-by: psandoz ! src/share/classes/java/util/Comparator.java ! test/java/util/Comparator/TypeTest.java

1 0

RFR: 8033565: Remove unused nativeNewStringPlatform and nativeGetStringPlatformChars
by roger riggs 06 Feb '14

06 Feb '14

Please review this change to remove unused support in jni_util.c for functions nativeNewStringPlatform and nativeGetStringPlatformChars. On Windows, both were conditional on jvm.dll being installed on a path containing "kernel". On Solaris, Linux, and Mac, they had no-op implementations. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-remove-nativenewstring-8033565/ Thanks, Roger

4 3

RFR: String.join(), StringJoiner additions
by Jim Gish 05 Feb '14

05 Feb '14

Please review http://cr.openjdk.java.net/~jgish/Bugs-5015163-7175206-7172553/ <http://cr.openjdk.java.net/%7Ejgish/Bugs-5015163-7175206-7172553/> These are changes that we made in lambda that we're now bringing into JDK8. I've made a couple of additions - making StringJoiner final and adding a couple of constructors to set the emptyOutput chars. Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish(a)oracle.com

12 48

RFR: Checking for JNI Exceptions in jni_util.c
by roger riggs 05 Feb '14

05 Feb '14

Please review these cleanups to check for JNI pending exceptions in jni_util.c. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-check_exception-8030993/ Thanks, Roger

3 6

RFR - 8033542: test/java/util/logging/SimpleLogManager might let Loggers be gc'ed too early.
by Daniel Fuchs 05 Feb '14

05 Feb '14

Hi, Please find below a fix [1] for: 8033542: test/java/util/logging/SimpleLogManager might let Loggers be gc'ed too early. [2] [1] http://cr.openjdk.java.net/~dfuchs/webrev_8033542/webrev.00 [2] https://bugs.openjdk.java.net/browse/JDK-8033542 The issue is that some of the loggers created by this test may not be strong referenced, which may cause the test to fail. -- daniel

2 1

ArrayList.removeAll()/retainAll()
by Benedict Elliott Smith 04 Feb '14

04 Feb '14

Hi, I notice this (or a related issue) has been mentioned before<http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017663.html>on this list, but I'm not convinced the correct resolution was reached. We are seeing this problem thrown by antlr, but rather than a bug in antlr, as surmised on the previous exchange, it looks to me that ArrayList is imposing a new constraint that is neither declared by itself nor Collection, and is unnecessary. ArrayList happily supports null elements, so requiring that the provided collection has no null elements is surely a bug? I've pasted the two declarations below for ease of reference. Neither javadocs describe the constraint that is imposed. ArrayList: /** * Removes from this list all of its elements that are contained in the * specified collection. * * @param c collection containing elements to be removed from this list * @return {@code true} if this list changed as a result of the call * @throws ClassCastException if the class of an element of this list * is incompatible with the specified collection * (<a href="Collection.html#optional-restrictions">optional</a>) * @throws NullPointerException if this list contains a null element and the * specified collection does not permit null elements * (<a href="Collection.html#optional-restrictions">optional</a>), * or if the specified collection is null * @see Collection#contains(Object) */ public boolean removeAll(Collection<?> c) { Objects.requireNonNull(c); return batchRemove(c, false); } Collection: /** * Removes all of this collection's elements that are also contained in the * specified collection (optional operation). After this call returns, * this collection will contain no elements in common with the specified * collection. * * @param c collection containing elements to be removed from this collection * @return <tt>true</tt> if this collection changed as a result of the * call * @throws UnsupportedOperationException if the <tt>removeAll</tt> method * is not supported by this collection * @throws ClassCastException if the types of one or more elements * in this collection are incompatible with the specified * collection * (<a href="#optional-restrictions">optional</a>) * @throws NullPointerException if this collection contains one or more * null elements and the specified collection does not support * null elements * (<a href="#optional-restrictions">optional</a>), * or if the specified collection is null * @see #remove(Object) * @see #contains(Object) */ boolean removeAll(Collection<?> c);

3 4

Re: Demo for Parallel Core Collection API
by Paul Sandoz 04 Feb '14

04 Feb '14

Hi, Some high-level points first: - try and use static import where possible. - suggest that all streams are sequential. There is an inconsistency in the use and in some cases it is embedded in other stream usages. - "ParallelCore" is not a very descriptive name. Suggest "streams". - suggest moving the data classes and XML parsing code to a separate package. - Unfortunately Supplier is overloaded with the functional interface in j.u.function. Not sure much could be done about that. More details below. I am not really commenting on the specific use-case, just the usages of the API itself, plus for brevity i have removed comments. Conversion -- A more compact form of mostTProductsByCategory (without comments) is: public static <T extends Comparable<T>> Product[] mostTProductsByCategory( Function<Product, T> func){ Map<String, Optional<Product>> m = products.stream(). collect(groupingBy(Product::getCategory, maxBy(comparing(func)))); return m.values().stream(). map(Optional::get). toArray(Product[]::new); } i.e. show the Map rather than the Collector. A DRYer form: public static Collection<String> countries(boolean ordered) { Stream<String> countries = customers.stream().map(Customer::getCountry); if (ordered) { return countries.distinct().collect(toCollection(LinkedList::new)); } else { return countries.collect(Collectors.<String>toSet()); } } Shame that the type witness is required. For sequential streams is probably no advantage here to providing a link list over a list except for API educational value. Elements -- Simpler form. The function to apply the terminal op obfuscates: public static Optional<Supplier> suppliersInCountry(boolean findAny, String country) { Stream<Supplier> s = suppliers.stream(). //filter supplier who is same sa given country filter(supplier -> country. equals(supplier.getCountry())); return findAny ? s.findAny() : s.findFirst(); } The use of the collector is complicating matters. Off the top of my head what you require is a reducer that reduces to the first product whose stock units is > 0: public static Map<String, Product> inStockProductsByCategory( boolean findAny) { BinaryOperator<Product> reduceToFirstMatch = (l, r) -> (l != null) ? l : (r != null && r.getUnitsInStock() > 0) ? r : null; return products.stream().collect( groupingBy(Product::getCategory, reducing(null, reduceToFirstMatch))); } The above relies on the associativity of the binary operator. There is no need to collect into a filtered list then stream with findAny/findFirst since we can reduce to the result as each element is received. That reduceToFirstMatch can easily be abstracted into a higher order function taking a predicate Grouping -- public static Map<String, List<Order>> groupOrdersByCustomer() { //a collector that generate a order list from a customer Collector<Customer, List<Order>, List<Order>> collectOrder = Collector. of(ArrayList<Order>::new, (orders, customer) -> { orders.addAll(customer.getOrders()); }, (left, right) -> { left.addAll(right); return left; }); return customers.parallelStream(). //convert customers to a Map which key is it name, value is its //orders collect(Collectors. groupingBy(Customer::getCompanyName, collectOrder)); } Not clear to me if there are multiple customers with the same name so the catamorphic collect may not be necessary and toMap can be used instead. Perhaps there is another way to show this collect usage? Can simplify: public static <S, T extends Comparable<T>> Map<S, Optional<Product>> productMaxByTGroupByS(boolean maxBy, Function<Product, S> groupFunc, Function<Product, T> compFunc){ //Comparator of Product which will compare on T by given function Comparator<Product> comp = Comparator.comparing(compFunc); return products.stream(). //collect products into a Map, which key is S by function of //groupFunc, value is max or min by given max and function of //compFunc collect(groupingBy( groupFunc, maxBy ? maxBy(comp) : minBy(comp))); } Simplify: public static <R, T> Map<R, Map<T, List<Order>>> ordersByRThenT( Function<Order, R> func1,Function<Order, T> func2){ return customers.stream(). //map every customer to orders flatMap(customer -> customer.getOrders().stream()). //group products to Map<R, T> by given two functions collect(groupingBy(func1, groupingBy(func2))); } Selection -- public static Map<Customer,Double> bigOrderCustomers(double bigValue) { Function<Customer, Double> orderTotal = c -> c.getOrders().stream(). //map order to order price mapToDouble(Order::getTotal). //calculate total price sum(); //A predicate which filter customer whose total order price is greater //than bigValue Predicate<Customer> bigOrder = c -> orderTotal.apply(c) >= bigValue; return customers.parallelStream(). //filter customer whose total order price is greater than bigValue filter(bigOrder). //grouping customer to a Map which key is customer, value is //total order price collect( Collectors.toMap(Function.identity(), orderTotal)); } This is essentially performing the sum twice on each element that passes through the filter. Unfortunately we don't have tuples yet and that is what i think you really need to retain the calculation for use later on. It might be simpler just to create a Map<Customer, Double> then iterate over entries and remove ones that do not match. Subset -- public static List<Order> firstNOrdersFromState(int number, String state, Consumer<Order> action){ return customers.parallelStream(). //only take customers from a particular state filter(c -> state.equals(c.getRegion())). //get orders of those customers flatMap(c -> c.getOrders().stream()). //get first number of orders substream(0, number). //apply the supplied action peek(action). //collect to a list collect(Collectors.toList()); } Replace substream(0, number) with limit(number). The use of peek here is setting a bad precedent, suggest changing to return void and using forEach or removing it. Same applies for topNCustomer. Tabulate -- This is another example of where tuples or MapStream (a stream for a tuple of 2 elements) could be useful. It would be good to remove the Pair class if at all possible, if not it is best if containing within Tabulate as this is the only place that uses it. For sameCountryCustomerAndSuppliers, off the top of my head, it is possible to reformulate using two Maps and we do not need Pair: Map<String, List<Supplier>> countryToSuppliers = suppliers.stream().collect( // group suppliers by their country Collectors.groupingBy(Supplier::getCountry)); Map<Customer, List<Supplier>> s = customers.stream().collect( Collectors.toMap(Function::identity, c -> countryToSuppliers.getOrDefault(c.getCountry(), null))); Simplify: public static Set<String> bothCustomerAndSupplier() { Set<String> suppliersName = suppliers.stream(). map(Supplier::getSupplierName). collect(toSet()); return customers.stream(). map(Customer::getCompanyName). flatMap(c -> suppliersName.contains(c) ? Stream.of(c) : Stream.empty()). collect(toSet()); } Hth, Paul. On Oct 14, 2013, at 5:27 AM, Tristan Yan <tristan.yan(a)oracle.com> wrote: > Hi all > > > > Could you please help to review the demo code for parallel core collection API? > > http://cr.openjdk.java.net/~pzhang/Tristan/8023555/webrev/ > > > > Thank you very much > > > > > > Tristan Yan(Haibo Yan) > > Office : 8610-61066212 > > Fax : 8610-61065441 > > Cell : 86-18610696822 > > > > 2F, Building No. 24, Zhongguancun Software Park > > Haidian District HYPERLINK "http://people.us.oracle.com/pls/oracle/f?p=8000:6:396067987304343:::6:P6_CI…"Beijing , 100193 > > oracle > > >

2 12

RFR : 8029020 : Check src/share/native/java/util/zip code for JNI pending exceptions
by Seán Coffey 04 Feb '14

04 Feb '14

Looking to correct some JNI handling code in java/util/zip for jdk9-dev. webrev : http://cr.openjdk.java.net/~coffeys/webrev.8029020/webrev/ <http://cr.openjdk.java.net/%7Ecoffeys/webrev.8029020/webrev/> regards, Sean.

4 5