Re: RFR: 8281631: HashMap.putAll can cause redundant space waste

[stefan-zobel]

On Thu, 10 Feb 2022 18:09:19 GMT, XenoAmess  wrote:

> I investigated most of the usages. They just give a size, and get a capacity, 
> even not change the 0.75 So maybe we can use some int calculation to replace 
> the 0.75, thus replace Math.ceil for such situations.

FWIW, `(int) Math.ceil(expected / 0.75)` and `(int) ((expected * 4L + 2L) / 
3L)` would be equivalent.

-

PR: https://git.openjdk.java.net/jdk/pull/7431

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v42]

[stefan-zobel]

On Mon, 5 Apr 2021 14:20:56 GMT, Jim Laskey  wrote:

>> This PR is to introduce a new random number API for the JDK. The primary API 
>> is found in RandomGenerator and RandomGeneratorFactory. Further description 
>> can be found in the JEP https://openjdk.java.net/jeps/356 .
>> 
>> javadoc can be found at 
>> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html
>> 
>> old PR:  https://github.com/openjdk/jdk/pull/1273
>
> Jim Laskey has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 78 commits:
> 
>  - Merge branch 'master' into 8248862
>  - Fix NotCompliantCauseTest to not rely on Random not being a bean
>  - Merge branch 'master' into 8248862
>  - Correct return type of RandomGeneratorFactory.of
>  - Merge branch 'master' into 8248862
>  - CSR review revisions
>  - CSR review updates
>  - Removed @since from nextDouble ThreadLocalRandom
>  - RandomGeneratorFactory::all(Class category) @implNote was out of date
>  - Clarify all()
>  - ... and 68 more: 
> https://git.openjdk.java.net/jdk/compare/a8005efd...ffd982b0

The package javadoc of java.util.random says that `mixLea32` is used as a 
mixing function in the L64 and L128 generators which doesn't seem to be 
correct. That should probably read `mixLea64`

-

PR: https://git.openjdk.java.net/jdk/pull/1292

Re: better random numbers

[Stefan Zobel]

>
> On this blog entry (year 2017), Lemire is not giving any technical or
> scientific argument in favor or against PCG.
>
> He also refers to, and quotes from, a blog entry (year 2015) of an
> influential researcher (whose work he respects) suggesting the entry has
> harsh words about PCG. The fact is, that entry doesn't mention PCG or
> O'Neill at all and the quotation if not found there.
>

That "influential researcher" is probably Sebastiano Vigna who has indeed
harsh words on PCG: https://pcg.di.unimi.it/pcg.php

Re: RFR: 8214761: Bug in parallel Kahan summation implementation

[stefan-zobel]

On Fri, 2 Jul 2021 20:30:24 GMT, Andrew Haley  wrote:

> Crikey, how did we get that wrong?
> It'd be nice if we had a regression test for this. Can you provide one, 
> please?

I found this: 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057239.html
Ivan Gerasimov already tackled this back then. His webrev still exists and it 
contains a simple regression test.

-

PR: https://git.openjdk.java.net/jdk/pull/4674

Re: RFR: 8214761: Bug in parallel Kahan summation implementation

[stefan-zobel]

On Fri, 2 Jul 2021 20:12:39 GMT, Ian Graves  wrote:

> 8214761: Bug in parallel Kahan summation implementation

What about `Collectors.computeFinalSum()` - should this be `double tmp = 
summands[0] + summands[1];` or `double tmp = summands[0] - summands[1];` ?

-

PR: https://git.openjdk.java.net/jdk/pull/4674

Re: RFR: 8214761: Bug in parallel Kahan summation implementation

[stefan-zobel]

On Fri, 2 Jul 2021 20:12:39 GMT, Ian Graves  wrote:

> 8214761: Bug in parallel Kahan summation implementation

src/java.base/share/classes/java/util/DoubleSummaryStatistics.java line 161:

> 159: 
> 160: //Negating this value because low-order bits are in negated form
> 161: sumWithCompensation(-other.sumCompensation);

Wouldn't that be `double tmp =  sum - sumCompensation;` in getSum() in line 246 
too?

-

PR: https://git.openjdk.java.net/jdk/pull/4674

Re: RFR: JDK-8261422: Adjust problematic String.format calls in jdk/internal/util/Preconditions.java outOfBoundsMessage

[stefan-zobel]

On Tue, 9 Feb 2021 15:47:39 GMT, Matthias Baesken  wrote:

