Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

[Tagir Valeev]

Hello!

If you are going to create long-living array which is likely to be empty,
it's good to deduplicate them to spare the heap space. This can be easily
done via existing toArray method like
collection.toArray(MyClass.EMPTY_ARRAY) assuming we have an empty array
constant. We use this pattern very often in IntelliJ IDEA source code (e.
g. think of method which returns an array of Java member annotations; often
there are none). New generator methods is much less convenient for this:
toArray(s -> s == 0?MyClass.EMPTY_ARRAY:new MyClass[s]). I'm actually
missing a method accepting an empty array instead of generator in the
Stream API. And I don't think that new collection method is useful. The
existing one is perfect.

With best regards,
Tagir Valeev

пт, 15 июня 2018 г., 8:03 Stuart Marks :

> Hi all,
>
> I wanted to restart review of the changeset for bug JDK-8060192 [1]. The
> latest
> webrev is here: [2].
>
> The previous review was from December 2017: [3]. The only difference
> between the
> current changeset and the previous one is the removal of the overriding
> implementations (see explanation below). Much of the discussion in the
> previous
> review thread was around performance and the shape of the API.
>
> # Performance
>
> I previously had done some benchmarking on this and reported the results:
> [4].
> (I've recently re-done the benchmarks and conclusions are essentially the
> same.)
>
> The upshot is that implementations that use Arrays.copyOf() are the
> fastest,
> probably because it's an intrinsic. It can avoid zero-filling the freshly
> allocated array because it knows the entire array contents will be
> overwritten.
> This is true regardless of what the public API looks like. The default
> implementation calls generator.apply(0) to get a zero-length array and
> then
> simply calls toArray(T[]). This goes through the Arrays.copyOf() fast
> path, so
> it's essentially the same speed as toArray(new T[0]).
>
> The overrides I had previously provided in specific implementation classes
> like
> ArrayList actually are slower, because the allocation of the array is done
> separately from filling it. This necessitates the extra zero-filling step.
> Thus,
> I've removed the overrides.
>
> # API Shape
>
> There was some discussion about whether it would be preferable to have a
> Class
> parameter as a type token for the array component type or the array type
> itself,
> instead of using an IntFunction generator. Most of this boils down to what
> people are comfortable with. However, there are a few points that make the
> generator function approach preferable.
>
> One point in favor of using a generator is that Stream already has a
> similar
> toArray(generator) method.
>
> Comparing this to the type token alternatives, for simple tasks like
> converting
> Collection to String[], things are about equal:
>
>  toArray(String[]::new)
>  toArray(String.class)
>  toArray(String[].class)
>
> However, things are hairier if the element type of the collection is
> generic,
> such as Collection>. The result type is a generic array
> List[]. Using class literals or array constructor references will
> necessitate using an unchecked cast, because none of these can be used to
> represent a generic type.
>
> However, it's possible to use a method reference to provide a properly
> generic
> IntFunction. This would enable the toArray(generator) method to be used
> without
> any unchecked casts. (The method it refers to might need have an unchecked
> cast
> within it, though.) Such a method could also be reused for the
> corresponding
> Stream.toArray(generator) method.
>
> For these reasons I'd like to proceed with adding toArray(generator) API.
>
> Thanks,
>
> s'marks
>
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8060192
>
> [2] http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.3/
>
> [3]
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050325.html
>
> [4]
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050585.html
>
>

Re: BiCollector

[Paul Sandoz]

Hi Tagir,

This is looking good.

My current favorite name for the factory method is “bisecting” since we are 
dividing the collector into two collectors, the results of which are then 
merged.
 
Suggested first paragraph of the factory method:

  "Returns a collector that passes the input elements to two specified 
collectors and merges their results with the specified merge function.”

Paul.
 

> On Jun 15, 2018, at 4:26 AM, Tagir Valeev  wrote:
> 
> Hello!
> 
> I created a preliminary webrev of my own implementation (no testcases yet):
> http://cr.openjdk.java.net/~tvaleev/patches/pairing/webrev/
> If anybody wants to sponsor my implementation, I will happily log an issue 
> and write tests.
> 
> The name "pairing" was invented by me, but as I'm not a native English 
> speaker I cannot judge whether it's optimal, so better ideas are welcome.
> Also I decided to remove accumulator types from public type variables. They 
> do not add anything to type signature, only clutter it 
> increasing the number of type parameters from 4 to 6. I think it was a 
> mistake to expose the accumulator type parameter in other cascading 
> collectors 
> like filtering(), collectingAndThen(), groupingBy(), etc. I'm not insisting 
> though, if you feel that conformance to existing collectors is 
> more important than simplicity.
> 
> With best regards,
> Tagir Valeev.
> 
> On Fri, Jun 15, 2018 at 5:05 AM Brian Goetz  wrote:
> 
> > Well, I don't see the need to pack the two results into a Map.Entry 
> > (or any similar) container as a drawback. 
> 
>  From an "integrity of the JDK APIs" perspective, it is unquestionably a 
> drawback.  These items are not a Key and an associated Value in a Map; 
> it's merely pretending that Map.Entry really means "Pair".  There's a 
> reason we don't have a Pair class in the JDK (and no, let's not reopen 
> that now); using something else as a Pair proxy that is supposed to have 
> specific semantics is worse. (It's fine to do this in your own code, but 
> not in the JDK. Different standards for code that has different audiences.)
> 
> Tagir's proposed sidestepping is nice, and it will also play nicely with 
> records, because then you can say:
> 
>   record NameAndCount(String name, int count);
> 
>   stream.collect(pairing(collectName, collectCount, NameAndCount::new));
> 
> and get a more properly abstract result out.  And its more in the spirit 
> of existing Collectors.  If you want to use Map.Entry as an 
> _implementation detail_, that's fine.
> 
> I can support this form.
> 
> > I also don't see a larger abstraction like BiStream as a natural fit 
> > for a similar thing. 
> 
> I think the BiStream connection is mostly tangential.  We tried hard to 
> support streams of (K,V) pairs when we did streams, as Paul can attest, 
> but it was a huge complexity-inflater and dropping this out paid an 
> enormous simplification payoff.
> 
> With records, having streams of tuples will be simpler to represent, but 
> no more performant; it will take until we get to value types and 
> specialized generics to get the performance we want out of this.
> 
> 

[11] RFR: 8042131: DateTimeFormatterBuilder Mapped-values do not work for JapaneseDate

[Naoto Sato]

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8042131

The proposed change is located at:

http://cr.openjdk.java.net/~naoto/8042131/webrev.00/

The fix is originally contributedy by Toshio Nakamura at IBM [1]. I am 
sponsoring the fix, with some trivial modifications to the test (diff is 
attached).


Naoto

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-June/053830.html
diff -r 9b997bfc60d5 
test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
--- 
a/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
+++ 
b/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -64,24 +64,23 @@
  */
 package test.java.time.format;
 
+import java.time.chrono.ChronoLocalDate;
 import java.time.chrono.Chronology;
 import java.time.chrono.IsoChronology;
 import java.time.chrono.JapaneseChronology;
 import java.time.chrono.JapaneseEra;
 import java.time.chrono.MinguoChronology;
 import java.time.chrono.ThaiBuddhistChronology;
-import java.time.chrono.ChronoLocalDate;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeFormatterBuilder;
 import java.time.format.FormatStyle;
 import java.time.LocalDate;
+import java.time.temporal.ChronoField;
 import java.time.temporal.Temporal;
-import java.time.temporal.ChronoField;
-import static java.time.temporal.ChronoUnit.YEARS;
 
+import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
-import java.util.HashMap;
 
 import static org.testng.Assert.assertEquals;
 
@@ -122,23 +121,21 @@
 }
 
 //---