>> src/java.base/share/classes/jdk/internal/util/Preconditions.java line 212:
>> 
>>> 210:  args.get(0), args.get(1), 
>>> args.get(2));
>>> 211: case "checkFromIndexSize":
>>> 212: return String.format("Range [%s, %>> bounds for length %s",
>> 
>> I assume we have a code coverage here and that we need tests to exercise 
>> these code paths.
>
> I haven't written the class Preconditions.java, this question should probably 
> go to the authors .
> I found the String.format syntax a bit  unusual,  should this be rewritten ?

AFAIK, Preconditions was introduced in 
https://bugs.openjdk.java.net/browse/JDK-8155794

-

PR: https://git.openjdk.java.net/jdk/pull/2483

Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message

[stefan-zobel]

On Fri, 4 Dec 2020 06:50:14 GMT, Stuart Marks  wrote:

> This rewrites the doc of ArraysSupport.newLength, adds detail to the 
> exception message, and adds a test. In addition to some renaming and a bit of 
> refactoring of the actual code, I also made two changes of substance to the 
> code:
> 
> 1. I fixed a problem with overflow checking. In the original code, if 
> oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this 
> method could return a negative value. It turns out that writing tests helps 
> find bugs!
> 
> 2. Under the old policy, if oldLength and minGrowth required a length above 
> SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would 
> return Integer.MAX_VALUE. That doesn't make any sense, because attempting to 
> allocate an array of that length will almost certainly cause the Hotspot to 
> throw OOME because its implementation limit was exceeded. Instead, if the 
> required length is in this range, this method returns that required length.
> 
> Separately, I'll work on retrofitting various call sites around the JDK to 
> use this method.

src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 626:

> 624:  * attempt an array allocation with that length and encounter an 
> OutOfMemoryError.
> 625:  * Of course, regardless of the length value returned from this 
> method, the caller
> 626:  * may encounter OutOfMemoryError if there is insufficient heap to 
> fufill the request.

typo in line 626: fufill -> fulfill

-

PR: https://git.openjdk.java.net/jdk/pull/1617

Re: RFR: 8235730: Incorrect javadoc in MatchKind

[Stefan Zobel]

FWIW, I believe this bug is duplicate of
https://bugs.openjdk.java.net/browse/JDK-8213238

Regards,
Stefan

Am Mi., 11. Dez. 2019 um 17:07 Uhr schrieb Arthur Eubanks :
>
> Hi, could I get a review for a trivial and tiny doc fix?
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8235730
> Webrev: http://cr.openjdk.java.net/~aeubanks/8235730/webrev.00/

Re: RFR 8190974 Parallel stream execution within a custom ForkJoinPool should obey the parallelism

[Stefan Zobel]

In CustomFJPoolTest#testCustomPools()

>>assertEquals(splitsForPC, splitsForPHalfC * 2);


I'm sure I'm slow on the uptake, but isn't this bound to
fail for every commonParallelism == 2^n + 1 in the closed
range [2, 127], i.e., for 3, 5, 9, 17, 33, 65?

Regards,
Stefan


2017-11-08 22:01 GMT+01:00 Paul Sandoz :
> Hi,
>
> Please review this patch to ensure that a parallel stream obeys the 
> parallelism of a custom fork join pool when it is executed within that pool:
>
>   
> http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8190974-par-stream-custom-pool/webrev/
>  
> 
>
> Streams currently do not support capabilities to control the level of 
> parallelism and therefore resources utilised (tricky API design problem to 
> get right).
>
> At the moment the trick is to run stream executions within a custom pool, 
> however the number of fork join tasks created will be in proportion to the 
> parallelism of the common pool thus the execution will be over-provisioned. 
> This can be especially noticeable on large machines with many cores.
>
> Paul.

Re: RFR(m): 8177290 add copy factory methods for unmodifiable List, Set, Map

[Stefan Zobel]

Hi Stuart,

only a minor nit. In Map.java, the class-level
Javadoc anchor id="immutable" doesn't match the
href="#unmodifiable" used in the methods.

+ * Unmodifiable Maps

vs.

+ * See Unmodifiable Maps for details.


Regards,
Stefan


2017-11-01 0:49 GMT+01:00 Stuart Marks :
> Updated webrev, based on comments from Brian and Roger:
>
> http://cr.openjdk.java.net/~smarks/reviews/8177290/webrev.2/
>
> s'marks
>
>
>
> On 10/30/17 3:50 PM, Stuart Marks wrote:
>>
>> (also includes 8184690: add Collectors for collecting into unmodifiable
>> List, Set, and Map)
>>
>> Hi all,
>>
>> Here's an updated webrev for this changeset; the previous review thread is
>> here:
>>
>>
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-September/049261.html
>>
>> This webrev includes the following:
>>
>> * specification revisions to provide clearer definitions of "view"
>> collections, "unmodifiable" collections, and "unmodifiable views"
>>
>> * new List.copyOf(), Set.copyOf(), and Map.copyOf() "copy factory" methods
>>
>> * new Collectors.toUnmodifiableList, Set, and Map methods
>>
>> * tests for the new API methods
>>
>> I've added some assertions that require some independence between the
>> source collection (or map) and the result of the copyOf() method.
>>
>> I've made a small but significant change to Set.copyOf compared to the
>> previous round. Previously, it specified that the first of any equal
>> elements was preserved. Now, it is explicitly unspecified which of any
>> equals elements is preserved. This is consistent with Set.addAll,
>> Collectors.toSet, and the newly added Collectors.toUnmodifiableSet, none of
>> which specify which of duplicate elements is preserved.
>>
>> (The outlier here is Stream.distinct, which specifies that the first
>> element of any duplicates is preserved, if the stream is ordered.)
>>
>> I've also made some minor wording/editorial changes in response to
>> suggestions from David Holmes and Roger Riggs. I've kept the wording changes
>> that give emphasis to "unmodifiable" over "immutable." The term "immutable"
>> is inextricably intertwined with "persistent" when it comes to data
>> structures, and I believe we'll be explaining this forever if Java's
>> "immutable" means something different from everybody else's.
>>
>> Webrev:
>>
>>  http://cr.openjdk.java.net/~smarks/reviews/8177290/webrev.1/
>>
>> Bugs:
>>
>>  https://bugs.openjdk.java.net/browse/JDK-8177290
>>  add copy factory methods for unmodifiable List, Set, Map
>>
>>  https://bugs.openjdk.java.net/browse/JDK-8184690
>>  add Collectors for collecting into unmodifiable List, Set, and
>> Map
>>
>> Thanks,
>>
>> s'marks

Re: RFR: jsr166 jdk9 integration wave 14

[Stefan Zobel]

Hi Martin,

minor thing in CopyOnWriteArrayListTest.java, line 57


55  public static Test suite() {
56  class Implementation implements CollectionImplementation {
57  public Class klazz() { return ArrayList.class; }

Shouldn't this be CopyOnWriteArrayList.class?


Regards,
Stefan

Re: RFR: jsr166 jdk9 integration wave 13

[Stefan Zobel]

2016-12-16 20:58 GMT+01:00 Martin Buchholz :
> Thanks, Stefan!
>
> The creation of these bridge classes seem like a misfeature/bug of javac.
> We should avoid them where possible.  Every unnecessary class file makes
> every single java program a little slower to start up.  We can discover such
> "bridge classes" heuristically by looking for "small" synthetic class files:
>
> find build/classes/java.base/ -name '*$[0-9].class' -size -800c
>
> I think adding the package-private constructor is slightly better software
> engineering than making the classes themselves package-private:
>

Hi Martin,

yes, I agree. It may be better to add a comment like in ArrayList.Itr:

// prevent creating a synthetic constructor

Otherwise someone might be tempted to drop these empty constructors.

See also
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-September/043803.html


Regards,
Stefan

Re: RFR: jsr166 jdk9 integration wave 13

[Stefan Zobel]

> 2016-12-15 18:35 GMT+01:00 Doug Lea :
>
> Thanks. Changed.
> Using s1 on that line wasn't wrong, but wasn't intentional.
> (The first use of s1 catches error-before-subscribe; the
> "s2" cases just check against side effects, but they should
> use s2 consistently for uniformity.)
>
> -Doug
>

Ah, I see, thanks. Something else (unrelated) about SubmissionPublisher:

I recently noticed that javac creates a synthetic class and a bridge constructor
for SubmissionPublisher.ThreadPerTaskExecutor because its generated constructor
is private. I don't know if it really matters but that could be avoided by
declaring a package-private constructor that does nothing.

>/** Fallback if ForkJoinPool.commonPool() cannot support parallelism */
>private static final class ThreadPerTaskExecutor implements Executor {
>// avoid creation of synthetic class and bridge constructor
>ThreadPerTaskExecutor() {}
>public void execute(Runnable r) { new Thread(r).start(); }
>}


Regards,
Stefan

Re: RFR: jsr166 jdk9 integration wave 13

[Stefan Zobel]

SubmissionPublisherTest, line 420: "s1.awaitSubscribe();"

I might be wrong, shouldn't this be "s2.awaitSubscribe();"?


Regards,
Stefan


2016-12-14 0:13 GMT+01:00 Martin Buchholz :
> We were supposed to be winding down jdk9, and there are a lot of changes
> here, but ... it would be a shame not to have fast specialized
> implementations for new collection methods added in Java 8.  There's also a
> fix for a serious bug in LinkedBlockingQueue.
>
> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/
>
> As with wave 12, very collection oriented.

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stefan Zobel]

>
> Turns out that Rémi's example exposes the difference between the wildcard
> approach and the type-parameter approach. Returning to the example,
>
> Optional oi = Optional.empty();
> Function fm = n -> Optional.empty();
> Optional ocs = oi.flatMap(fm);
>
> If the flatMapper function itself has a wildcard type, for example,
>
> Function fm = n ->
> Optional.empty();
>
> then this will still work with the wildcard approach but fail with the
> type-parameter approach. As Rémi also pointed out, a wildcarded type can
> result from the capture of a type with a wildcarded type parameter.
>
> Based on this, I believe the nested wildcard approach to be the correct one.
>
> s'marks
>

Yes, this argument is unchallengeable.
I'm convinced now - though not happy.

Kind regards,
Stefan

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stefan Zobel]

Hi Stuart,

thanks for explaining.

> ... After having looked at lots of generic APIs, it seems to me that a
> style has emerged where wildcards are used whenever possible, and type
> parameters are used only when necessary, ...

Yes, I'm familiar with that kind of reasoning (I think Josh Bloch stated
that principle in "Effective Java"). But, to be honest, I never thought
that it should apply as a strict rule in all circumstances.

Rhetorical question: do you really think that a signature employing 3
wildcards is easier to understand for the proverbial "average Joe" than
a bounded type parameter that expresses the sub-type relationship clearly?
I do not.

But anyway, you probably have to follow the established "style".

It's just too bad that most Java programmers I know won't understand the
proposed signature of 'flatMap'.

Kind regards,
Stefan

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stefan Zobel]

2016-10-07 21:42 GMT+02:00 Michael Nascimento :
> Doesn't work, as Stuart has noted (s'marks, as far as I know your
> explanation is 100% correct). Nested generics == pain :-(

Hi Michael,

sorry for being obtrusive. What exactly doesn't work?

Stuart's example

Optional oi = Optional.empty();
Function fm = n -> Optional.empty();
Optional ocs = oi.flatMap(fm);
System.out.println(ocs.orElse("empty"));

work's for me (on 1.8.0_51). Sorry, I'm just trying to understand.

Kind regards,
Stefan


>
> Regards,
> Michael
>

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stefan Zobel]

2016-10-07 21:22 GMT+02:00 Stuart Marks <stuart.ma...@oracle.com>:
>
>
> On 10/7/16 11:23 AM, Paul Sandoz wrote:
>>>
>>>flatMap(Function> mapper)
>>
>>
>> Optional is final so why do you need to express “? extends Optional” ?
>
>
> The short answer is, it doesn't work if you don't have it. :-)
>
> The theoretical answer is that in this context, "? extends P" means "some
> subtype of P" and not necessarily just a subclass of P.
>
> (And even though Optional is final, it's not "permanently final" in that a
> hypothetical future version of the class library might change it to
> non-final, allowing subclasses.)
>
> This issue is covered in Angelika Langer's Generics FAQ entry, "What do
> multi-level (i.e., nested) wildcards mean?" [1] Note that the answer begins,
> "It depends." :-) I also note in passing that I've read this about five
> times and I'm still not quite sure I understand it entirely.
>
> For me, the best explanation comes from looking at examples. First, the
> history is that the signature in Java 8 is:
>
>   #1flatMap(Function> mapper)
>
> I believe Rémi originally proposed something like this (although it was
> about or(), the same issue applies to flatMap()):
>
>   #2flatMap(Function> mapper)
>
> The suggested fix that ended up in bug report was this:
>
>   #3flatMap(Function> mapper)
>
> But this doesn't work for reasons I explain below. Finally, I'm proposing:
>
>   #4flatMap(Function> mapper)
>
> In researching old email threads and chasing links, I ran across an example
> from Stefan Zobel [2] that fails with #3. He had an alternative that didn't
> seem quite right to me, so I ended up with #4.
>
> I've adapted Stefan's example as follows:
>
> Optional oi = Optional.empty();
> Function<Number, Optional> fm = n ->
> Optional.empty();
> Optional ocs = oi.flatMap(fm);
>
> The flatmapper function 'fm' returns Optional. In the
> assignment on the last line, U is CharSequence, so we can compare this to
> the various signatures shown above with U filled in.
>
> Case #1 fails because Optional is incompatible with
> Optional. This is the usual "generics are invariant thing".
> Even though SB is a subtype of CS, Optional isn't a subtype of
> Optional.
>
> Case #2 fails because adding a wildcard there doesn't help matters, since
> Optional is still unrelated to Optional.
>
> Now for the tricky part. :-)
>
> Surely case #3 should work, because adding an inner wildcard provides a
> subtyping relationship, so Optional is a subtype of Optional CS>.
>
> That much is true, but these are nested within the Function<> generic type,
> so the "generics are invariant" rule applies again. Thus,
>
> Function<..., Optional>
>
> is not a subtype of
>
> Function<..., Optional>
>
> To get around this, we have to add the outer wildcard as well, so that
>
> Function<..., Optional>
>
> is a subtype of
>
> Function<..., ? extends Optional>
>
> So that's what ended up in the signature.
>
> Similar analysis applies to the or() case.
>
> Now awaiting a message from Rémi telling me my explanation is incorrect in
> 3... 2... 1... :-) :-)
>
> s'marks
>
>
>
> [1]
> http://angelikalanger.com/GenericsFAQ/FAQSections/TypeArguments.html#What%20do%20multilevel%20wildcards%20mean?
>
> [2] https://sourceforge.net/p/streamsupport/tickets/125/#2d90
>



Hhm, that's really mind-boggling!

What's wrong with the alternative "additional bounded type parameter" approach?

Couldn't we also get by with something like

<V, U extends V> Optional flatMap(Function> mapper)


and


 Optional or(Supplier<Optional> supplier)


Personally, I find that much easier to digest. But that's only me, of course.


Regards,
Stefan

Re: RFR 8164691 Stream specification clarifications for iterate and collect

[Stefan Zobel]

2016-09-07 1:22 GMT+02:00 Paul Sandoz <paul.san...@oracle.com>:
>
>> On 3 Sep 2016, at 15:06, Stefan Zobel <splitera...@gmail.com> wrote:
>>
>> Hi Paul,
>>
>> there's a small copy & paste error in the code samples in
>> Double/Int/LongStream#iterate()
>>
>> Example DoubleStream#iterate:
>>
>> * {@code
>> * for (T index=seed; hasNext.test(index); index = next.apply(index)) 
>> {
>> * ...
>> * }
>> * }
>>
>> That should be
>>
>> * for (double index=seed; hasNext.test(index); index =
>> next.applyAsDouble(index)) {
>>
>>
>> Same for Int/LongStream#iterate()
>>
>
> Thanks, well spotted, fixed in place.


Almost: next.apply(index) => next.applyAsDouble(index)


>
> When can we have generics over primitives? :-)
>
> Paul.

Regards,
Stefan

Re: RFR 8164691 Stream specification clarifications for iterate and collect

[Stefan Zobel]

Hi Paul,

there's a small copy & paste error in the code samples in
Double/Int/LongStream#iterate()

Example DoubleStream#iterate:

 * {@code
 * for (T index=seed; hasNext.test(index); index = next.apply(index)) {
 * ...
 * }
 * }

That should be

 * for (double index=seed; hasNext.test(index); index =
next.applyAsDouble(index)) {


Same for Int/LongStream#iterate()



Regards,
Stefan


2016-08-27 1:13 GMT+02:00 Paul Sandoz :
> Hi,
>
> Please review some minor tweaks to the stream specification:
>
>   
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8164691-stream-iterate-collect-spec-updates/webrev/index.html
>  
> 
>
> The first tweak is to clarify the iterate methods and HB edges between 
> function calls, the functions could potentially be stateful, they will never 
> be called concurrently due to the nature of the source, but may be called in 
> different threads.
>
> The second tweak is to the three-arg collect method. The combiner of result 
> containers neglected to state how result containers should be merged.
>
> Thanks,
> Paul.

Re: RFR: 8154387 - Parallel unordered Stream.limit() tries to collect 128 elements even if limit is less

[Stefan Zobel]

2016-04-18 14:01 GMT+02:00 Tagir F. Valeev :
> Hello!
>
> It was just a quick test not in the clean environment, so you should
> not draw any conclusions from the error numbers. It's quite expected
> that for limit = 2000 the performance is the same as I have 4 CPU
> machine and 2000 is greater than 128*4. On the other hand, 200 is less
> than 128*4, so this case is also improved (though not so drastically
> as less limits).

Thanks for the clarification Tagir. Makes sense now.
I missed the "commonPoolParallelism + 1" bit ...

Regards, Stefan

>
> With best regards,
> Tagir Valeev.
>

Re: RFR: 8154387 - Parallel unordered Stream.limit() tries to collect 128 elements even if limit is less

[Stefan Zobel]

Hi Tagir,

nice catch. I think this optimization is worthwile.

I'm a bit surprised about the JMH results for limits 200 and 2000.

limit = 200 is significantly faster than the unpatched code (with
higher variance, though) and limit = 2000 is about the same, but
with a significantly reduced variance. Maybe you'd need to increase
the number of iterations / forks to get more stable results that
are in line with expectations - or do I miss something here?


Regards,
Stefan


2016-04-16 15:05 GMT+02:00 Tagir F. Valeev :
> Hello!
>
> Please review and sponsor the following patch:
> https://bugs.openjdk.java.net/browse/JDK-8154387
> http://cr.openjdk.java.net/~tvaleev/webrev/8154387/r1/
>
> The rationale is to speed-up the parallel processing for unordered
> streams with low limit value. Such problems occur when you want to
> perform expensive filtering and select at most x elements which pass
> the filter (order does not matter). Currently unordered limit
> operation buffers up to 128 elements for each parallel task before it
> checks whether limit is reached. This is actually harmful when
> requested limit is lower: much more elements are requested from the
> upstream than necessary. Here's simple JMH test which illustrates the
> problem:
>
> http://cr.openjdk.java.net/~tvaleev/webrev/8154387/jmh/
> It extracts the requested number of probable-primes from the list of
> 1 BigInteger numbers. The results with 9ea+111:
>
> Benchmark(limit)  Mode  Cnt  Score  Error  Units
> LimitTest.parLimit 2  avgt   30108,971 ±0,643  us/op
> LimitTest.parLimit20  avgt   30934,176 ±   14,003  us/op
> LimitTest.parLimit   200  avgt   30   8772,417 ±  190,609  us/op
> LimitTest.parLimit  2000  avgt   30  41775,463 ± 1800,537  us/op
> LimitTest.parUnorderedLimit2  avgt   30   2557,798 ±   13,161  us/op
> LimitTest.parUnorderedLimit   20  avgt   30   2578,283 ±   23,547  us/op
> LimitTest.parUnorderedLimit  200  avgt   30   4577,318 ±   40,793  us/op
> LimitTest.parUnorderedLimit 2000  avgt   30  12279,346 ±  523,823  us/op
> LimitTest.seqLimit 2  avgt   30 34,831 ±0,190  us/op
> LimitTest.seqLimit20  avgt   30369,729 ±1,427  us/op
> LimitTest.seqLimit   200  avgt   30   3690,544 ±   13,907  us/op
> LimitTest.seqLimit  2000  avgt   30  36681,637 ±  156,538  us/op
>
> When the limit is 2 or 20, parallel unordered version is slower than
> parallel ordered! Even for limit = 200 it's still slower than
> sequential operation.
>
> The idea of the patch is to tweak the CHUNK_SIZE using the given limit and
> parallelism level. I used the following formula:
>
> this.chunkSize = limit >= 0 ? (int)Math.min(CHUNK_SIZE,
>  (skip + limit) / ForkJoinPool.getCommonPoolParallelism() + 1) : 
> CHUNK_SIZE;
>
> This does not affect cases when limit is big or not set at all (in
> skip mode). However it greatly improves cases when limit is small:
>
> Benchmark(limit)  Mode  Cnt  Score  Error  Units
> LimitTest.parLimit 2  avgt   30109,502 ±0,750  us/op
> LimitTest.parLimit20  avgt   30954,716 ±   39,276  us/op
> LimitTest.parLimit   200  avgt   30   8706,226 ±  184,330  us/op
> LimitTest.parLimit  2000  avgt   30  42126,346 ± 3163,444  us/op
> LimitTest.parUnorderedLimit2  avgt   30 39,303 ±0,177  us/op 
> !!!
> LimitTest.parUnorderedLimit   20  avgt   30266,107 ±0,492  us/op 
> !!!
> LimitTest.parUnorderedLimit  200  avgt   30   2547,177 ±   58,538  us/op 
> !!!
> LimitTest.parUnorderedLimit 2000  avgt   30  12216,402 ±  430,574  us/op
> LimitTest.seqLimit 2  avgt   30 34,993 ±0,704  us/op
> LimitTest.seqLimit20  avgt   30369,497 ±1,754  us/op
> LimitTest.seqLimit   200  avgt   30   3716,059 ±   61,054  us/op
> LimitTest.seqLimit  2000  avgt   30  36814,356 ±  161,531  us/op
>
> Here you can see that unordered cases are significantly improved. Now
> they are always faster than parallel ordered and faster than
> sequential for limit >= 20.
>
> I did not think up how to test this patch as it does not change
> visible behavior, only speed. However all the existing tests pass.
>
> What do you think?
>
> With best regards,
> Tagir Valeev.
>

Re: RFR: 8153293 - Stream API: Preserve SORTED and DISTINCT characteristics for boxed() and asLongStream() operations

[Stefan Zobel]

Hi Tagir,

thanks! Looks fine to me now - especially the new testSortDistinct() methods.
But note: I'm only a participant, not a reviewer of any sort.

Thanks,
Stefan


2016-04-12 18:37 GMT+02:00 Tagir F. Valeev :
> Hello!
>
> Thank you, Stefan and Paul for review. Here's updated webrev:
>
> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/r2/
>
> Changes:
> - all new mapToObj are private and not final
> - IntStream.asDoubleStream preserves distinct now
> - Tests fixed according to Stefan's suggestions
> - Additional tests added which test how sorted and distinct actually
>   work, as Paul suggests. Values close to Integer.MAX_VALUE and Long.MAX_VALUE
>   are tested. TreeSet and TreeSet is used to produce
>   expected result.
>
> With best regards,
> Tagir Valeev.
>

Re: RFR: 8153293 - Stream API: Preserve SORTED and DISTINCT characteristics for boxed() and asLongStream() operations

[Stefan Zobel]

Hi Tagir,

another minor issue. The testFlags() methods in IntPrimitiveOpsTests
 / LongPrimitiveOpsTests each have a duplicated assert:


IntPrimitiveOpsTests:

assertFalse(IntStreams.of(1, 10).boxed().spliterator()
  .hasCharacteristics(Spliterator.SORTED));


LongPrimitiveOpsTests:

assertFalse(LongStreams.of(1, 10).boxed().spliterator()
  .hasCharacteristics(Spliterator.SORTED));


The asserts for IntStreams.range(1, 10).asDoubleStream() would have
to be changed to account for DISTINCTness, of course.

Regards,
Stefan


2016-04-01 18:25 GMT+02:00 Tagir F. Valeev :
> Hello!
>
> Please review and sponsor the following patch:
> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/r1/
>
> The patch preserves more characteristics on primitive stream
> operations:
> IntStream/LongStream/DoubleStream.boxed() preserves SORTED and DISTINCT
> IntStream.asLongStream() preserves SORTED and DISTINCT
> IntStream.asDoubleStream() and LongStream.asDoubleStream() preserves SORTED
> (different longs can be converted into the same double, so DISTINCT is
> not preserved here; not sure whether this is possible for ints)
>
> Fixing the boxed() case is especially important as distinct() for
> primitive streams is implemented like boxed().distinct().unbox, so the
> actual distinct() operation cannot take the advantage of DISTINCT flag
> (skip the operation at all) or SORTED flag (switch to more efficient
> implementation).
>
> Here's the small JMH benchmark which measures the performance boost of
> quite common operation: sort the input numbers and leave only distinct
> ones:
> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/jmh/
>
> new Random(1).ints(size).sorted().distinct().toArray()
>
> I've got the following results.
>
> 9ea+111:
>
> Benchmark(size)  Mode  Cnt  Score  Error  
> Units
> SortDistinctTest.sortedDistinct  10  avgt   30  0,612 ±0,004  
> us/op
> SortDistinctTest.sortedDistinct1000  avgt   30 92,848 ±1,039  
> us/op
> SortDistinctTest.sortedDistinct  10  avgt   30  32147,205 ± 3487,422  
> us/op
>
> 9ea+111 patched:
>
> Benchmark(size)  Mode  Cnt ScoreError  Units
> SortDistinctTest.sortedDistinct  10  avgt   30 0,435 ±  0,001  us/op
> SortDistinctTest.sortedDistinct1000  avgt   3040,555 ±  0,772  us/op
> SortDistinctTest.sortedDistinct  10  avgt   30  9031,651 ± 73,956  us/op
>
> With best regards,
> Tagir Valeev.
>

Re: RFR: 8153293 - Stream API: Preserve SORTED and DISTINCT characteristics for boxed() and asLongStream() operations

[Stefan Zobel]

Hi Tagir,

a few comments below


a) IntPipeline: public final mapToObj(IntFunction mapper,
int opFlags) should be private, not public