-@DataProvider(name="mappedPatterns")
-Object[][] localizedMappedPatterns() {
+@DataProvider(name="mapTextLookup")
+Object[][] data_mapTextLookup() {
 return new Object[][] {
 {IsoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
-{JapaneseChronology.INSTANCE.date(JapaneseEra.HEISEI,
-  1,1,8), Locale.ENGLISH},
+{JapaneseChronology.INSTANCE.date(JapaneseEra.HEISEI, 1, 1, 8), 
Locale.ENGLISH},
 {MinguoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
 {ThaiBuddhistChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
 };
 }
 
-@Test(dataProvider="mappedPatterns")
-public void test_getLocalizedMappedPattern(ChronoLocalDate date, Locale 
locale) {
+@Test(dataProvider="mapTextLookup")
+public void test_appendText_mapTextLookup(ChronoLocalDate date, Locale 
locale) {
 final String new1st = "1st";
 Map yearMap = new HashMap<>();
 yearMap.put(1L, new1st);
-DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
 builder.appendText(ChronoField.YEAR_OF_ERA, yearMap);
 
 String actual = date.format(builder.toFormatter(locale));

Re: RFR 8170159 Improve the performance of BitSet traversal

[Martin Buchholz]

+1

On Fri, Jun 15, 2018 at 2:52 PM, Paul Sandoz  wrote:

>
>
> On Jun 15, 2018, at 2:24 PM, Martin Buchholz  wrote:
>
> Still looks correct, but I might keep looking for something  more elegant.
> What bothers me a little now is
>
> 1290 long word = words[u] & (WORD_MASK << i);
>
>
> which is part of the initial setup and is not necessary on each iteration.
>
>
> I was looking at this too. From a performance perspective i am not very
> concerned. Aesthetically it bothers me, but at this point i am willing to
> declare victory and patch later if there is a better way.
>
> Paul.
>

Re: RFR 8198669: Refactor annotation array value parsing to reduce duplication

[Paul Sandoz]

> On Jun 15, 2018, at 2:31 PM, Liam Miller-Cushon  wrote:
> 
> On Fri, Jun 15, 2018 at 1:44 PM Paul Sandoz  > wrote:
> A nice cleanup. Did you run any tiered tests just to double check that the 
> use of lambdas causes no bootstrapping issues? (i suspect it probably does 
> not).
> 
> Thanks, I ran `run-test-tier1` and everything looks fine, are there other 
> tests I should check?
>  


tier1 should be ok.

Thanks,
Paul.

Re: RFR 8170159 Improve the performance of BitSet traversal

[Paul Sandoz]

> On Jun 15, 2018, at 2:24 PM, Martin Buchholz  wrote:
> 
> Still looks correct, but I might keep looking for something  more elegant.
> What bothers me a little now is
> 
> 1290 long word = words[u] & (WORD_MASK << i);
> 
> which is part of the initial setup and is not necessary on each iteration. 
> 

I was looking at this too. From a performance perspective i am not very 
concerned. Aesthetically it bothers me, but at this point i am willing to 
declare victory and patch later if there is a better way.

Paul.

Re: RFR 8170159 Improve the performance of BitSet traversal

[Martin Buchholz]

Still looks correct, but I might keep looking for something  more elegant.
What bothers me a little now is

1290 long word = words[u] & (WORD_MASK << i);


which is part of the initial setup and is not necessary on each iteration.

On Fri, Jun 15, 2018 at 1:13 PM, Paul Sandoz  wrote:

> Thanks! Doh, obvious in hindsight.
>
> On Jun 15, 2018, at 11:05 AM, Martin Buchholz  wrote:
>
> It took me a while to realize why you were checking tz < 63.  It's because
> the shift by 64 that might occur below would be a no-op!
>
>
> Right!
>
> 1301 action.accept(i++);1302  
>word &= (WORD_MASK << i);
>
> Can we rewrite to simply flip the one bit via
>
> word &= ~(1L << i);
> action.accept(i++);
>
> and then we can simplify the tz handling?
>
>
> Yes, and there is no need to increment i:
>
>   http://cr.openjdk.java.net/~psandoz/jdk/JDK-8170159-
> bitset-traverse/webrev/src/java.base/share/classes/java/
> util/BitSet.java.sdiff.html
>
> Paul.
>

Re: Proposal: optimization of Map.copyOf and Collectors.toUnmodifiableMap

[Stuart Marks]

Hi Peter,

Finally getting back to this.

I think there's clearly room for improvement in the creation of the unmodifiable 
collections. Right now the creation paths (mostly) use only public APIs, which 
can result in unnecessary allocation and copying. Certainly Map.copyOf does 
this, but there are also other cases as well.


For copying from an unknown map, there are a couple approaches:

* accumulate keys and values in a single ArrayList, which can be presized as 
necessary, but which will grow if necessary; then copy elements from a subrange 
of ArrayList's internal array (similar to your MapN(Object[], len) overload)


* accumulate keys and values into a SpinedBuffer, which doesn't require copying 
to grow, which is preferable if for some reason we can't pre-size accurately; 
and then copy the elements out of it


The Collectors.toUnmodifiableMap implementations are known to create HashMap 
instances, so they could pass the HashMap directly to a private MapN constructor 
that in turn could talk directly to HashMap to get the keys and values. This 
avoids allocation of a full-sized buffer and one copy.


Note that these techniques involve creating new interfaces, sometimes that cross 
package boundaries. It's a bit of an irritant to have to plumb new paths that go 
through SharedSecrets, but it seems likely to be worthwhile if we can avoid bulk 
allocation and copying steps.


Given these, it doesn't seem to me that the BiBuffer approach helps very much. I 
think there are many other avenues that would be worthwhile to explore, and that 
possibly can provide bigger savings.


s'marks





On 6/11/18 3:57 AM, Peter Levart wrote:

Hi,

Those two methods were added in JDK 10 and they are not very optimal. 
Map.copyOf(Map map) 1st dumps the source map into an array of Map.Entry(s) 
(map.entrySet().toArray(new Entry[0])), which typically creates new Map.Entry 
objects, then passes the array to Map.ofEntries(Map.Entry[] entries) factory 
method that iterates the array and constructs a key/value interleaved array from 
it which is passed to ImmutableCollections.MapN constructor, which then 
constructs a linear-probing hash table from it. So each key and value is copied 
3 times, while several temporary objects are created in the process.


One copying step could be eliminated and construction of temporary Map.Entry 
objects too:


http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/webrev.01/

Collecting stream(s) using Collectors.toUnmodifiableMap() 1st collects key/value 
pairs into a HashMap, then it performs equivalent operations as 
Map.copyOf(hashMap) at the end. Using Map.copyOf() directly benefits this 
collection operation too.


The following benchmark:

http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/UnmodifiableMapBench.java 




Shows up to 30% improvement of .copyOf operation with this patch applied:

Original:

Benchmark   (size)  Mode Cnt  Score Error  Units
UnmodifiableMapBench.copyOf 10  avgt 10    403.633 ± 2.640  
ns/op
UnmodifiableMapBench.copyOf    100  avgt   10 3489.623 ± 44.590  
ns/op
UnmodifiableMapBench.copyOf   1000  avgt   10 40030.572 ± 277.075  
ns/op

UnmodifiableMapBench.toUnmodifiableMap  10  avgt 10    831.221 ± 3.816  
ns/op
UnmodifiableMapBench.toUnmodifiableMap 100  avgt   10 9783.519 ± 43.097  
ns/op
UnmodifiableMapBench.toUnmodifiableMap    1000  avgt   10 96524.536 ± 670.818  
ns/op


Patched:

Benchmark   (size)  Mode Cnt  Score Error  Units
UnmodifiableMapBench.copyOf 10  avgt 10    264.172 ± 1.882  
ns/op
UnmodifiableMapBench.copyOf    100  avgt   10 2318.974 ± 15.877  
ns/op
UnmodifiableMapBench.copyOf   1000  avgt   10 29291.782 ± 3139.737  
ns/op

UnmodifiableMapBench.toUnmodifiableMap  10  avgt 10    771.221 ± 65.432  
ns/op
UnmodifiableMapBench.toUnmodifiableMap 100  avgt   10 9275.016 ± 725.722  
ns/op
UnmodifiableMapBench.toUnmodifiableMap    1000  avgt   10 82204.342 ± 851.741  
ns/op



Production of garbage is also reduced, since no Map.Entry temporary objects are 
constructed:


Original:

Benchmark (size)  Mode  Cnt  Score   Error   Units
UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 10  avgt   10    416.001 ± 
0.002    B/op
UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 100  avgt   10 2936.005 ± 
0.019    B/op
UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 1000  avgt   10 28136.059 ± 
0.199    B/op
UnmodifiableMapBench.toUnmodifiableMap:·gc.alloc.rate.norm 10  avgt 10   
1368.001 ± 0.004    B/op
UnmodifiableMapBench.toUnmodifiableMap:·gc.alloc.rate.norm 100  avgt 10  
10208.139 ± 0.045    B/op
UnmodifiableMapBench.toUnmodifiableMap:·gc.alloc.rate.norm 1000  avgt 10  
93025.923 ± 0.573    B/op


Patched:

Benchmark (size)  Mode  Cnt  Score   Error   Units
UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 10  avgt   10   

Re: RFR 8198669: Refactor annotation array value parsing to reduce duplication

[Paul Sandoz]

+1

A nice cleanup. Did you run any tiered tests just to double check that the use 
of lambdas causes no bootstrapping issues? (i suspect it probably does not).

Paul.

> On Jun 15, 2018, at 10:58 AM, Liam Miller-Cushon  wrote:
> 
> Hello,
> 
> This change is a follow-up to JDK-7183985 which replaces the interior of
> parseClassArray, parseEnumArray, and parseAnnotationArray with a shared
> method that is passed a lambda for the type-specific parsing logic.
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8198669
> webrev: http://cr.openjdk.java.net/~cushon/8198669/webrev.00/
> 
> Liam

Re: RFR 8170159 Improve the performance of BitSet traversal

[Paul Sandoz]

Thanks! Doh, obvious in hindsight.

> On Jun 15, 2018, at 11:05 AM, Martin Buchholz  wrote:
> 
> It took me a while to realize why you were checking tz < 63.  It's because 
> the shift by 64 that might occur below would be a no-op!

Right!

> 1301 action.accept(i++);
> 1302 word &= (WORD_MASK << i);
> Can we rewrite to simply flip the one bit via
> 
> word &= ~(1L << i);
> action.accept(i++);
> 
> and then we can simplify the tz handling?
> 


Yes, and there is no need to increment i:

  
http://cr.openjdk.java.net/~psandoz/jdk/JDK-8170159-bitset-traverse/webrev/src/java.base/share/classes/java/util/BitSet.java.sdiff.html
 


Paul.

Re: RFR 8199435 : Unsafe publication of java.util.Properties.map

[Paul Sandoz]

Yes, like that, thanks,
Paul.

> On Jun 15, 2018, at 11:01 AM, Brent Christian  
> wrote:
> 
> On 6/15/18 10:35 AM, Paul Sandoz wrote:
>> I would also publish the map in readHashtable after you have placed in the 
>> keys/values.
> 
> Like this?
> 
> // create CHM of appropriate capacity
> -map = new ConcurrentHashMap<>(elements);
> +ConcurrentHashMap tmpMap = new 
> ConcurrentHashMap<>(elements);
> 
> // Read all the key/value objects
> for (; elements > 0; elements--) {
> Object key = s.readObject();
> Object value = s.readObject();
> -map.put(key, value);
> +tmpMap.put(key, value);
> }
> +
> +UNSAFE.putObjectVolatile(this, MAP_OFFSET, tmpMap);
> }
> 
> -B

Re: RFR 8170159 Improve the performance of BitSet traversal

[Martin Buchholz]

It took me a while to realize why you were checking tz < 63.  It's because
the shift by 64 that might occur below would be a no-op!

1301 action.accept(i++);1302
  word &= (WORD_MASK << i);

Can we rewrite to simply flip the one bit via

word &= ~(1L << i);
action.accept(i++);

and then we can simplify the tz handling?

Re: RFR 8199435 : Unsafe publication of java.util.Properties.map

[Brent Christian]

On 6/15/18 10:35 AM, Paul Sandoz wrote:


I would also publish the map in readHashtable after you have placed in the 
keys/values.



Like this?

 // create CHM of appropriate capacity
-map = new ConcurrentHashMap<>(elements);
+ConcurrentHashMap tmpMap = new 
ConcurrentHashMap<>(elements);


 // Read all the key/value objects
 for (; elements > 0; elements--) {
 Object key = s.readObject();
 Object value = s.readObject();
-map.put(key, value);
+tmpMap.put(key, value);
 }
+
+UNSAFE.putObjectVolatile(this, MAP_OFFSET, tmpMap);
 }

-B

Re: Durations in existing JDK APIs

[Martin Buchholz]

On Wed, May 30, 2018 at 11:32 AM, Doug Lea  wrote:

>
> The original rationale for designing j.u.c.TimeUnit using the Flyweight
> pattern was to to reduce allocation and GC-related overhead and timing
> jitter for methods that otherwise may operate on the order of
> nanoseconds. But there are many cases in which this is not much of a
> concern (plus JVMs can now sometimes optimize), so people should be
> given a choice. It would be a lot of tedious work (and aggregate code
> bulk) to retrofit every time-related j.u.c method though, and it's not
> clear where to compromise. But at least adding converters should not be
> controversial.
>

Re-reading Doug's assessment, Doug seems reluctant but open to adding at
least some Duration overloads.  Here's  an obvious first candidate in Future
(yes, we have a test that discovers get(Duration) and checks that get(null)
throws UOE instead of NPE.).
(It's a lot of tedious work)

a/util/concurrent/CompletableFuture.java
===
RCS file:
/export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/CompletableFuture.java,v
retrieving revision 1.212
diff -u -r1.212 CompletableFuture.java
--- src/main/java/util/concurrent/CompletableFuture.java 11 Mar 2018
18:00:05 - 1.212
+++ src/main/java/util/concurrent/CompletableFuture.java 15 Jun 2018
17:39:09 -
@@ -1983,13 +1983,22 @@
  * while waiting
  * @throws TimeoutException if the wait timed out
  */
-@SuppressWarnings("unchecked")
 public T get(long timeout, TimeUnit unit)
 throws InterruptedException, ExecutionException, TimeoutException {
-long nanos = unit.toNanos(timeout);
+return getNanos(unit.toNanos(timeout));
+}
+
+public T get(java.time.Duration timeout)
+throws InterruptedException, ExecutionException, TimeoutException {
+return getNanos(TimeUnit.NANOSECONDS.convert(timeout));
+}
+
+@SuppressWarnings("unchecked")
+private T getNanos(long timeoutNanos)
+throws InterruptedException, ExecutionException, TimeoutException {
 Object r;
 if ((r = result) == null)
-r = timedGet(nanos);
+r = timedGet(timeoutNanos);
 return (T) reportGet(r);
 }

@@ -2797,6 +2806,8 @@
 throw new UnsupportedOperationException(); }
 @Override public T get(long timeout, TimeUnit unit) {
 throw new UnsupportedOperationException(); }
+@Override public T get(java.time.Duration duration) {
+throw new UnsupportedOperationException(); }
 @Override public T getNow(T valueIfAbsent) {
 throw new UnsupportedOperationException(); }
 @Override public T join() {
Index: src/main/java/util/concurrent/Future.java
===
RCS file:
/export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/Future.java,v
retrieving revision 1.41
diff -u -r1.41 Future.java
--- src/main/java/util/concurrent/Future.java 8 Oct 2016 18:52:37 - 1.41
+++ src/main/java/util/concurrent/Future.java 15 Jun 2018 17:39:09 -
@@ -6,6 +6,10 @@

 package java.util.concurrent;

+import static java.util.concurrent.TimeUnit.NANOSECONDS;
+
+import java.time.Duration;
+
 /**
  * A {@code Future} represents the result of an asynchronous
  * computation.  Methods are provided to check if the computation is
@@ -126,7 +130,30 @@
  * @throws InterruptedException if the current thread was interrupted
  * while waiting
  * @throws TimeoutException if the wait timed out
+ * @throws NullPointerException if {@code unit} is null
  */
 V get(long timeout, TimeUnit unit)
 throws InterruptedException, ExecutionException, TimeoutException;
+
+/**
+ * Waits if necessary for at most the given time for the computation
+ * to complete, and then retrieves its result, if available.
+ *
+ * Equivalent to:  {@code
+ * get(NANOSECONDS.convert(timeout), NANOSECONDS)}
+ *
+ * @param timeout the maximum time to wait
+ * @return the computed result
+ * @throws CancellationException if the computation was cancelled
+ * @throws ExecutionException if the computation threw an
+ * exception
+ * @throws InterruptedException if the current thread was interrupted
+ * while waiting
+ * @throws TimeoutException if the wait timed out
+ * @throws NullPointerException if {@code timeout} is null
+ */
+default V get(Duration timeout)
+throws InterruptedException, ExecutionException, TimeoutException {
+return get(NANOSECONDS.convert(timeout), NANOSECONDS);
+}
 }

Re: RFR 8199435 : Unsafe publication of java.util.Properties.map

[Paul Sandoz]

> On Jun 15, 2018, at 8:44 AM, Brent Christian  
> wrote:
> 
> Hi,
> 
> In JDK 9, 8029891[1] refactored java.util.Properties to store its values in 
> an internal ConcurrentHashMap, and removed synchronization from "reader" 
> methods in order to avoid potential hangs/deadlocks during classloading.
> 
> Claes has noticed that there is the possibility of the new 'map' field being 
> observed with its default value (null), before being set.
> 

Yes.

I would also publish the map in readHashtable after you have placed in the 
keys/values.

Pail.

> After looking at the JSR 133 FAQ[2], I agree with Claes that we should make 
> 'map' a field final.
> 
> Please review my change to do this:
> 
> Webrev:
> http://cr.openjdk.java.net/~bchristi/8199435/webrev/
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8199435
> 
> Thanks,
> -Brent
> 
> 1. https://bugs.openjdk.java.net/browse/JDK-8029891
> 2. https://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html#finalRight

Re: RFR 8170159 Improve the performance of BitSet traversal

[Paul Sandoz]

Hi Martin,

Thanks for reviewing.

> On Jun 15, 2018, at 7:57 AM, Martin Buchholz  wrote:
> 
> Code looks correct to me, but as usual there are some things I would try to 
> do differently,  
> 

Indeed, i tried a few different approaches before settling on this code shape.

> 1292 while (word != 0 && tz < 63) {
> 
> I'd try to make this loop simply
> while (word != 0)
> 

I started out like that but in the end preferred that both loop conditions were 
upfront, to avoid another break or explicitly setting word to 0, then the only 
break is the weird one to deal with the annoying bug magnet :-)


> ---
> 
> Probably neither of us is fond of the bug magnet special case for 
> Integer.MAX_VALUE.  Maybe just store fence as a long or as a wordindex which 
> can't overflow (while giving up on intra-word splitting?)
> 
> Here's a fun program demonstrating cardinality overflow:
> 
> import java.util.BitSet;
> 
> public class BitSetCardinality {
> public static void main(String[] args) throws Throwable {
> BitSet s = new BitSet();
> s.flip(0, Integer.MAX_VALUE);
> System.out.println(s.cardinality());
> s.flip(Integer.MAX_VALUE);
> System.out.println(s.cardinality());
> }
> }
> 
> 
> ==> java -Xmx20g -esa -ea BitSetCardinality
> 2147483647
> -2147483648
> 

Oh dear!

Thanks,
Paul.


> 
> On Thu, Jun 14, 2018 at 2:35 PM, Paul Sandoz  > wrote:
> Hi,
> 
> Please review this enhancement to improve the performance of BitSet traversal 
> when using a stream.
> 
>   http://cr.openjdk.java.net/~psandoz/jdk/JDK-8170159-bitset-traverse/webrev/ 
>  
>  >
> 
> The associated issue started out life referring to the BitSet’s spliterator 
> reporting SIZED, and to report that the cardinality has to be computed. This 
> has some cost but performance analysis (see issue for attached benchmark) has 
> shown that the cost is small compared to the cost of traversal. It is 
> recognized that the cost is higher for smaller BitSets but not unduly so. 
> Thus it was concluded that it was reasonable for the spliterator to report 
> SIZED. 
> 
> The issue was adjusted to focus on improving the performance of the BitSet’s 
> spliterator forEachRemaining method. For large randomized BitSets a 1.6x 
> speedup can be observed, and for smaller sizes a more modest speed up. The 
> prior conclusion about reporting SIZED still holds.
> 
> Thanks,
> Paul.
> 

Re: JDK-8042131: Proposal of Mapped-values formatter for non-IsoChronology

[naoto . sato]

It does seem to be a good change. I will take a look and sponsor it.

Naoto

On 6/15/18 7:38 AM, Roger Riggs wrote:

Hi,

A good suggestion to reconsider, I re-opened the issue.

I have not looked closely to see if/where the spec needs to be changed.

Thanks, Roger


On 6/15/18 5:30 AM, Stephen Colebourne wrote:

 From the looks of it, this is a perfectly reasonable enhancement.
Sadly I can't sponsor it as I'm not a committer.

Stephen


On 15 June 2018 at 10:10, Toshio 5 Nakamura  wrote:

Hello core-libs and i18n folks,

We'd like to request to reconsider JDK-8042131,
"DateTimeFormatterBuilder Mapped-values do not work for JapaneseDate".
The report was posted by our team long time ago, and was closed as 
not an

issue.
At that time, the feature was for only IsoChronology.

Now, I found the attached patch can activate the mapped-values formatter
for
non-IsoChronology.

Additionally, the Japanese new era will be expected in May 2019. The 
first

year of
each era has a special expression in Japanese, "U+5143 U+5e74" (Gannen).
java.util.JapaneseImperialCalendar uses this expression.
We'd like to use the expression with JapaneseChronology by mapped-values
formatter as an option.

Can we have a sponsor of this proposal?

--sample usage of Japanese new era--
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
Map yearMap = new HashMap<>();
yearMap.put(1L, "\u5143");
builder.appendText(ChronoField.ERA, TextStyle.FULL)
 .appendText(ChronoField.YEAR_OF_ERA, yearMap)
 .appendLiteral("\u5e74");
---

Report:
https://bugs.openjdk.java.net/browse/JDK-8042131

Patch:
--- 
old/src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java  
2018-06-15 17:39:11.489303979 +0900
+++ 
new/src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java  
2018-06-15 17:39:11.157303972 +0900

@@ -793,6 +793,11 @@
  final LocaleStore store = new LocaleStore(map);
  DateTimeTextProvider provider = new DateTimeTextProvider() {
  @Override
+    public String getText(Chronology chrono, TemporalField 
field,
+  long value, TextStyle style, 
Locale locale) {

+    return store.getText(value, style);
+    }
+    @Override
  public String getText(TemporalField field, long value, 
TextStyle style, Locale locale) {

  return store.getText(value, style);
  }
--- 
old/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
2018-06-15 17:39:12.664304007 +0900
+++ 
new/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
2018-06-15 17:39:12.298303999 +0900

@@ -67,14 +67,21 @@
  import java.time.chrono.Chronology;
  import java.time.chrono.IsoChronology;
  import java.time.chrono.JapaneseChronology;
+import java.time.chrono.JapaneseEra;
  import java.time.chrono.MinguoChronology;
+import java.time.chrono.ThaiBuddhistChronology;
+import java.time.chrono.ChronoLocalDate;
  import java.time.format.DateTimeFormatter;
  import java.time.format.DateTimeFormatterBuilder;
  import java.time.format.FormatStyle;
  import java.time.LocalDate;
  import java.time.temporal.Temporal;
+import java.time.temporal.ChronoField;
+import static java.time.temporal.ChronoUnit.YEARS;

  import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;

  import static org.testng.Assert.assertEquals;

@@ -115,6 +122,31 @@
  }

  
//--- 


+    @DataProvider(name="mappedPatterns")
+    Object[][] localizedMappedPatterns() {
+    return new Object[][] {
+    {IsoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
+    {JapaneseChronology.INSTANCE.date(JapaneseEra.HEISEI,
+  1,1,8), Locale.ENGLISH},
+    {MinguoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
+    {ThaiBuddhistChronology.INSTANCE.date(1,1,1), 
Locale.ENGLISH},

+    };
+    }
+
+    @Test(dataProvider="mappedPatterns")
+    public void test_getLocalizedMappedPattern(ChronoLocalDate date, 
Locale locale) {

+    final String new1st = "1st";
+    Map yearMap = new HashMap<>();
+    yearMap.put(1L, new1st);
+    DateTimeFormatterBuilder builder = new 
DateTimeFormatterBuilder();

+    builder.appendText(ChronoField.YEAR_OF_ERA, yearMap);
+
+    String actual = date.format(builder.toFormatter(locale));
+    assertEquals(actual, new1st);
+    }
+
+
+
//--- 


  @DataProvider(name="localePatterns")
  Object[][] localizedDateTimePatterns() {
  return new Object[][] {

---
Best regards,
Toshio Nakamura, IBM Japan

RFR 8199435 : Unsafe publication of java.util.Properties.map

[Brent Christian]

Hi,

In JDK 9, 8029891[1] refactored java.util.Properties to store its values 
in an internal ConcurrentHashMap, and removed synchronization from 
"reader" methods in order to avoid potential hangs/deadlocks during 
classloading.


Claes has noticed that there is the possibility of the new 'map' field 
being observed with its default value (null), before being set.


After looking at the JSR 133 FAQ[2], I agree with Claes that we should 
make 'map' a field final.


Please review my change to do this:

Webrev:
http://cr.openjdk.java.net/~bchristi/8199435/webrev/
Issue:
https://bugs.openjdk.java.net/browse/JDK-8199435

Thanks,
-Brent

1. https://bugs.openjdk.java.net/browse/JDK-8029891
2. https://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html#finalRight

Re: RFR 8170159 Improve the performance of BitSet traversal

[Martin Buchholz]

Code looks correct to me, but as usual there are some things I would try to
do differently,

1292 while (word != 0 && tz < 63) {


I'd try to make this loop simply
while (word != 0)

---

Probably neither of us is fond of the bug magnet special case for
Integer.MAX_VALUE.  Maybe just store fence as a long or as a wordindex
which can't overflow (while giving up on intra-word splitting?)

Here's a fun program demonstrating cardinality overflow:

import java.util.BitSet;

public class BitSetCardinality {
public static void main(String[] args) throws Throwable {
BitSet s = new BitSet();
s.flip(0, Integer.MAX_VALUE);
System.out.println(s.cardinality());
s.flip(Integer.MAX_VALUE);
System.out.println(s.cardinality());
}
}


==> java -Xmx20g -esa -ea BitSetCardinality
2147483647
-2147483648


On Thu, Jun 14, 2018 at 2:35 PM, Paul Sandoz  wrote:

> Hi,
>
> Please review this enhancement to improve the performance of BitSet
> traversal when using a stream.
>
>   http://cr.openjdk.java.net/~psandoz/jdk/JDK-8170159-
> bitset-traverse/webrev/  psandoz/jdk/JDK-8170159-bitset-traverse/webrev/>
>
> The associated issue started out life referring to the BitSet’s
> spliterator reporting SIZED, and to report that the cardinality has to be
> computed. This has some cost but performance analysis (see issue for
> attached benchmark) has shown that the cost is small compared to the cost
> of traversal. It is recognized that the cost is higher for smaller BitSets
> but not unduly so. Thus it was concluded that it was reasonable for the
> spliterator to report SIZED.
>
> The issue was adjusted to focus on improving the performance of the
> BitSet’s spliterator forEachRemaining method. For large randomized BitSets
> a 1.6x speedup can be observed, and for smaller sizes a more modest speed
> up. The prior conclusion about reporting SIZED still holds.
>
> Thanks,
> Paul.

Re: JDK-8042131: Proposal of Mapped-values formatter for non-IsoChronology

[Roger Riggs]

Hi,

A good suggestion to reconsider, I re-opened the issue.

I have not looked closely to see if/where the spec needs to be changed.

Thanks, Roger


On 6/15/18 5:30 AM, Stephen Colebourne wrote:

 From the looks of it, this is a perfectly reasonable enhancement.
Sadly I can't sponsor it as I'm not a committer.

Stephen


On 15 June 2018 at 10:10, Toshio 5 Nakamura  wrote:

Hello core-libs and i18n folks,

We'd like to request to reconsider JDK-8042131,
"DateTimeFormatterBuilder Mapped-values do not work for JapaneseDate".
The report was posted by our team long time ago, and was closed as not an
issue.
At that time, the feature was for only IsoChronology.

Now, I found the attached patch can activate the mapped-values formatter
for
non-IsoChronology.

Additionally, the Japanese new era will be expected in May 2019. The first
year of
each era has a special expression in Japanese, "U+5143 U+5e74" (Gannen).
java.util.JapaneseImperialCalendar uses this expression.
We'd like to use the expression with JapaneseChronology by mapped-values
formatter as an option.

Can we have a sponsor of this proposal?

--sample usage of Japanese new era--
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
Map yearMap = new HashMap<>();
yearMap.put(1L, "\u5143");
builder.appendText(ChronoField.ERA, TextStyle.FULL)
 .appendText(ChronoField.YEAR_OF_ERA, yearMap)
 .appendLiteral("\u5e74");
---

Report:
https://bugs.openjdk.java.net/browse/JDK-8042131

Patch:
--- 
old/src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java  
2018-06-15 17:39:11.489303979 +0900
+++ 
new/src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java  
2018-06-15 17:39:11.157303972 +0900
@@ -793,6 +793,11 @@
  final LocaleStore store = new LocaleStore(map);
  DateTimeTextProvider provider = new DateTimeTextProvider() {
  @Override
+public String getText(Chronology chrono, TemporalField field,
+  long value, TextStyle style, Locale locale) {
+return store.getText(value, style);
+}
+@Override
  public String getText(TemporalField field, long value, TextStyle 
style, Locale locale) {
  return store.getText(value, style);
  }
--- 
old/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
2018-06-15 17:39:12.664304007 +0900
+++ 
new/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
2018-06-15 17:39:12.298303999 +0900
@@ -67,14 +67,21 @@
  import java.time.chrono.Chronology;
  import java.time.chrono.IsoChronology;
  import java.time.chrono.JapaneseChronology;
+import java.time.chrono.JapaneseEra;
  import java.time.chrono.MinguoChronology;
+import java.time.chrono.ThaiBuddhistChronology;
+import java.time.chrono.ChronoLocalDate;
  import java.time.format.DateTimeFormatter;
  import java.time.format.DateTimeFormatterBuilder;
  import java.time.format.FormatStyle;
  import java.time.LocalDate;
  import java.time.temporal.Temporal;
+import java.time.temporal.ChronoField;
+import static java.time.temporal.ChronoUnit.YEARS;

  import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;

  import static org.testng.Assert.assertEquals;

@@ -115,6 +122,31 @@
  }

  //---
+@DataProvider(name="mappedPatterns")
+Object[][] localizedMappedPatterns() {
+return new Object[][] {
+{IsoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
+{JapaneseChronology.INSTANCE.date(JapaneseEra.HEISEI,
+  1,1,8), Locale.ENGLISH},
+{MinguoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
+{ThaiBuddhistChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
+};
+}
+
+@Test(dataProvider="mappedPatterns")
+public void test_getLocalizedMappedPattern(ChronoLocalDate date, Locale 
locale) {
+final String new1st = "1st";
+Map yearMap = new HashMap<>();
+yearMap.put(1L, new1st);
+DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
+builder.appendText(ChronoField.YEAR_OF_ERA, yearMap);
+
+String actual = date.format(builder.toFormatter(locale));
+assertEquals(actual, new1st);
+}
+
+
+//---
  @DataProvider(name="localePatterns")
  Object[][] localizedDateTimePatterns() {
  return new Object[][] {

---
Best regards,
Toshio Nakamura, IBM Japan

Re: RFR: 8204967: Resolve disabled warnings for libunpack

[Jim Laskey]

+1


> On Jun 15, 2018, at 1:45 AM, Srinivas Dama  wrote:
> 
> Hi Magnus/Jim,
> 
> Thank you for the comments.
> Here is the latest webrev with suggested changes:
> webrev: http://cr.openjdk.java.net/~sdama/8204967/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8204967
> 
> Jim,
> gnu c supports -Wimplicit-fallthrough=3.
> clang takes -Wimplicit-fallthrough without any parameters and ignores it 
> without any effect.
> clang++ works with -Wimplicit-fallthrough along with -std=c++11(it is cpp 
> specific option)
> 
> Note: My earlier fix http://cr.openjdk.java.net/~sdama/8204967/webrev.00/ 
> broke linux
> build because some switch case statements which are having 
> implicit-fallthroughs will be enabled
> only in debug build(src/jdk.pack/share/native/common-unpack/unpack.cpp)
> Earlier,I missed to fix those so debug build failed in linux.
> 
> Regards,
> Srinivas
> 
> - Original Message -
> From: magnus.ihse.bur...@oracle.com
> To: srinivas.d...@oracle.com, core-libs-dev@openjdk.java.net
> Sent: Thursday, 14 June, 2018 3:23:23 PM GMT +05:30 Chennai, Kolkata, Mumbai, 
> New Delhi
> Subject: Re: RFR: 8204967: Resolve disabled warnings for libunpack
> 
> On 2018-06-13 18:57, Srinivas Dama wrote:
>> Hi,
>> 
>> Please review http://cr.openjdk.java.net/~sdama/8204967/webrev.00/
>> for https://bugs.openjdk.java.net/browse/JDK-8204967
> 
> Hi Srinivas,
> 
> In src/jdk.pack/share/native/common-unpack/zip.cpp, you just read and 
> discard the return value from fread to hide the warning. But the purpose 
> of the warning is to point out that this kind of code is incorrect. The 
> proper fix would be to check the return code from fread to make sure 
> that the read did not fail. Otherwise you're just making it impossible 
> for the compiler to point out the badly written code, and if you're not 
> going to fix this properly, it's better to keep the warning (that way we 
> know the code is fishy).
> 
> /Magnus
> 
>> 
>> Regards,
>> Srinivas
> 

Re: BiCollector

[Tagir Valeev]

Peter,

> EnumSet intersection =
EnumSet.copyOf(keyCollector.characteristics());
>  intersection.retainAll(valCollector.characteristics());
>  return intersection;

Please note that `copyOf` should either receive an EnumSet or non-empty
Collection to obtain the enum class. This is not guaranteed by
`characteristics()`
implementation, thus failure is possible.

Testcase:
new BiCollector<>(Collectors.counting(),
Collectors.toSet()).characteristics();
Fails with
Exception in thread "main" java.lang.IllegalArgumentException: Collection
is empty
at java.base/java.util.EnumSet.copyOf(EnumSet.java:173)
at BiCollector.characteristics(BiCollector.java:77)
at Main.main(Main.java:21)


With best regards,
Tagir Valeev.

On Mon, Jun 11, 2018 at 7:40 PM Peter Levart  wrote:

> Hi,
>
> Have you ever wanted to perform a collection of the same Stream into two
> different targets using two Collectors? Say you wanted to collect
> Map.Entry elements into two parallel lists, each of them containing keys
> and values respectively. Or you wanted to collect elements into  groups
> by some key, but also count them at the same time? Currently this is not
> possible to do with a single Stream. You have to create two identical
> streams, so you end up passing Supplier to other methods instead
> of bare Stream.
>
> I created a little utility Collector implementation that serves the
> purpose quite well:
>
> /**
>   * A {@link Collector} implementation taking two delegate Collector(s)
> and producing result composed
>   * of two results produced by delegating collectors, wrapped in {@link
> Map.Entry} object.
>   *
>   * @param  the type of elements collected
>   * @param  the type of 1st delegate collector collected result
>   * @param  tye type of 2nd delegate collector collected result
>   */
> public class BiCollector implements Collector Map.Entry, Map.Entry> {
>  private final Collector keyCollector;
>  private final Collector valCollector;
>
>  @SuppressWarnings("unchecked")
>  public BiCollector(Collector keyCollector, Collector V> valCollector) {
>  this.keyCollector = (Collector)
> Objects.requireNonNull(keyCollector);
>  this.valCollector = (Collector)
> Objects.requireNonNull(valCollector);
>  }
>
>  @Override
>  public Supplier> supplier() {
>  Supplier keySupplier = keyCollector.supplier();
>  Supplier valSupplier = valCollector.supplier();
>  return () -> new
> AbstractMap.SimpleImmutableEntry<>(keySupplier.get(), valSupplier.get());
>  }
>
>  @Override
>  public BiConsumer, T> accumulator() {
>  BiConsumer keyAccumulator = keyCollector.accumulator();
>  BiConsumer valAccumulator = valCollector.accumulator();
>  return (accumulation, t) -> {
>  keyAccumulator.accept(accumulation.getKey(), t);
>  valAccumulator.accept(accumulation.getValue(), t);
>  };
>  }
>
>  @Override
>  public BinaryOperator> combiner() {
>  BinaryOperator keyCombiner = keyCollector.combiner();
>  BinaryOperator valCombiner = valCollector.combiner();
>  return (accumulation1, accumulation2) -> new
> AbstractMap.SimpleImmutableEntry<>(
>  keyCombiner.apply(accumulation1.getKey(),
> accumulation2.getKey()),
>  valCombiner.apply(accumulation1.getValue(),
> accumulation2.getValue())
>  );
>  }
>
>  @Override
>  public Function, Map.Entry>
> finisher() {
>  Function keyFinisher = keyCollector.finisher();
>  Function valFinisher = valCollector.finisher();
>  return accumulation -> new AbstractMap.SimpleImmutableEntry<>(
>  keyFinisher.apply(accumulation.getKey()),
>  valFinisher.apply(accumulation.getValue())
>  );
>  }
>
>  @Override
>  public Set characteristics() {
>  EnumSet intersection =
> EnumSet.copyOf(keyCollector.characteristics());
>  intersection.retainAll(valCollector.characteristics());
>  return intersection;
>  }
> }
>
>
> Do you think this class is general enough to be part of standard
> Collectors repertoire?
>
> For example, accessed via factory method Collectors.toBoth(Collector
> coll1, Collector coll2), bi-collection could then be coded simply as:
>
>  Map map = ...
>
>  Map.Entry, List> keys_values =
>  map.entrySet()
> .stream()
> .collect(
> toBoth(
> mapping(Map.Entry::getKey, toList()),
> mapping(Map.Entry::getValue, toList())
> )
> );
>
>
>  Map.Entry, Long> histogram_count =
>  ThreadLocalRandom
>  .current()
>  .ints(100, 0, 10)
>  .boxed()
>  .collect(
>  toBoth(
>  groupingBy(Function.identity(), 

Re: BiCollector

[Tagir Valeev]

Hello!

I created a preliminary webrev of my own implementation (no testcases yet):
http://cr.openjdk.java.net/~tvaleev/patches/pairing/webrev/
If anybody wants to sponsor my implementation, I will happily log an issue
and write tests.

The name "pairing" was invented by me, but as I'm not a native English
speaker I cannot judge whether it's optimal, so better ideas are welcome.
Also I decided to remove accumulator types from public type variables. They
do not add anything to type signature, only clutter it
increasing the number of type parameters from 4 to 6. I think it was a
mistake to expose the accumulator type parameter in other cascading
collectors
like filtering(), collectingAndThen(), groupingBy(), etc. I'm not insisting
though, if you feel that conformance to existing collectors is
more important than simplicity.

With best regards,
Tagir Valeev.

On Fri, Jun 15, 2018 at 5:05 AM Brian Goetz  wrote:

>
> > Well, I don't see the need to pack the two results into a Map.Entry
> > (or any similar) container as a drawback.
>
>  From an "integrity of the JDK APIs" perspective, it is unquestionably a
> drawback.  These items are not a Key and an associated Value in a Map;
> it's merely pretending that Map.Entry really means "Pair".  There's a
> reason we don't have a Pair class in the JDK (and no, let's not reopen
> that now); using something else as a Pair proxy that is supposed to have
> specific semantics is worse. (It's fine to do this in your own code, but
> not in the JDK. Different standards for code that has different audiences.)
>
> Tagir's proposed sidestepping is nice, and it will also play nicely with
> records, because then you can say:
>
>   record NameAndCount(String name, int count);
>
>   stream.collect(pairing(collectName, collectCount,
> NameAndCount::new));
>
> and get a more properly abstract result out.  And its more in the spirit
> of existing Collectors.  If you want to use Map.Entry as an
> _implementation detail_, that's fine.
>
> I can support this form.
>
> > I also don't see a larger abstraction like BiStream as a natural fit
> > for a similar thing.
>
> I think the BiStream connection is mostly tangential.  We tried hard to
> support streams of (K,V) pairs when we did streams, as Paul can attest,
> but it was a huge complexity-inflater and dropping this out paid an
> enormous simplification payoff.
>
> With records, having streams of tuples will be simpler to represent, but
> no more performant; it will take until we get to value types and
> specialized generics to get the performance we want out of this.
>
>
>

Re: JDK-8042131: Proposal of Mapped-values formatter for non-IsoChronology

[Stephen Colebourne]

>From the looks of it, this is a perfectly reasonable enhancement.
Sadly I can't sponsor it as I'm not a committer.

Stephen


On 15 June 2018 at 10:10, Toshio 5 Nakamura  wrote:
>
> Hello core-libs and i18n folks,
>
> We'd like to request to reconsider JDK-8042131,
> "DateTimeFormatterBuilder Mapped-values do not work for JapaneseDate".
> The report was posted by our team long time ago, and was closed as not an
> issue.
> At that time, the feature was for only IsoChronology.
>
> Now, I found the attached patch can activate the mapped-values formatter
> for
> non-IsoChronology.
>
> Additionally, the Japanese new era will be expected in May 2019. The first
> year of
> each era has a special expression in Japanese, "U+5143 U+5e74" (Gannen).
> java.util.JapaneseImperialCalendar uses this expression.
> We'd like to use the expression with JapaneseChronology by mapped-values
> formatter as an option.
>
> Can we have a sponsor of this proposal?
>
> --sample usage of Japanese new era--
> DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
> Map yearMap = new HashMap<>();
> yearMap.put(1L, "\u5143");
> builder.appendText(ChronoField.ERA, TextStyle.FULL)
> .appendText(ChronoField.YEAR_OF_ERA, yearMap)
> .appendLiteral("\u5e74");
> ---
>
> Report:
> https://bugs.openjdk.java.net/browse/JDK-8042131
>
> Patch:
> --- 
> old/src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
>   2018-06-15 17:39:11.489303979 +0900
> +++ 
> new/src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
>   2018-06-15 17:39:11.157303972 +0900
> @@ -793,6 +793,11 @@
>  final LocaleStore store = new LocaleStore(map);
>  DateTimeTextProvider provider = new DateTimeTextProvider() {
>  @Override
> +public String getText(Chronology chrono, TemporalField field,
> +  long value, TextStyle style, Locale 
> locale) {
> +return store.getText(value, style);
> +}
> +@Override
>  public String getText(TemporalField field, long value, TextStyle 
> style, Locale locale) {
>  return store.getText(value, style);
>  }
> --- 
> old/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
> 2018-06-15 17:39:12.664304007 +0900
> +++ 
> new/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
> 2018-06-15 17:39:12.298303999 +0900
> @@ -67,14 +67,21 @@
>  import java.time.chrono.Chronology;
>  import java.time.chrono.IsoChronology;
>  import java.time.chrono.JapaneseChronology;
> +import java.time.chrono.JapaneseEra;
>  import java.time.chrono.MinguoChronology;
> +import java.time.chrono.ThaiBuddhistChronology;
> +import java.time.chrono.ChronoLocalDate;
>  import java.time.format.DateTimeFormatter;
>  import java.time.format.DateTimeFormatterBuilder;
>  import java.time.format.FormatStyle;
>  import java.time.LocalDate;
>  import java.time.temporal.Temporal;
> +import java.time.temporal.ChronoField;
> +import static java.time.temporal.ChronoUnit.YEARS;
>
>  import java.util.Locale;
> +import java.util.Map;
> +import java.util.HashMap;
>
>  import static org.testng.Assert.assertEquals;
>
> @@ -115,6 +122,31 @@
>  }
>
>  //---
> +@DataProvider(name="mappedPatterns")
> +Object[][] localizedMappedPatterns() {
> +return new Object[][] {
> +{IsoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
> +{JapaneseChronology.INSTANCE.date(JapaneseEra.HEISEI,
> +  1,1,8), Locale.ENGLISH},
> +{MinguoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
> +{ThaiBuddhistChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
> +};
> +}
> +
> +@Test(dataProvider="mappedPatterns")
> +public void test_getLocalizedMappedPattern(ChronoLocalDate date, Locale 
> locale) {
> +final String new1st = "1st";
> +Map yearMap = new HashMap<>();
> +yearMap.put(1L, new1st);
> +DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
> +builder.appendText(ChronoField.YEAR_OF_ERA, yearMap);
> +
> +String actual = date.format(builder.toFormatter(locale));
> +assertEquals(actual, new1st);
> +}
> +
> +
> +//---
>  @DataProvider(name="localePatterns")
>  Object[][] localizedDateTimePatterns() {
>  return new Object[][] {
>
> ---
> Best regards,
> Toshio Nakamura, IBM Japan

JDK-8042131: Proposal of Mapped-values formatter for non-IsoChronology

[Toshio 5 Nakamura]

Hello core-libs and i18n folks,

We'd like to request to reconsider JDK-8042131,
"DateTimeFormatterBuilder Mapped-values do not work for JapaneseDate".
The report was posted by our team long time ago, and was closed as not an
issue.
At that time, the feature was for only IsoChronology.

Now, I found the attached patch can activate the mapped-values formatter
for
non-IsoChronology.

Additionally, the Japanese new era will be expected in May 2019. The first
year of
each era has a special expression in Japanese, "U+5143 U+5e74" (Gannen).
java.util.JapaneseImperialCalendar uses this expression.
We'd like to use the expression with JapaneseChronology by mapped-values
formatter as an option.

Can we have a sponsor of this proposal?

--sample usage of Japanese new era--
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
Map yearMap = new HashMap<>();
yearMap.put(1L, "\u5143");
builder.appendText(ChronoField.ERA, TextStyle.FULL)
.appendText(ChronoField.YEAR_OF_ERA, yearMap)
.appendLiteral("\u5e74");
---

Report:
https://bugs.openjdk.java.net/browse/JDK-8042131

Patch:
--- 
old/src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java  
2018-06-15 17:39:11.489303979 +0900
+++ 
new/src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java  
2018-06-15 17:39:11.157303972 +0900
@@ -793,6 +793,11 @@
 final LocaleStore store = new LocaleStore(map);
 DateTimeTextProvider provider = new DateTimeTextProvider() {
 @Override
+public String getText(Chronology chrono, TemporalField field,
+  long value, TextStyle style, Locale locale) {
+return store.getText(value, style);
+}
+@Override
 public String getText(TemporalField field, long value, TextStyle 
style, Locale locale) {
 return store.getText(value, style);
 }
--- 
old/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
2018-06-15 17:39:12.664304007 +0900
+++ 
new/test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
2018-06-15 17:39:12.298303999 +0900
@@ -67,14 +67,21 @@
 import java.time.chrono.Chronology;
 import java.time.chrono.IsoChronology;
 import java.time.chrono.JapaneseChronology;
+import java.time.chrono.JapaneseEra;
 import java.time.chrono.MinguoChronology;
+import java.time.chrono.ThaiBuddhistChronology;
+import java.time.chrono.ChronoLocalDate;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeFormatterBuilder;
 import java.time.format.FormatStyle;
 import java.time.LocalDate;
 import java.time.temporal.Temporal;
+import java.time.temporal.ChronoField;
+import static java.time.temporal.ChronoUnit.YEARS;

 import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;

 import static org.testng.Assert.assertEquals;

@@ -115,6 +122,31 @@
 }

 //---
+@DataProvider(name="mappedPatterns")
+Object[][] localizedMappedPatterns() {
+return new Object[][] {
+{IsoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
+{JapaneseChronology.INSTANCE.date(JapaneseEra.HEISEI,
+  1,1,8), Locale.ENGLISH},
+{MinguoChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
+{ThaiBuddhistChronology.INSTANCE.date(1,1,1), Locale.ENGLISH},
+};
+}
+
+@Test(dataProvider="mappedPatterns")
+public void test_getLocalizedMappedPattern(ChronoLocalDate date, Locale 
locale) {
+final String new1st = "1st";
+Map yearMap = new HashMap<>();
+yearMap.put(1L, new1st);
+DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
+builder.appendText(ChronoField.YEAR_OF_ERA, yearMap);
+
+String actual = date.format(builder.toFormatter(locale));
+assertEquals(actual, new1st);
+}
+
+
+//---
 @DataProvider(name="localePatterns")
 Object[][] localizedDateTimePatterns() {
 return new Object[][] {

---
Best regards,
Toshio Nakamura, IBM Japan

Re: Add x-IBM-1129 charset

[Ichiroh Takiguchi]

Hello.

I tested IBM-1129 charset on AIX platform.
It worked fine for default encoding,
$ LANG=Vi_VN ~/jdk/bin/java PrintDefaultCharset
Vi_VN   x-IBM1129   IBM-1129IBM-1129
$

And also code table is fine.

We would appreciate to open a bug/RFE for this change request.

Ichiroh Takiguchi
IBM Japan, Ltd.

On 2018-05-30 10:41, Nasser Ebrahim wrote:

Hello,

I just realized that I missed to provide the details of the test I have
conducted for the new charset IBM-1129. Please note that I have done 
the

following jtreg tests to make sure the new charset is working properly.

sun.nio.cs.TestCharsetMapping
java.nio.charset.Charset.RegisteredCharsets
sun.nio.cs.CheckHistoricalNamesTest
java.nio.charset.RemovingSunIO.SunioAlias

Please let me know if I have to run any additional tests.

Am aware of the suggestion from Alan and Thomas in the mail thread
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-May/053316.html
to move the IBM platform specific charsets to a separate module instead 
of
adding to jdk.charsets.  We will open another thread to discuss that 
topic

as we would like to contribute many more extended charsets. However, I
believe we can still consider integrating this charset without waiting 
for

the conclusion of that discussion as this charset is part of default
charset for AIX and needs to be part of the java.base. If we decided to
move the IBM charsets out of jdk.charsets, we can move this charsets as
well for non-AIX platforms.

Kindly request you to open a bug/RFE for this change request, review 
the

fix and provide your feedback.

Thank you,
Nasser Ebrahim


From:   Nasser Ebrahim/India/IBM
To: core-libs-dev@openjdk.java.net
Date:   05/19/2018 12:51 PM
Subject:Add  x-IBM-1129 charset



Hello,

With the following three bugs, all the default locale charsets except 
two

(Vi_VN.IBM-1129 & ja_JP.IBM-eucJP) are fixed for AIX platform.

- JDK-8201540: [AIX] Extend the set of supported charsets in java.base
- JDK-8202329: Codepage mappings for IBM-943 and Big5 (aix)
-
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-May/053050.html
: [AIX] Add charset IBM-964 (default charset for zh_TW.IBM-eucTW) to 
stdcs

[bug not yet opened].

For those fixed charsets, the charsets were existing in the extended
charsets (jdk.charsets) and they were not working with default locale
charset as it did not exist in the standard charset (java.base). The
charsets correspond to the two pending locale (Vi_VN.IBM-1129 &
ja_JP.IBM-eucJP) does not exist in the jdk. They need to be added to 
the

extended charsets before adding to stdcs on AIX platform.

Here, am including the patch to fix  the charset IBM-1129 for the 
locale

Vi_VN.IBM-1129. We are working on the other missing charset (for
ja_JP.IBM-eucJP) which will be contributed in some time.

The webrev of the fix is available at
http://cr.openjdk.java.net/~aleonard/IBM1129/webrev.00/

Kindly request you to open a bug and review the fix. Please let me know 
if

you have any questions.

Thank you,
Nasser Ebrahim

Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

[Remi Forax]

- Mail original -
> De: "Stuart Marks" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 15 Juin 2018 03:02:04
> Objet: RFR(s): 8060192: Add default method Collection.toArray(generator)

> Hi all,

Hi Stuart,

> 
> I wanted to restart review of the changeset for bug JDK-8060192 [1]. The 
> latest
> webrev is here: [2].
> 
> The previous review was from December 2017: [3]. The only difference between 
> the
> current changeset and the previous one is the removal of the overriding
> implementations (see explanation below). Much of the discussion in the 
> previous
> review thread was around performance and the shape of the API.
> 
> # Performance
> 
> I previously had done some benchmarking on this and reported the results: [4].
> (I've recently re-done the benchmarks and conclusions are essentially the 
> same.)
> 
> The upshot is that implementations that use Arrays.copyOf() are the fastest,
> probably because it's an intrinsic. It can avoid zero-filling the freshly
> allocated array because it knows the entire array contents will be 
> overwritten.
> This is true regardless of what the public API looks like. The default
> implementation calls generator.apply(0) to get a zero-length array and then
> simply calls toArray(T[]). This goes through the Arrays.copyOf() fast path, so
> it's essentially the same speed as toArray(new T[0]).
> 
> The overrides I had previously provided in specific implementation classes 
> like
> ArrayList actually are slower, because the allocation of the array is done
> separately from filling it. This necessitates the extra zero-filling step. 
> Thus,
> I've removed the overrides.


for ArrayList, you may use a code like this,
   T[] toArray(IntFunction generator) {
return (T[]) Arrays.copyOf(elementData, size, 
generator.apply(0).getClass());
  }
so you win only one comparison (yeah !), which can be perfectly predicted, so 
you should not see any perf difference :)

> 
> # API Shape
> 
> There was some discussion about whether it would be preferable to have a Class
> parameter as a type token for the array component type or the array type 
> itself,
> instead of using an IntFunction generator. Most of this boils down to what
> people are comfortable with. However, there are a few points that make the
> generator function approach preferable.
> 
> One point in favor of using a generator is that Stream already has a similar
> toArray(generator) method.
> 
> Comparing this to the type token alternatives, for simple tasks like 
> converting
> Collection to String[], things are about equal:
> 
> toArray(String[]::new)
> toArray(String.class)
> toArray(String[].class)
> 
> However, things are hairier if the element type of the collection is generic,
> such as Collection>. The result type is a generic array
> List[]. Using class literals or array constructor references will
> necessitate using an unchecked cast, because none of these can be used to
> represent a generic type.
> 
> However, it's possible to use a method reference to provide a properly generic
> IntFunction. This would enable the toArray(generator) method to be used 
> without
> any unchecked casts. (The method it refers to might need have an unchecked 
> cast
> within it, though.) Such a method could also be reused for the corresponding
> Stream.toArray(generator) method.

List.class or List[].class do not work either.

> 
> For these reasons I'd like to proceed with adding toArray(generator) API.
> 

so thumb up for me !

> Thanks,
> 
> s'marks

Rémi

> 
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8060192
> 
> [2] http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.3/
> 
> [3]
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050325.html
> 
> [4]
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050585.html

Re: RFR JDK-8204930 Reader:nullReader() spec does not match the behavior

[Patrick Reinhart]

On 2018-06-14 13:03, Alan Bateman wrote:

On 14/06/2018 11:50, Patrick Reinhart wrote:

Hi everybody,

I re-proposed the CSR with the changed API.

The CSR is associated with the original change-set that was pushed a
few months so the simplest is to just create a follow-up CSR that is
linked to the new bug.


I created a follow-up CSR with relation to the existing one now.

In the meantime I got a comment on the initial issue according the 
specification of the  [ ready(), skip(), transferTo() ] where it seen to 
be unclear what is the expected behavior "if end of stream reached" 
exactly means.


Any suggestions on that part? Should it state that ready() will always 
return false, skip() and transferTo() always return zero?


-Patrick