b) IntPipeline: asDoubleStream() - as already discussed, 0 should be
passed as the opFlags value to the DoublePipeline.StatelessOp
constructor

c) I think, the new "private final mapToObj(...)" methods can be
"private" (dropping the "final")


Regards,
Stefan


2016-04-01 18:25 GMT+02:00 Tagir F. Valeev :
> Hello!
>
> Please review and sponsor the following patch:
> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/r1/
>
> The patch preserves more characteristics on primitive stream
> operations:
> IntStream/LongStream/DoubleStream.boxed() preserves SORTED and DISTINCT
> IntStream.asLongStream() preserves SORTED and DISTINCT
> IntStream.asDoubleStream() and LongStream.asDoubleStream() preserves SORTED
> (different longs can be converted into the same double, so DISTINCT is
> not preserved here; not sure whether this is possible for ints)
>
> Fixing the boxed() case is especially important as distinct() for
> primitive streams is implemented like boxed().distinct().unbox, so the
> actual distinct() operation cannot take the advantage of DISTINCT flag
> (skip the operation at all) or SORTED flag (switch to more efficient
> implementation).
>
> Here's the small JMH benchmark which measures the performance boost of
> quite common operation: sort the input numbers and leave only distinct
> ones:
> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/jmh/
>
> new Random(1).ints(size).sorted().distinct().toArray()
>
> I've got the following results.
>
> 9ea+111:
>
> Benchmark(size)  Mode  Cnt  Score  Error  
> Units
> SortDistinctTest.sortedDistinct  10  avgt   30  0,612 ±0,004  
> us/op
> SortDistinctTest.sortedDistinct1000  avgt   30 92,848 ±1,039  
> us/op
> SortDistinctTest.sortedDistinct  10  avgt   30  32147,205 ± 3487,422  
> us/op
>
> 9ea+111 patched:
>
> Benchmark(size)  Mode  Cnt ScoreError  Units
> SortDistinctTest.sortedDistinct  10  avgt   30 0,435 ±  0,001  us/op
> SortDistinctTest.sortedDistinct1000  avgt   3040,555 ±  0,772  us/op
> SortDistinctTest.sortedDistinct  10  avgt   30  9031,651 ± 73,956  us/op
>
> With best regards,
> Tagir Valeev.
>

Re: RFR: 8153293 - Stream API: Preserve SORTED and DISTINCT characteristics for boxed() and asLongStream() operations

[Stefan Zobel]

Hi Tagir,

good catch! I like the proposal.

>
> (different longs can be converted into the same double, so DISTINCT is
> not preserved here; not sure whether this is possible for ints)
>

I think IntStream.asDoubleStream() can also preserve DISTINCT as
different ints can't be mapped to the same double.


Math.ulp((double) Integer.MIN_VALUE) ~ 4.7E-7

in contrast to

Math.ulp((double) Long.MIN_VALUE) = 2048.0


So there are more than enough doubles in the vicinity of large int
values. It's only when ulp get's >= 1.0 that distinct integral values
need to be mapped to the same double (that happens between 1.0E15 and
1.0E16 for longs). Please anyone correct me if I'm wrong.


Regards,
Stefan

2016-04-01 18:25 GMT+02:00 Tagir F. Valeev :
> Hello!
>
> Please review and sponsor the following patch:
> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/r1/
>
> The patch preserves more characteristics on primitive stream
> operations:
> IntStream/LongStream/DoubleStream.boxed() preserves SORTED and DISTINCT
> IntStream.asLongStream() preserves SORTED and DISTINCT
> IntStream.asDoubleStream() and LongStream.asDoubleStream() preserves SORTED
> (different longs can be converted into the same double, so DISTINCT is
> not preserved here; not sure whether this is possible for ints)
>
> Fixing the boxed() case is especially important as distinct() for
> primitive streams is implemented like boxed().distinct().unbox, so the
> actual distinct() operation cannot take the advantage of DISTINCT flag
> (skip the operation at all) or SORTED flag (switch to more efficient
> implementation).
>
> Here's the small JMH benchmark which measures the performance boost of
> quite common operation: sort the input numbers and leave only distinct
> ones:
> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/jmh/
>
> new Random(1).ints(size).sorted().distinct().toArray()
>
> I've got the following results.
>
> 9ea+111:
>
> Benchmark(size)  Mode  Cnt  Score  Error  
> Units
> SortDistinctTest.sortedDistinct  10  avgt   30  0,612 ±0,004  
> us/op
> SortDistinctTest.sortedDistinct1000  avgt   30 92,848 ±1,039  
> us/op
> SortDistinctTest.sortedDistinct  10  avgt   30  32147,205 ± 3487,422  
> us/op
>
> 9ea+111 patched:
>
> Benchmark(size)  Mode  Cnt ScoreError  Units
> SortDistinctTest.sortedDistinct  10  avgt   30 0,435 ±  0,001  us/op
> SortDistinctTest.sortedDistinct1000  avgt   3040,555 ±  0,772  us/op
> SortDistinctTest.sortedDistinct  10  avgt   30  9031,651 ± 73,956  us/op
>
> With best regards,
> Tagir Valeev.
>

CollectionAndMapModifyStreamTest: minor copy-paste error

[Stefan Zobel]

Hi all,

just noticed that

org.openjdk.tests.java.util.stream.CollectionAndMapModifyStreamTest


has a small copy & paste error in line 116:

115: maps.put(HashMap.class.getName(), () -> new HashMap<>(content));
116: maps.put(HashMap.class.getName(), () -> new LinkedHashMap<>(content));

The same key is used for HashMap and LinkedHashMap, so the tests for
HashMap never get executed.


Btw, the "ThowableHelper" from /bootlib/java.base/java/util/stream
should be renamed to the correct "ThRowableHelper" someday ;-)

Regards,
Stefan

Re: RFR: 8072727 - add variation of Stream.iterate() that's finite

[Stefan Zobel]

2016-02-15 7:48 GMT+01:00 Peter Levart :
>
>
> What about even more permissive:
>
> static  Stream iterate3(S seed, Predicate
> predicate, Function f)
>


Hi Peter,

yes, Function instead of UnaryOperator is
indeed tempting.

On the other hand, for my taste, I find the wildcard-ridden signature
of iterate3 oversteps a certain complexity budget a little (that's
just my personal opinion, of course).

But you may be right - we already have such complex signatures in a
couple of other places. The question is, whether it would be really useful.


Regards,
Stefan



>
> public class SignatureTest {
>
> static  Stream iterate1(T seed, Predicate predicate,
> UnaryOperator f) {
> ...
> }
>
> static  Stream iterate2(S seed, Predicate
> predicate, UnaryOperator f) {
> ...
> }
>
> static  Stream iterate3(S seed, Predicate
> predicate, Function f) {
> ...
> }
>
>
> public static void main(String[] args) {
>
> Stream si1 = iterate1(0, i -> i < 10, i -> i + 1); // OK
> Stream si2 = iterate2(0, i -> i < 10, i -> i + 1); // OK
> Stream si3 = iterate3(0, i -> i < 10, i -> i + 1); // OK
>
> Stream sn1 = iterate1(0, i -> i < 10, i -> i + 1);
>
> //SignatureTest.java:32: error: bad operand types for binary
> operator '<'
> //Stream sn1 = iterate1(0, i -> i < 10, i -> i + 1);
> //^
> //SignatureTest.java:32: error: bad operand types for binary
> operator '+'
> //Stream sn1 = iterate1(0, i -> i < 10, i -> i + 1);
> // ^
>
>
> Stream sn2 = iterate2(0, i -> i < 10, i -> i + 1); // OK
> Stream sn3 = iterate3(0, i -> i < 10, i -> i + 1); // OK
>
>
> Predicate csNotEmpty = cs -> cs.length() > 0;
>
> Stream css1 = iterate1("abcde", csNotEmpty, (String s)
> -> s.substring(1));
>
> //SignatureTest.java:44: error: method iterate1 in class
> SignatureTest cannot be applied to given types;
> //Stream css1 = iterate1("abcde", csNotEmpty,
> (String s) -> s.substring(1));
> //^
> //  required: T,Predicate,UnaryOperator
> //  found: String,Predicate,(String s)[...]ng(1)
> //  reason: inference variable T has incompatible equality
> constraints String,CharSequence
> //  where T is a type-variable:
> //T extends Object declared in method
> iterate1(T,Predicate,UnaryOperator)
>
>
> Stream css2 = iterate2("abcde", csNotEmpty, (String s)
> -> s.substring(1));
>
> //SignatureTest.java:54: error: method iterate2 in class
> SignatureTest cannot be applied to given types;
> //Stream css2 = iterate2("abcde", csNotEmpty,
> (String s) -> s.substring(1));
> //^
> //  required: S,Predicate,UnaryOperator
> //  found: String,Predicate,(String s)[...]ng(1)
> //  reason: inference variable S has incompatible equality
> constraints String,CharSequence
> //  where S,T are type-variables:
> //S extends T declared in method
> iterate2(S,Predicate,UnaryOperator)
> //T extends Object declared in method
> iterate2(S,Predicate,UnaryOperator)
>
>
> Stream css3 = iterate3("abcde", csNotEmpty, (String s)
> -> s.substring(1)); // OK
> }
> }
>
>
> Regards, Peter
>
>
>
> Regards,
> Stefan
>
>
> 2016-02-14 15:53 GMT+01:00 Tagir F. Valeev :
>
> Hello!
>
> I wanted to work on foldLeft, but Brian asked me to take this issue
> instead. So here's webrev:
> http://cr.openjdk.java.net/~tvaleev/webrev/8072727/r1/
>
> I don't like iterator-based Stream source implementations, so I made
> them AbstractSpliterator-based. I also implemented manually
> forEachRemaining as, I believe, this improves the performance in
> non-short-circuiting cases.
>
> I also decided to keep two flags (started and finished) to track the
> state. Currently existing implementation of infinite iterate() does
> not use started flag, but instead reads one element ahead for
> primitive streams. This seems wrong to me and may even lead to
> unexpected exceptions (*). I could get rid of "started" flag for
> Stream.iterate() using Streams.NONE, but this would make object
> implementation different from primitive implementations. It would also
> be possible to keep single three-state variable (byte or int,
> NOT_STARTED, STARTED, FINISHED), but I doubt that this would improve
> the performance or footprint. Having two flags looks more readable to
> me.
>
> Currently existing two-arg iterate methods can now be expressed as a
> partial case of the new method:
>
> public static Stream iterate(final T seed, final UnaryOperator f) {
> return iterate(seed, x -> true, f);
> }
> (same for 

Re: RFR: 8072727 - add variation of Stream.iterate() that's finite

[Stefan Zobel]

Hi Tagir,

this looks good. I wonder, however, if the signature should
accept a wider range of types, i.e., something like


static  Stream iterate(S seed, Predicate
predicate, UnaryOperator f)


I know that the corresponding

static  Stream iterate(T seed, UnaryOperator f)

is less general than that, but I don't know whether this is
the outcome of a thoughtful decision or just an oversight.

What do you think?


Regards,
Stefan


2016-02-14 15:53 GMT+01:00 Tagir F. Valeev :
> Hello!
>
> I wanted to work on foldLeft, but Brian asked me to take this issue
> instead. So here's webrev:
> http://cr.openjdk.java.net/~tvaleev/webrev/8072727/r1/
>
> I don't like iterator-based Stream source implementations, so I made
> them AbstractSpliterator-based. I also implemented manually
> forEachRemaining as, I believe, this improves the performance in
> non-short-circuiting cases.
>
> I also decided to keep two flags (started and finished) to track the
> state. Currently existing implementation of infinite iterate() does
> not use started flag, but instead reads one element ahead for
> primitive streams. This seems wrong to me and may even lead to
> unexpected exceptions (*). I could get rid of "started" flag for
> Stream.iterate() using Streams.NONE, but this would make object
> implementation different from primitive implementations. It would also
> be possible to keep single three-state variable (byte or int,
> NOT_STARTED, STARTED, FINISHED), but I doubt that this would improve
> the performance or footprint. Having two flags looks more readable to
> me.
>
> Currently existing two-arg iterate methods can now be expressed as a
> partial case of the new method:
>
> public static Stream iterate(final T seed, final UnaryOperator f) {
> return iterate(seed, x -> true, f);
> }
> (same for primitive streams). I may do this if you think it's
> reasonable.
>
> I created new test class and added new iterate sources to existing
> data providers.
>
> Please review and sponsor!
>
> With best regards,
> Tagir Valeev.
>
> (*) Consider the following code:
>
> int[] data = {1,2,3,4,-1};
> IntStream.iterate(0, x -> data[x])
>  .takeWhile(x -> x >= 0)
>  .forEach(System.out::println);
>
> Currently this unexpectedly throws an AIOOBE, because
> IntStream.iterate unnecessarily tries to read one element ahead.
>

Re: RFR-8148838: Stream.flatMap(...).spliterator() cannot properly split after tryAdvance()

[Stefan Zobel]

> 2016-02-08 15:05 GMT+01:00 Paul Sandoz :
>
>> On 8 Feb 2016, at 14:53, Tagir F. Valeev  wrote:
>>
>> PS> Why don’t you check if "buffer == null” at #189? i.e. similar to 
>> forEachRemaining:
>>
>> That would make minimal behavioral change to fix this issue (fix
>> flatMap keys only, but not affect other intermediate ops which were
>> working correctly). Well, if buffer == null check is enough, here's
>> update:
>>
>
> Many thanks, yes it’s sufficient (finished == true when buffer != null && 
> buffer.count() == 0).
>


Hi all,

Question: will this get backported to Java 8 ?


Regards,
Stefan

Re: Stream.limit parallel ordered performance

[Stefan Zobel]

2016-01-24 12:36 GMT+01:00 Tagir F. Valeev :
> Hello!
>
> I'm investigating Stream.limit() performance for parallel ordered
> streams when no SUBSIZED optimization is performed.
>
> Consider the following code, for example:
>
> AtomicInteger counter = new AtomicInteger();
> int[] result = IntStream.range(0, 1_000_000).parallel().filter(x -> true)
> .peek(x -> counter.incrementAndGet()).limit(10).toArray();
> System.out.println(Arrays.toString(result));
> System.out.println(counter.get());
>
> How much the counter.get() would print? It changes from launch to
> launch, but usually within 375000..625000. This is just insane. On my
> 4-core system parallel stream creates 16 individual tasks. I expect
> that every individual task should consume no more than 10 elements, so
> in total no more than 160 elements should be consumed in this case.
>
> Here's a patch which addresses this issue:
> http://cr.openjdk.java.net/~tvaleev/patches/limit/limit-patch.txt
>
> In the limit case non-root leaf tasks may switch to copyIntoWithCancel
> to control the count of consumed elements and do not consume more than
> necessary.
>
> This change seems to fix the issue addressed in comment (at least
> partially):
>
> // @@@ OOMEs will occur for LongStream.range(0, Long.MAX_VALUE).filter(i -> 
> true).limit(n)
> // regardless of the value of n
> // Need to adjust the target size of splitting for the
> // SliceTask from say (size / k) to say min(size / k, 1 << 14)
> // This will limit the size of the buffers created at the leaf nodes
> // cancellation will be more aggressive cancelling later tasks
> // if the target slice size has been reached from a given task,
> // cancellation should also clear local results if any
>
> I checked with the following code:
>
> for(int n : new int[] {10, 100, 1000, 5000, 1, 5, 10, 50, 
> 100}) {
> System.out.println(n);
> long[] arr = LongStream.range(0, Long.MAX_VALUE).filter(i -> 
> true).parallel().limit(n).toArray();
> long[] ref = LongStream.range(0, n).toArray();
> System.out.println(Arrays.equals(arr, ref));
> }
>
> It works correctly after applying my patch (while dies with OOME
> without patch, as comment suggests).
>
> Currently existing unit tests also pass with my patch. However I'm
> very new in the internals of parallel stream processing, so it's
> possible that I'm missing something. Please review! If this looks
> reasonable I will log an issue and write new test cases.
>
> Thank you in advance,
> With best regards,
> Tagir Valeev.
>


Hi Tagir,

I'm not an expert. To me, your suggestion appears to be very sensible.

Regards,
Stefan

Re: RFR 8144675: Add a filtering collector

[Stefan Zobel]

Hi shinyafox,

minor typo in the code example:


s/wellPaidEmployeesByDeparetment/wellPaidEmployeesByDepartment


Regards,
Stefan


2015-12-08 13:04 GMT+01:00 ShinyaYoshida :
> Thank you so much!
> I've updated in webrev.02.
>
> Best regard,
> shinyafox(Shinya Yoshida)
>

Re: Optional.or() doesn't use a wildcard in its signature

[Stefan Zobel]

2015-11-01 1:12 GMT+01:00 Michael Nascimento <mist...@gmail.com>:
> Hi Vitaly,
>
> Exactly, I was just trying to point out the method signature seems broken
> anyway.
>
> Regards,
> Michael


Hi Michael,

I don't think the signature is broken.

An Optional is not a subtype of Optional.
So even if the signature were

public  Optional or(Supplier<Optional> supplier)

we'd have to upcast the result of supplier.get() to Optional
which appears to be wrong from a type perspective.


Regards,
Stefan



>
> On 31 Oct 2015 11:59, "Vitaly Davidovich" <vita...@gmail.com> wrote:
>>
>> This would require Supplier<Optional>, not Supplier> Optional>.
>>
>> sent from my phone
>>
>> On Oct 31, 2015 2:49 PM, "Michael Nascimento" <mist...@gmail.com> wrote:
>>>
>>> If this instance is an Optional ,  passing an
>>> Optional will fail to compile.
>>>
>>> Regards,
>>> Michael
>>> On 31 Oct 2015 11:21, "Stefan Zobel" <splitera...@gmail.com> wrote:
>>>
>>> > 2015-10-31 19:11 GMT+01:00 Remi Forax <fo...@univ-mlv.fr>:
>>> >
>>> > > Hi all, hi Paul,
>>> > >
>>> > > I've just seen that Optional.or is declared as
>>> > >   public Optional or(Supplier<Optional> supplier) {
>>> > > instead of
>>> > >   public Optional or(Supplier> supplier) {
>>> > >
>>> > > regards,
>>> > > Rémi
>>> > >
>>> >
>>> >
>>> > I don't get it. Optional is final anyway. Can you explain?
>>> >
>>> > Thanks,
>>> > Stefan

Re: Optional.or() doesn't use a wildcard in its signature

[Stefan Zobel]

2015-11-01 15:41 GMT+01:00 Vitaly Davidovich <vita...@gmail.com>:
> One could argue that this is perfectly sound to do given Optional is
> readonly, we just can't express this variance cleanly in java (i.e. without
> upcast).
>

Sure, as in Scala's Option[+A].
I should have said "... from a **Java** type perspective ".

Regards,
Stefan


>
> On Nov 1, 2015 7:31 AM, "Stefan Zobel" <splitera...@gmail.com> wrote:
>>
>> 2015-11-01 1:12 GMT+01:00 Michael Nascimento <mist...@gmail.com>:
>> > Hi Vitaly,
>> >
>> > Exactly, I was just trying to point out the method signature seems
>> > broken
>> > anyway.
>> >
>> > Regards,
>> > Michael
>>
>>
>> Hi Michael,
>>
>> I don't think the signature is broken.
>>
>> An Optional is not a subtype of Optional.
>> So even if the signature were
>>
>> public  Optional or(Supplier<Optional> supplier)
>>
>> we'd have to upcast the result of supplier.get() to Optional
>> which appears to be wrong from a type perspective.
>>
>>
>> Regards,
>> Stefan
>>
>>
>>
>> >
>> > On 31 Oct 2015 11:59, "Vitaly Davidovich" <vita...@gmail.com> wrote:
>> >>
>> >> This would require Supplier<Optional>, not Supplier> >> extends
>> >> Optional>.
>> >>
>> >> sent from my phone
>> >>
>> >> On Oct 31, 2015 2:49 PM, "Michael Nascimento" <mist...@gmail.com>
>> >> wrote:
>> >>>
>> >>> If this instance is an Optional ,  passing an
>> >>> Optional will fail to compile.
>> >>>
>> >>> Regards,
>> >>> Michael
>> >>> On 31 Oct 2015 11:21, "Stefan Zobel" <splitera...@gmail.com> wrote:
>> >>>
>> >>> > 2015-10-31 19:11 GMT+01:00 Remi Forax <fo...@univ-mlv.fr>:
>> >>> >
>> >>> > > Hi all, hi Paul,
>> >>> > >
>> >>> > > I've just seen that Optional.or is declared as
>> >>> > >   public Optional or(Supplier<Optional> supplier) {
>> >>> > > instead of
>> >>> > >   public Optional or(Supplier> supplier)
>> >>> > > {
>> >>> > >
>> >>> > > regards,
>> >>> > > Rémi
>> >>> > >
>> >>> >
>> >>> >
>> >>> > I don't get it. Optional is final anyway. Can you explain?
>> >>> >
>> >>> > Thanks,
>> >>> > Stefan

Re: Optional.or() doesn't use a wildcard in its signature

[Stefan Zobel]

2015-11-01 1:12 GMT+01:00 Michael Nascimento :
> Hi Vitaly,
>
> Exactly, I was just trying to point out the method signature seems broken
> anyway.
>

Rethinking it, I believe you are right.

public  Optional or(Supplier supplier)

would be more general and still be sound type-wise.

There's a similar issue for Stream generate / iterate in the bugtracker
https://bugs.openjdk.java.net/browse/JDK-8132097


Thanks,
Stefan

> On 31 Oct 2015 11:59, "Vitaly Davidovich"  wrote:
>>
>> This would require Supplier, not Supplier> Optional>.
>>
>> sent from my phone
>>
>> On Oct 31, 2015 2:49 PM, "Michael Nascimento"  wrote:
>>>
>>> If this instance is an Optional ,  passing an
>>> Optional will fail to compile.
>>>
>>> Regards,
>>> Michael

Re: Optional.or() doesn't use a wildcard in its signature

[Stefan Zobel]

2015-10-31 19:11 GMT+01:00 Remi Forax :

> Hi all, hi Paul,
>
> I've just seen that Optional.or is declared as
>   public Optional or(Supplier supplier) {
> instead of
>   public Optional or(Supplier> supplier) {
>
> regards,
> Rémi
>


I don't get it. Optional is final anyway. Can you explain?

Thanks,
Stefan

Re: Optional.or name Re: RFR 8080418 Add Optional.or()

[Stefan Zobel]

>
> “Optional.or” is the best name so far i have come up with. The “orElse” 
> prefix is reversed for terminal methods and i don’t want to overload that.
>
> --
>
> Alternative suggestions:
>
> - “Optional.otherwise”
> A bit of a mouthful, but reasonably accurate
>
> - “Optional.mapElse"
> The name is not entirely accurate since it accepts a Suppler not a T, a 
> more accurate name would be the following alternative.
>
> - “Optional.flatMapElse"
> This name is likely to confuse.
>
> Paul.
>



- "Optional.orIfAbsent" perhaps?

Actually, I don't regard "Optional.or" as a poor name.


Stefan.

Re: RFR 8080418 Add Optional.or()

[Stefan Zobel]

Looks good. One more nitpicking. "ifPresent() " in OptionalDouble/Int/Long
still uses the old wording


 * If a value is present, perform the given action with the value,
 * otherwise do nothing.


whereas in Optional you have the better


 * If a value is present, performS the given action with the value,
 * otherwise doES nothing.


Stefan


2015-09-29 9:39 GMT+02:00 Paul Sandoz <paul.san...@oracle.com>:

>
> > On 28 Sep 2015, at 19:45, Stefan Zobel <splitera...@gmail.com> wrote:
> >
> > Hi Paul,
> >
> > is it a good idea to add the "{@inheritDoc}" to the toString() Javadoc of
> > Optional (and to retain it in OptionalDouble/Int/Long)?
> >
> > As Stuart Marks has observed in the Double/Int/LongSummaryStatistics case
> > the inherited Object.toString() doc is "mostly irrelevant"
> > (see
> >
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-June/034282.html
> )
> >
> > So the {@inheritDoc} got removed from Double/Int/LongSummaryStatistics in
> > JDK-8080450. I feel this would also be better for Optional.
> >
>
> Done.
>
>
> > On 28 Sep 2015, at 20:00, Stefan Zobel <splitera...@gmail.com> wrote:
> >
> > In the OptionalDouble/Int/Long Javadoc of method "empty()"
> >
> >
> > * {@code Optional.empty()}. There is no guarantee that it is a singleton.
> >
> >
> > should be
> >
> >
> > * {@code OptionalDouble.empty()}.  There is no guarantee that it is a
> > singleton.
> >
> > and so on.
>
> Done.
>
> Thanks,
> Paul.
>

Re: RFR 8080418 Add Optional.or()

[Stefan Zobel]

Hi Paul,

is it a good idea to add the "{@inheritDoc}" to the toString() Javadoc of
Optional (and to retain it in OptionalDouble/Int/Long)?

As Stuart Marks has observed in the Double/Int/LongSummaryStatistics case
the inherited Object.toString() doc is "mostly irrelevant"
(see
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-June/034282.html)

So the {@inheritDoc} got removed from Double/Int/LongSummaryStatistics in
JDK-8080450. I feel this would also be better for Optional.


Stefan


2015-09-25 12:58 GMT+02:00 Paul Sandoz :

> Hi,
>
> Please review this change to add a method Optional.or that allows one to
> better compose optionals:
>
>   http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8080418-optional-or/webrev/
>
> I also took the opportunity to clear up the JavaDoc, it was a little
> inconsistent and i personally found it harder to read in source code.
>
> —
>
> Separately while we are on the topic of Optional i would be interested in
> opinions on the following changes:
>
>
> http://cr.openjdk.java.net/~psandoz/jdk9/optional-prims-filter-map/webrev/
>
> 1) add methods that were missing on the primitive specializations; and
>
> 2) add to all variants a mapOrElseGet (otherwise known as a “fold”), which
> is the equivalent of map(mapper).orElseGet(supplier). That is arguably less
> mind-bending to use when transforming from Optional to Optional.
>
> Paul.
>

Re: RFR 8080418 Add Optional.or()

[Stefan Zobel]

In the OptionalDouble/Int/Long Javadoc of method "empty()"


* {@code Optional.empty()}. There is no guarantee that it is a singleton.


should be


* {@code OptionalDouble.empty()}.  There is no guarantee that it is a
singleton.

and so on.


Stefan


2015-09-28 11:24 GMT+02:00 Paul Sandoz :

>
> > On 26 Sep 2015, at 21:36, Chris Hegarty 
> wrote:
> >
> >
> >> On 25 Sep 2015, at 11:58, Paul Sandoz  wrote:
> >>
> >> Hi,
> >>
> >> Please review this change to add a method Optional.or that allows one
> to better compose optionals:
> >>
> >>
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8080418-optional-or/webrev/
> >>
> >> I also took the opportunity to clear up the JavaDoc, it was a little
> inconsistent and i personally found it harder to read in source code.
> >
> > The new method and code cleanup look good.
> >
> > Not related to your changes, but I noticed when reviewing the clean up,
> typo “OptionAL.empty()"
> >
> >  L80 * {@code Option.empty()}. There is no guarantee that it is a
> singleton.
> >
>
> Thanks, updated,
> Paul.
>

Re: RFR Collector.finisher refers to IDENTITY_TRANSFORM rather than INDENTITY_FINISH

[Stefan Zobel]

Hi Paul,

perhaps you could take the opportunity and also add the missing @since
1.9 tags to all the new dropWhile() / takeWhile() methods (JDK-8071597).

Also in dropWhile() / takeWhile() there is a small typo (I guess) in the
Javadoc:


takeWhile: takes all elements (the result is the same is the input)

dropWhile: the stream match the given predicate then no elements are
dropped (the result is the same is the input)


I guess that should read: (the result is the same as is the input)?


Stefan


2015-07-30 17:08 GMT+02:00 Paul Sandoz paul.san...@oracle.com:

 Hi,

 Please review this simple fix to the JavaDoc on
 j.u.stream.Collector.finisher.

 I am also opportunistically fixing some internal comments identified by
 Tagir.

 Paul.

 diff -r 4e3135fac8cc
 src/java.base/share/classes/java/util/stream/Collector.java
 --- a/src/java.base/share/classes/java/util/stream/Collector.java
  Fri Jul 24 15:33:13 2015 -0700
 +++ b/src/java.base/share/classes/java/util/stream/Collector.java
  Thu Jul 30 17:05:13 2015 +0200
 @@ -223,7 +223,7 @@
   * Perform the final transformation from the intermediate
 accumulation type
   * {@code A} to the final result type {@code R}.
   *
 - * pIf the characteristic {@code IDENTITY_TRANSFORM} is
 + * pIf the characteristic {@code IDENTITY_FINISH} is
   * set, this function may be presumed to be an identity transform
 with an
   * unchecked cast from {@code A} to {@code R}.
   *
 diff -r 4e3135fac8cc
 src/java.base/share/classes/java/util/stream/SliceOps.java
 --- a/src/java.base/share/classes/java/util/stream/SliceOps.java
 Fri Jul 24 15:33:13 2015 -0700
 +++ b/src/java.base/share/classes/java/util/stream/SliceOps.java
 Thu Jul 30 17:05:13 2015 +0200
 @@ -138,7 +138,7 @@
  skip, limit, size);
  }
  else {
 -// @@@ OOMEs will occur for
 LongStream.longs().filter(i - true).limit(n)
 +// @@@ OOMEs will occur for LongStream.range(0,
 Long.MAX_VALUE)).filter(i - true).limit(n)
  // regardless of the value of n
  // Need to adjust the target size of splitting
 for the
  // SliceTask from say (size / k) to say min(size
 / k, 1  14)
 diff -r 4e3135fac8cc
 src/java.base/share/classes/java/util/stream/Streams.java
 --- a/src/java.base/share/classes/java/util/stream/Streams.java Fri Jul 24
 15:33:13 2015 -0700
 +++ b/src/java.base/share/classes/java/util/stream/Streams.java Thu Jul 30
 17:05:13 2015 +0200
 @@ -156,10 +156,9 @@
   * than a balanced tree at the expense of a higher-depth for the
 right
   * side of the range.
   *
 - * pThis is optimized for cases such as IntStream.ints() that is
 - * implemented as range of 0 to Integer.MAX_VALUE but is likely
 to be
 - * augmented with a limit operation that limits the number of
 elements
 - * to a count lower than this threshold.
 + * pThis is optimized for cases such as IntStream.range(0,
 Integer.MAX_VALUE)
 + * that is likely to be augmented with a limit operation that
 limits the
 + * number of elements to a count lower than this threshold.
   */
  private static final int BALANCED_SPLIT_THRESHOLD = 1  24;

 @@ -280,10 +279,9 @@
   * than a balanced tree at the expense of a higher-depth for the
 right
   * side of the range.
   *
 - * pThis is optimized for cases such as LongStream.longs() that
 is
 - * implemented as range of 0 to Long.MAX_VALUE but is likely to be
 - * augmented with a limit operation that limits the number of
 elements
 - * to a count lower than this threshold.
 + * pThis is optimized for cases such as LongStream.range(0,
 Long.MAX_VALUE)
 + * that is likely to be augmented with a limit operation that
 limits the
 + * number of elements to a count lower than this threshold.
   */
  private static final long BALANCED_SPLIT_THRESHOLD = 1  24;

Re: RFR 8130828: Fix some typos and omissions in the the j.u.stream JavaDoc

[Stefan Zobel]

Hi Paul,

looks good.


Stefan


2015-07-30 18:32 GMT+02:00 Paul Sandoz paul.san...@oracle.com:

 Hi Stefan, Tagir,

 Updated:


 http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8130828-stream-doc-typos/webrev/

 Paul.

Typo in AbstractSpliterator Javadoc

[Stefan Zobel]

Hi all,


I just noticed some typos in the
Spliterators.Abstract(Double/Int/Long)Spliterator Javadoc:

a) The forEachRemaining link label is forEach instead of
forEachRemaining in all AbstractSpliterators.

b) The primitive AbstractSpliterators Javadoc has a surplus } just before
the tryAdvance and forEach link labels.



Stefan

Re: RFR 8071597 Add Stream dropWhile and takeWhile operations

[Stefan Zobel]

I'm still trying to wrap my head around the test logic for the (par  !ord)
 (op == WhileOp.Drop) case
in the whileResultAsserter() method in WhileOpTest.

Wouldn't it be possible that, for an unordered parallel stream, dropWhile()
won't drop anything at all (i.e.,
drops an empty set)?

In that case, input.size() == output.size() and the set of matching output
elements is no longer a _proper_ subset
of the set of matching input elements. The whileResultAsserter() would fail
even though dropWhile() works correctly
(in a nondeterministic sense)?

It's a bit late now and I guess it's all in my imagination. Still curious
...


Thanks,
Stefan


2015-06-02 15:13 GMT+02:00 Paul Sandoz paul.san...@oracle.com:

 Hi,

 Please review this webrev that adds take/dropWhile operations to streams:


 http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8071597-take-drop-while/webrev/

 I opted to weight the documentation of the operations towards ordered
 streams in the first paragraph. That is what makes most sense in terms of
 usage and what most people will read. Thus i refer to the longest prefix
 in the first paragraph then define what that means in subsequent paragraphs
 for ordered and unordered streams:

  482 /**
  483  * Returns a stream consisting of the longest prefix of elements
 taken from
  484  * this stream that match the given predicate.
  485  *
  486  * pIf this stream is ordered then the prefix is a contiguous
 sequence of
  487  * elements of this stream.  All elements of the sequence match
 the given
  488  * predicate, the first element of the sequence is the first
 element
  489  * (if any) of this stream, and the element (if any) immediately
 following
  490  * the last element of the sequence does not match the given
 predicate.
  491  *
  492  * pIf this stream is unordered then the prefix is a subset of
 elements of
  493  * this stream.  All elements (if any) of the subset match the
 given
  494  * predicate.  In this case the behavior of this operation is
  495  * nondeterministic; it is free to select any valid subset as the
 prefix.
  496  *
  497  * pThis is a a
 href=package-summary.html#StreamOpsshort-circuiting
  498  * stateful intermediate operation/a.
  499  *
 ...
  528 default StreamT takeWhile(Predicate? super T predicate) {

  537 /**
  538  * Returns a stream consisting of the remaining elements of this
 stream
  539  * after dropping the longest prefix of elements that match the
 given
  540  * predicate.
  541  *
  542  * pIf this stream is ordered then the prefix is a contiguous
 sequence of
  543  * elements of this stream.  All elements of the sequence match
 the given
  544  * predicate, the first element of the sequence is the first
 element
  545  * (if any) of this stream, and the element (if any) immediately
 following
  546  * the last element of the sequence does not match the given
 predicate.
  547  *
  548  * pIf this stream is unordered then the prefix is a subset of
 elements of
  549  * this stream.  All elements (if any) of the subset match the
 given
  550  * predicate.  In this case the behavior of this operation is
  551  * nondeterministic; it is free to select any valid subset as the
 prefix.
  552  *
 ...
  584 default StreamT dropWhile(Predicate? super T predicate) {


 After this has been reviewed i will follow up with a further issue
 regarding the specification of takeWhile, stateful predicates and
 cancellation. I avoided such specification here as it's likely to rathole
 :-)

 Basically the takeWhile operation is implemented such that one can do:

  long t = System.currentTimeMillis();
  ListBigInteger pps = Stream
  .generate(() - BigInteger.probablePrime(1024,
 ThreadLocalRandom.current()))
  .parallel()
  .takeWhile(e - (System.currentTimeMillis() - t) 
 TimeUnit.SECONDS.toMillis(5))
  .collect(toList());

 Paul.

Re: RFR 8072773 (fs) Files.lines needs a better splitting implementation for stream source

[Stefan Zobel]

With respect to the 'stream test library not closing streams' bug:

would you mind to create a separate bugtracker issue for that, or do you
think it's too special to carry that weight?


Thanks,
Stefan


2015-06-04 22:06 GMT+02:00 Paul Sandoz paul.san...@oracle.com:

 On Jun 3, 2015, at 6:18 PM, Alan Bateman alan.bate...@oracle.com wrote:
  As this code path is only for the default provider case then there's a
 good chance that it will be a FileChannelImpl, in which case you can call
 its unmap method (directly or via a shared secret). It is possible to
 interpose on the default provider so you can't be guaranteed it is a
 FileChannelImpl of course.
 

 Not yet done. I would like to revisit this as a separate issue if i may.


  In passing, you might consider moving  ByteBufferLinesSpliterator to its
 own source file because Files is getting very big.
 

 Done.

 I am going to revisit the set of support charsets at another time. For the
 moment i have limited to a subset of standard charsets.

 Also the file lines test tickled a bug in the stream test library, it was
 not closing streams, which becomes an issue when ~15000 tests are executed
 leaving the process with no more file descriptors:

 http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8072773-File-lines/webrev/

 Paul.

Fwd: RFR 8071597 Add Stream dropWhile and takeWhile operations

[Stefan Zobel]

Hi Paul,

Looks good.

I was wondering why the truncate method in Node.OfInt / OfLong / OfDouble
did not receive the same


+if (to == count()) {
+spliterator.forEachRemaining(nodeBuilder);
+} else {
+for (int i = 0; i  size 
spliterator.tryAdvance(nodeBuilder); i++) { }
+}


treatment as the truncate method in Node itself. Looks pretty symmetric to
my untrained eyes?


Thanks,
Stefan

DualPivotQuicksort webrev for JDK-8080945

[Stefan Zobel]

Hi all,


Unless I'm doing something immensely stupid, the DualPivotQuicksort
proposal in http://cr.openjdk.java.net/~psandoz/tmp/gs/sort/webrev.2/
doesn't work for me.


This little program


public static void main(String[] args) {
int[] a = new int[287];

for (int i = 0; i  a.length; i++) {
a[i] = -((i % 143) + 1);
}

System.out.println(Arrays.toString(a));

DualPivotQuicksort.sort(a, 0, a.length - 1, null, 0, 0);
}



sends the sort into an infinite loop. Something wrong with the test??


Stefan