Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-21 Thread forax
- Mail original -
> De: "Peter Levart" 
> À: "Remi Forax" , "Brian Goetz" 
> Cc: "core-libs-dev" 
> Envoyé: Jeudi 21 Mars 2019 15:54:21
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> On 3/21/19 2:17 PM, fo...@univ-mlv.fr wrote:
>> for some reason i was convinced that IntStream.iterator() was returning a
>> PrimitiveIterator.OfInt and not an Iterator,
>> so yes, it will work but i don't understand why it's not BaseStream instead 
>> of
>> Stream that inherits from Iterable.
> 
> I think it's the Iterable.forEach(Consumer) that would clash
> with IntStream.forEach(IntConsumer), LongStream.forEach(LongConsumer)
> and DoubleStream.forEach(DoubleConsumer) if Iterable was a supertype of
> BaseStream.
> 
> ...unless IntConsumer was made to extend Consumer, LongConsumer
> was made to extend Consumer and DoubleConsumer was made to extend
> Consumer...
> 
> I tried that and it (almost) compiles. The sole problem presents the
> following class:
> 
>     public class LongSummaryStatistics implements LongConsumer,
> IntConsumer { ...
> 
> So I had another idea. What if IntConsumer, LongConsumer and
> DoubleConsumer were all made to extend Consumer ?
> 
> In that case LongSummaryStatistics would only have to override the
> method to disambiguate it.
> 
> By doing that, the problem springs in java.util.stream.Sink hierarchy
> (Sink extends Consumer) where Sink.OfInt extends both
> Sink and IntConsumer...
> 
> Perhaps this could be fixed (as it is internal non-public API), but the
> same problem could exist in 3rd party code...


having a class that implements 2 consumers like IntConsumer and LongConsumer is 
hard to retrofit if as you said IntConsumer is a subtype of Consumer and 
LongConsumer is a subtype of Consumer, it means that the bridge 
accept(Object) has to dispatch to accept(int) and accept(long) at the same time 

A code like
 Object o = ...
 Consumer c = new LongSummaryStatistics(...);
 c.accept(o);
shows the issue.

Having LongSummaryStatistics implementing IntConsumer was a mistake in 
retrospect.

so thanks, i've my answer why BaseStream can not implement Iterable until at 
least we have reified generics.

> 
> Regards, Peter

regards,
Rémi


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-21 Thread Peter Levart




On 3/21/19 2:17 PM, fo...@univ-mlv.fr wrote:

for some reason i was convinced that IntStream.iterator() was returning a 
PrimitiveIterator.OfInt and not an Iterator,
so yes, it will work but i don't understand why it's not BaseStream instead of 
Stream that inherits from Iterable.


I think it's the Iterable.forEach(Consumer) that would clash 
with IntStream.forEach(IntConsumer), LongStream.forEach(LongConsumer) 
and DoubleStream.forEach(DoubleConsumer) if Iterable was a supertype of 
BaseStream.


...unless IntConsumer was made to extend Consumer, LongConsumer 
was made to extend Consumer and DoubleConsumer was made to extend 
Consumer...


I tried that and it (almost) compiles. The sole problem presents the 
following class:


    public class LongSummaryStatistics implements LongConsumer, 
IntConsumer { ...


So I had another idea. What if IntConsumer, LongConsumer and 
DoubleConsumer were all made to extend Consumer ?


In that case LongSummaryStatistics would only have to override the 
method to disambiguate it.


By doing that, the problem springs in java.util.stream.Sink hierarchy 
(Sink extends Consumer) where Sink.OfInt extends both 
Sink and IntConsumer...


Perhaps this could be fixed (as it is internal non-public API), but the 
same problem could exist in 3rd party code...


Regards, Peter



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-21 Thread forax
- Mail original -
> De: "Brian Goetz" 
> À: "Remi Forax" , "Stuart Marks" 
> Cc: "core-libs-dev" 
> Envoyé: Mardi 19 Mars 2019 00:58:04
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

>> My fear is more than we will want in the future to have one code for all 
>> kinds
>> of Stream, but Stream will have to implement Iterable while
>> Stream will not, this not something you can actually do with the current
>> generics, we may be able to do that with the reified generics but some
>> languages that already have reified generics like Swift are not able to do
>> that.
>> So by making Stream to have different set of supertypes than Stream, 
>> you
>> are forcing the future reified generics implementation to work on this case
>> because we will never introduce an implementation of reified generics that
>> doesn't support the classe of java.util.
> 
> This will work fine; Stream <: IterableOnce, and when we can
> instantiate T=int in the first, we'll be able to do so in the second as
> well.  (Having spent hundreds of hours banging my head against how we're
> going to migrate collections and stream to specialization, this one is
> not even on the list.)

for some reason i was convinced that IntStream.iterator() was returning a 
PrimitiveIterator.OfInt and not an Iterator,
so yes, it will work but i don't understand why it's not BaseStream instead of 
Stream that inherits from Iterable.

And i still think that add IterableOnce is a very bad idea because it's the 
same trap as trying to introduce an interface UnmodifiableList as subtype of 
List.

Rémi
 


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-18 Thread Brian Goetz




My fear is more than we will want in the future to have one code for all kinds of Stream, but 
Stream will have to implement Iterable while Stream will 
not, this not something you can actually do with the current generics, we may be able to do that 
with the reified generics but some languages that already have reified generics like Swift are 
not able to do that.
So by making Stream to have different set of supertypes than Stream, 
you are forcing the future reified generics implementation to work on this case because we 
will never introduce an implementation of reified generics that doesn't support the classe 
of java.util.


This will work fine; Stream <: IterableOnce, and when we can 
instantiate T=int in the first, we'll be able to do so in the second as 
well.  (Having spent hundreds of hours banging my head against how we're 
going to migrate collections and stream to specialization, this one is 
not even on the list.)


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-18 Thread forax
- Mail original -
> De: "Stuart Marks" 
> À: "Remi Forax" 
> Cc: "core-libs-dev" 
> Envoyé: Samedi 16 Mars 2019 02:04:05
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

>> I'm talking about the perf difference between stream.forEach and for(var
>> element: stream), forEachRemaining may be slower because for the VM the ideal
>> case is to see the creation of the Stream and the call to the terminal
>> operation inside the same inlining horizon so the creation of the Stream 
>> itself
>> can be elided.
>> 
>> A bit of history: they have been several prototypes of how to implement the
>> stream API before the current one, one of them (i think it's the first one) 
>> was
>> based on iterators and iterators of iterators, one for each step of the 
>> Stream.
>> The perf of that implementation was good until there was too many 
>> intermediary
>> ops calls on the Stream and at that point perf were really bad. It's because
>> the VM has two way to find the type of something in a generic code, it can
>> build a profile by remembering what class was used for a method call or it 
>> can
>> propagate the type of an argument to the type of the corresponding parameter.
>> Because an iterator stores the element to return in a field, you are loosing
>> the later way to optimize and the former only work if you have no more than 2
>> different classes in the profile.
>> So while Stream.iterator() may be optimized, it's not that simple.
> 
> Yes, I remember this prototype. Sure, iterating from stream.iterator() will
> likely be slower than stream.forEach(), because of (current) limitations in 
> JIT
> compilation. This may be important for performance-critical applications. So 
> if
> you have such an application, you should be aware of possible performance 
> issues
> using such an iterator(), measure, and recode if necessary.
> 
> Is this an argument not to allow Stream in a for-loop? I don't think so. 
> There's
> a (fairly narrow) set of use cases where it's really necessary, and in most
> cases performance isn't an issue. After all, people use things like
> List which is known to be terrible for large, performance-critical
> applications. But most apps are small and aren't performance critical, and for
> those, it's just fine.

I suppose that if you do a presentation a devoxx on that subject and if IDEs 
recommend to use Stream.forEach instead of the enhanced for loop when possible 
then it will be fine.

> 
>>>> This proposal has the side effect of making Stream more different from its
>>>> primitive counterpart IntStream, LongStream and DoubleStream which may be
>>>> problematic because we are trying to introduce reified generics as part of
>>>> Valhalla (there is a recent mail of Brian about not adding methods to
>>>> OptionalInt for the same reason).
>>>
>>> Well, yes, I think that it means that Stream evolves somewhat independently 
>>> of
>>> Int/Long/DoubleStream, but I don't see that this imposes an impediment on
>>> generic specialization in Valhalla. In that world, Stream should 
>>> (mostly)
>>> just work. It may also be possible in a specialized world to add the 
>>> specific
>>> things from IntStream (such as sum() and max()) to Stream.
>> 
>> We may want more here, like having Stream being a subtype of IntStream 
>> so
>> there is only one implementation for IntStream and Stream.
>> Thus adding a method that make IntStream and Stream different just 
>> make
>> this kind of retrofitting more unlikely.
> 
> I think the argument about specialization runs the other way, which is not to
> add stuff to IntStream.
> 
> Adding IterableOnce to Stream shouldn't really affect anything with respect to
> generic specialization. The type is already Stream. The Iterable methods
> that are inherited (iterator, spliterator, forEach) all match existing methods
> on Stream, at least structurally. So I don't see that this would cause a
> problem.
> 
> (Hm, I note that there is a slight semantic disagreement between
> Iterable::forEach and Stream::forEach. Stream::forEach allows parallelism, 
> which
> isn't mentioned in Iterable::forEach. Somebody could conceivably call
> Iterable::forEach with a consumer that's not thread-safe, and if a parallel
> stream gets passed in, it would break that consumer. This strikes me as an 
> edge
> case to be filed off, rather than a fatal problem, though.)

My fear is more than we will want in the future to have one code for all kinds 
of Stream, but Stream will have to implement Ite

Re: I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-18 Thread Peter Levart




On 3/15/19 10:07 AM, Ivan Gerasimov wrote:



On 3/15/19 1:51 AM, Peter Levart wrote:



On 3/15/19 9:38 AM, Ivan Gerasimov wrote:

Hi Peter!


On 3/15/19 1:24 AM, Peter Levart wrote:



On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote:

  * @since 13
  */
 interface Once {}


What do you think of that?
It's not clear to me if an annotation, available at runtime, is 
not a better fit.
Anyway, i'm not sure not sure introducing such 
interface/annotation worth its maintenance cost, as you said the 
use case is pretty narrow.




It is narrow, but in a situation like that, where you want to code 
an optimal generic algorithm and all you have access to is an 
Iterable, there's no other way (short of providing additional 
methods, which is ugly). Just think of this situation. You have to 
decide upfront if you need to buffer the elements obtained from 1st 
iteration or not, but 1st iteration always succeeds...



Can you please explain how the interface Once would help to solve this?
If an Iterable does not implement Once, it does not mean it allows 
multiple passes, right?


It does not guarantee multiple passes, that's right, but that's 
legacy. This situation is the same for IterableOnce as a subtype of 
Iterable, but marker interface has less chance to intrude into 
"visible" static types that consist of method signatures, type 
parameters and variable types and therefore does not cause confusion 
in that area.


"Once" is not perfect, but allows generic algorithms to work on those 
instances that do implement it at least.



Thanks for clarifying!
My point was that in the situation you described, an interface Many 
(or MultiPass, or how ever it's named) would be more appropriate.
If an Iterable implements it, there is a guarantee.  Otherwise it has 
to be assumed a one-shot Iterable.


With kind regards,
Ivan


Yes, that would be safer, but also less useful as most current 
implementations support multi-pass iteration and would have to be 
updated to implement this new interface. Unless majority of multi-pass 
capable implementations consist of Collection(s) in which case this 
"MultiPass" interface could be added as a super-interface to 
java.util.Collection.


If this is true, then testing against java.util.Collection could be a 
usefull-enough runtime check for the edge cases of hypothetical generic 
algorithms that need multiple passes and we don't even need the 
"MultiPass" interface...


Regards, Peter



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-15 Thread Stuart Marks





I'm talking about the perf difference between stream.forEach and for(var 
element: stream), forEachRemaining may be slower because for the VM the ideal 
case is to see the creation of the Stream and the call to the terminal 
operation inside the same inlining horizon so the creation of the Stream itself 
can be elided.

A bit of history: they have been several prototypes of how to implement the 
stream API before the current one, one of them (i think it's the first one) was 
based on iterators and iterators of iterators, one for each step of the Stream. 
The perf of that implementation was good until there was too many intermediary 
ops calls on the Stream and at that point perf were really bad. It's because 
the VM has two way to find the type of something in a generic code, it can 
build a profile by remembering what class was used for a method call or it can 
propagate the type of an argument to the type of the corresponding parameter. 
Because an iterator stores the element to return in a field, you are loosing 
the later way to optimize and the former only work if you have no more than 2 
different classes in the profile.
So while Stream.iterator() may be optimized, it's not that simple.


Yes, I remember this prototype. Sure, iterating from stream.iterator() will 
likely be slower than stream.forEach(), because of (current) limitations in JIT 
compilation. This may be important for performance-critical applications. So if 
you have such an application, you should be aware of possible performance issues 
using such an iterator(), measure, and recode if necessary.


Is this an argument not to allow Stream in a for-loop? I don't think so. There's 
a (fairly narrow) set of use cases where it's really necessary, and in most 
cases performance isn't an issue. After all, people use things like 
List which is known to be terrible for large, performance-critical 
applications. But most apps are small and aren't performance critical, and for 
those, it's just fine.



This proposal has the side effect of making Stream more different from its
primitive counterpart IntStream, LongStream and DoubleStream which may be
problematic because we are trying to introduce reified generics as part of
Valhalla (there is a recent mail of Brian about not adding methods to
OptionalInt for the same reason).


Well, yes, I think that it means that Stream evolves somewhat independently of
Int/Long/DoubleStream, but I don't see that this imposes an impediment on
generic specialization in Valhalla. In that world, Stream should (mostly)
just work. It may also be possible in a specialized world to add the specific
things from IntStream (such as sum() and max()) to Stream.


We may want more here, like having Stream being a subtype of IntStream so there 
is only one implementation for IntStream and Stream.
Thus adding a method that make IntStream and Stream different just make 
this kind of retrofitting more unlikely.


I think the argument about specialization runs the other way, which is not to 
add stuff to IntStream.


Adding IterableOnce to Stream shouldn't really affect anything with respect to 
generic specialization. The type is already Stream. The Iterable methods 
that are inherited (iterator, spliterator, forEach) all match existing methods 
on Stream, at least structurally. So I don't see that this would cause a problem.


(Hm, I note that there is a slight semantic disagreement between 
Iterable::forEach and Stream::forEach. Stream::forEach allows parallelism, which 
isn't mentioned in Iterable::forEach. Somebody could conceivably call 
Iterable::forEach with a consumer that's not thread-safe, and if a parallel 
stream gets passed in, it would break that consumer. This strikes me as an edge 
case to be filed off, rather than a fatal problem, though.)




And, the real issue is how to deal with checked exceptions inside the Stream
API, i would prefer to fix that issue instead of trying to find a way to
workaround it.


Well I'd like to have a solution for checked exceptions as well, but there
doesn't appear to be one on the horizon. I mean, there are some ideas floating
around, but nobody is working on them as far as I know.


as far as i know, there are two of them,
- one is to get ride of checked exception, even Kotlin which tout itself as a 
language that is more safe that Java doesn't have checked exception, basically 
Java is the only language that run of the JVM and have checked exception.
- the other is to automatically wrap checked exceptions into a corresponding 
unchecked exception by letting the compiler generate the code that users 
currently write when the checked exception appear some context
   by example with the keyword autowrap,
   - you have the autowrap block (syntactically like a synchronized block)
   autowrap {
 return Files.newInputStream(path);   // IOException is transformed to 
UncheckedIOException by calling IOException.wrap()
   }
   - you can use autowrap on a method 

Re: I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-15 Thread Ivan Gerasimov




On 3/15/19 1:51 AM, Peter Levart wrote:



On 3/15/19 9:38 AM, Ivan Gerasimov wrote:

Hi Peter!


On 3/15/19 1:24 AM, Peter Levart wrote:



On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote:

  * @since 13
  */
 interface Once {}


What do you think of that?
It's not clear to me if an annotation, available at runtime, is not 
a better fit.
Anyway, i'm not sure not sure introducing such interface/annotation 
worth its maintenance cost, as you said the use case is pretty narrow.




It is narrow, but in a situation like that, where you want to code 
an optimal generic algorithm and all you have access to is an 
Iterable, there's no other way (short of providing additional 
methods, which is ugly). Just think of this situation. You have to 
decide upfront if you need to buffer the elements obtained from 1st 
iteration or not, but 1st iteration always succeeds...



Can you please explain how the interface Once would help to solve this?
If an Iterable does not implement Once, it does not mean it allows 
multiple passes, right?


It does not guarantee multiple passes, that's right, but that's 
legacy. This situation is the same for IterableOnce as a subtype of 
Iterable, but marker interface has less chance to intrude into 
"visible" static types that consist of method signatures, type 
parameters and variable types and therefore does not cause confusion 
in that area.


"Once" is not perfect, but allows generic algorithms to work on those 
instances that do implement it at least.



Thanks for clarifying!
My point was that in the situation you described, an interface Many (or 
MultiPass, or how ever it's named) would be more appropriate.
If an Iterable implements it, there is a guarantee.  Otherwise it has to 
be assumed a one-shot Iterable.


With kind regards,
Ivan



Regards, Peter



With kind regards,
Ivan





--
With kind regards,
Ivan Gerasimov



Re: I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-15 Thread Peter Levart




On 3/15/19 9:38 AM, Ivan Gerasimov wrote:

Hi Peter!


On 3/15/19 1:24 AM, Peter Levart wrote:



On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote:

  * @since 13
  */
 interface Once {}


What do you think of that?
It's not clear to me if an annotation, available at runtime, is not 
a better fit.
Anyway, i'm not sure not sure introducing such interface/annotation 
worth its maintenance cost, as you said the use case is pretty narrow.




It is narrow, but in a situation like that, where you want to code an 
optimal generic algorithm and all you have access to is an Iterable, 
there's no other way (short of providing additional methods, which is 
ugly). Just think of this situation. You have to decide upfront if 
you need to buffer the elements obtained from 1st iteration or not, 
but 1st iteration always succeeds...



Can you please explain how the interface Once would help to solve this?
If an Iterable does not implement Once, it does not mean it allows 
multiple passes, right?


It does not guarantee multiple passes, that's right, but that's legacy. 
This situation is the same for IterableOnce as a subtype of Iterable, 
but marker interface has less chance to intrude into "visible" static 
types that consist of method signatures, type parameters and variable 
types and therefore does not cause confusion in that area.


"Once" is not perfect, but allows generic algorithms to work on those 
instances that do implement it at least.


Regards, Peter



With kind regards,
Ivan




Re: I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-15 Thread Ivan Gerasimov

Hi Peter!


On 3/15/19 1:24 AM, Peter Levart wrote:



On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote:

  * @since 13
  */
 interface Once {}


What do you think of that?
It's not clear to me if an annotation, available at runtime, is not a 
better fit.
Anyway, i'm not sure not sure introducing such interface/annotation 
worth its maintenance cost, as you said the use case is pretty narrow.




It is narrow, but in a situation like that, where you want to code an 
optimal generic algorithm and all you have access to is an Iterable, 
there's no other way (short of providing additional methods, which is 
ugly). Just think of this situation. You have to decide upfront if you 
need to buffer the elements obtained from 1st iteration or not, but 
1st iteration always succeeds...



Can you please explain how the interface Once would help to solve this?
If an Iterable does not implement Once, it does not mean it allows 
multiple passes, right?


With kind regards,
Ivan

Annotations are not suitable for that as the check has to be quick and 
they don't play well with inheritance etc...


Peter




--
With kind regards,
Ivan Gerasimov



Re: I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-15 Thread Peter Levart




On 3/15/19 9:24 AM, Peter Levart wrote:



On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote:

  * @since 13
  */
     interface Once {}


What do you think of that?

It's not clear to me if an annotation, available at runtime, is not a better 
fit.
Anyway, i'm not sure not sure introducing such interface/annotation worth its 
maintenance cost, as you said the use case is pretty narrow.



It is narrow, but in a situation like that, where you want to code an 
optimal generic algorithm and all you have access to is an Iterable, 
there's no other way (short of providing additional methods, which is 
ugly). Just think of this situation. You have to decide upfront if you 
need to buffer the elements obtained from 1st iteration or not, but 
1st iteration always succeeds...


Ok, I confess, there is a way to do it without marker interface, but is 
ugly:


Iterator iterator1 = iterable.iterator();
Iterator iterator2;
try {
   iterator2 = iterable.iterator();
} catch (IllegalStateException e) {
    // we have one-shot iterable
   iterator2 = null;
}

if (iterator2 == null) {
   // buffer elements of iterator1 while iterating for 2nd and 
subsequent iterations

    ...
} else {
    // consume iterator1, then iterator2, then create more iterators 
for subsequent iterations

    ...
}



Peter



Re: I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-15 Thread Peter Levart




On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote:

  * @since 13
  */
     interface Once {}


What do you think of that?

It's not clear to me if an annotation, available at runtime, is not a better 
fit.
Anyway, i'm not sure not sure introducing such interface/annotation worth its 
maintenance cost, as you said the use case is pretty narrow.



It is narrow, but in a situation like that, where you want to code an 
optimal generic algorithm and all you have access to is an Iterable, 
there's no other way (short of providing additional methods, which is 
ugly). Just think of this situation. You have to decide upfront if you 
need to buffer the elements obtained from 1st iteration or not, but 1st 
iteration always succeeds...


Annotations are not suitable for that as the check has to be quick and 
they don't play well with inheritance etc...


Peter



Re: I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-15 Thread forax
- Mail original -
> De: "Peter Levart" 
> À: "Remi Forax" 
> Cc: "Brian Goetz" , "Stuart Marks" 
> , "core-libs-dev"
> 
> Envoyé: Vendredi 15 Mars 2019 08:57:10
> Objet: Re: I don't understand why we need IterableOnce ? Was: Proposal: 
> JDK-8148917 Enhanced-For Statement Should Allow
> Streams

> Hi,
> 
> On 3/14/19 9:51 PM, Remi Forax wrote:
>> yes, i think i prefer this solution, one Iterable to rule them all.
>>
>> First, it's not in the spirit of the Collection API to multiply the 
>> interfaces,
>> by example, we have only one kind of Iterator and not an Iterator and a
>> ReadOnlyIterator even if a lot of iterators doesn't implement remove. It's a
>> central design of the Collection API, reduce the number of interfaces to ease
>> the use even if it means that each interface may have a broader definition. 
>> The
>> Collection API design has chosen his side between users and library writers
>> (people that provides implementations) because from the library writer point 
>> of
>> view you can not specify exactly the semantics you want.
>>
>> Then from the user POV, what is point of IterableOnce ? I will not using it 
>> as
>> parameter because using Iterable is a super-type (like i will use a List
>> instead of an ArrayList as parameter) and if i using it as return type, codes
>> that call that method can put it in an Iterable, this is exactly what the
>> for-each-loop will do BTW, so it's seems useless.
>>
>> Also as Peter said, there are already codes in the wild that create an 
>> Iterable
>> that can only be iterated once, ASM has such class, if IterableOnce is added 
>> to
>> the JDK, i will have people ask me to retrofit the ASM class to use
>> IterableOnce just for the sake of having the right semantics ? So basically 
>> by
>> introducing IterableOnce, all codes that were perfectly fine in term of
>> semantics before introducing IterableOnce are now not quite right because 
>> they
>> are not implementing the right interface ? Hum, i think i still not get why 
>> we
>> need such interface.
>>
>> Rémi
> 
> The IterableOnce really does not have utility as a static type and I
> think it might only confuse some (Should I return IterableOnce here?
> Should I consume IterableOnce here? Many would not get the difference at
> first). So in this respect it would be better that there is no
> IterableOnce. But there is a runtime utility (although a marginal one)
> where the code can take two branches depending on iterable implementing
> IterableOnce or not. Similar to RandomAccess marker interface with
> List(s). So the question is whether this situation would be better
> served by introducing an unrelated marker interface instead of a subtype
> of Iterable? For example:
> 
> 
> /**
>  * Implementing this interface allows an object to be the target of the
> enhanced
>  * {@code for} statement (sometimes called the "for-each loop" statement).
>  * 
>  * There are generally two kinds of {@code Iterable} implementations:
>  * 
>  * Those that allow
>  * multiple calls to {@link #iterator()} each returning new instance
> which can
>  * be used for independent iterations over elements. {@link
> java.util.Collection}s
>  * are general representatives of this type of {@code Iterable}s.
>  * 
>  * And those that allow only one call to {@link #iterator()},
> providing a
>  * single iteration and throwing {@link IllegalStateException} on 2nd
> and subsequent
>  * calls. It is recommended that this kind of implementations also
> implement
>  * {@link Once} marker interface to allow code to detect the kind of
> {@code Iterable}
>  * during runtime. {@link java.util.stream.Stream} is an example
> implementation
>  * of this type of {@code Iterable}.
>  * 
>  * 
>  *
>  * @param  the type of elements returned by the iterator
>  * @jls 14.14.2 The enhanced {@code for} statement
>  * @since 1.5
>  */
> public interface Iterable {
> 
>     /**
>  * Marker interface used by {@code Iterable} implementations to
> indicate that
>  * they support only a single call to {@link #iterator()} method
> while 2nd and
>  * subsequent calls throw {@link IllegalStateException}. The primary
>  * purpose of this interface is to allow generic algorithms to
> alter their
>  * behavior if they need multiple passes over elements of {@link
> Iterable}.
>  *
>  * @since 13
>  */
>     interface Once {}
> 
> 
> What do you think of that?

It's not clear to me if an annotation, available at runtime, is not a better 
fit.
Anyway, i'm not sure not sure introducing such interface/annotation worth its 
maintenance cost, as you said the use case is pretty narrow.

> 
> Regards, Peter

Rémi


Re: I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-15 Thread Peter Levart

Hi,

On 3/14/19 9:51 PM, Remi Forax wrote:

yes, i think i prefer this solution, one Iterable to rule them all.

First, it's not in the spirit of the Collection API to multiply the interfaces, 
by example, we have only one kind of Iterator and not an Iterator and a 
ReadOnlyIterator even if a lot of iterators doesn't implement remove. It's a 
central design of the Collection API, reduce the number of interfaces to ease 
the use even if it means that each interface may have a broader definition. The 
Collection API design has chosen his side between users and library writers 
(people that provides implementations) because from the library writer point of 
view you can not specify exactly the semantics you want.

Then from the user POV, what is point of IterableOnce ? I will not using it as 
parameter because using Iterable is a super-type (like i will use a List 
instead of an ArrayList as parameter) and if i using it as return type, codes 
that call that method can put it in an Iterable, this is exactly what the 
for-each-loop will do BTW, so it's seems useless.

Also as Peter said, there are already codes in the wild that create an Iterable 
that can only be iterated once, ASM has such class, if IterableOnce is added to 
the JDK, i will have people ask me to retrofit the ASM class to use 
IterableOnce just for the sake of having the right semantics ? So basically by 
introducing IterableOnce, all codes that were perfectly fine in term of 
semantics before introducing IterableOnce are now not quite right because they 
are not implementing the right interface ? Hum, i think i still not get why we 
need such interface.

Rémi


The IterableOnce really does not have utility as a static type and I 
think it might only confuse some (Should I return IterableOnce here? 
Should I consume IterableOnce here? Many would not get the difference at 
first). So in this respect it would be better that there is no 
IterableOnce. But there is a runtime utility (although a marginal one) 
where the code can take two branches depending on iterable implementing 
IterableOnce or not. Similar to RandomAccess marker interface with 
List(s). So the question is whether this situation would be better 
served by introducing an unrelated marker interface instead of a subtype 
of Iterable? For example:



/**
 * Implementing this interface allows an object to be the target of the 
enhanced

 * {@code for} statement (sometimes called the "for-each loop" statement).
 * 
 * There are generally two kinds of {@code Iterable} implementations:
 * 
 * Those that allow
 * multiple calls to {@link #iterator()} each returning new instance 
which can
 * be used for independent iterations over elements. {@link 
java.util.Collection}s

 * are general representatives of this type of {@code Iterable}s.
 * 
 * And those that allow only one call to {@link #iterator()}, 
providing a
 * single iteration and throwing {@link IllegalStateException} on 2nd 
and subsequent
 * calls. It is recommended that this kind of implementations also 
implement
 * {@link Once} marker interface to allow code to detect the kind of 
{@code Iterable}
 * during runtime. {@link java.util.stream.Stream} is an example 
implementation

 * of this type of {@code Iterable}.
 * 
 * 
 *
 * @param  the type of elements returned by the iterator
 * @jls 14.14.2 The enhanced {@code for} statement
 * @since 1.5
 */
public interface Iterable {

    /**
 * Marker interface used by {@code Iterable} implementations to 
indicate that
 * they support only a single call to {@link #iterator()} method 
while 2nd and

 * subsequent calls throw {@link IllegalStateException}. The primary
 * purpose of this interface is to allow generic algorithms to 
alter their
 * behavior if they need multiple passes over elements of {@link 
Iterable}.

 *
 * @since 13
 */
    interface Once {}


What do you think of that?

Regards, Peter



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-15 Thread Peter Levart




On 3/15/19 12:16 AM, Stephen Colebourne wrote:

On Thu, 14 Mar 2019 at 19:45, Brian Goetz  wrote:

Why not make `Iterator` implement `IterableOnce`? The default method
would obviously just return `this`.

Such a default would not conform to the contract, as IO requires that 
subsequent calls throw.

IterableOnce.wrap(iterator) ?

Not providing some kind of connection between these types will look
pretty silly I think.
Stephen
That makes sense, yes. As a utility method for adapting in situations 
where there's an unconsumed Iterator at hand and you have to provide a 
"well behaved" IterableOnce. I would call it IterableOnce.of(Iterator) 
as it does not really adapt the Iterator, but creates a factory with 
provided Iterator. Using this would be preferable to sprinkling ad-hoc 
expressions like:


(IterableOnce) () -> iterator

as such IterableOnce instances would not conform to the spec.

Regards, Peter



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Stephen Colebourne
On Thu, 14 Mar 2019 at 19:45, Brian Goetz  wrote:
> Why not make `Iterator` implement `IterableOnce`? The default method
> would obviously just return `this`.
>
> Such a default would not conform to the contract, as IO requires that 
> subsequent calls throw.

IterableOnce.wrap(iterator) ?

Not providing some kind of connection between these types will look
pretty silly I think.
Stephen


I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Remi Forax
- Mail original -
> De: "Peter Levart" 
> À: "Brian Goetz" , "Stuart Marks" 
> , "core-libs-dev"
> 
> Envoyé: Mardi 12 Mars 2019 18:34:58
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> On 3/12/19 5:04 PM, Brian Goetz wrote:
>> No. You have the LSP backwards (though this is easy to do.)
>>
>> IterableOnce means "*must throw* on subsequent use"; under this spec,
>> an arbitrary Iterable is most certainly *not* an IterableOnce, and
>> therefore an LSP violation.
>>
>> It sounds like you are suggesting that we instead spec
>> IterableAtLeastOnce, of which Iterable *would* be a credible subtype
>> -- but due to how Iterable is specified now, the problem is that
>> Iterable *already is* really "iterable at least once."  So there would
>> be no point in introducing this type; we already have it.
> 
> Due to how Iterable is specified now it does not promise multiple
> iterations, but due to how most (all that I know of except
> DirectoryStream or some 3rd party) Iterables are implemented now, the
> common expectation is that it does. By implementing more Iterable(s)
> that don't support multiple iteration (regardless of whether they
> implement this new sub-type or not), this expectation will be less and
> less honored. Perhaps this is not so bad. At various occasions the
> specification has been changed to accommodate the long-standing behavior
> or expectation of behavior, but this seems not to be one of them.
> 
> Let's pretend for a moment that Iterable specification was changed to
> guarantee multi-pass iteration. In that case the IterableAtLeastOnce as
> a supertype of multi-pass Iterable would make much more sense, wouldn't it?
> 
> But as there could be Iterables out there that by current spec are
> perfectly valid and don't support multi-pass iteration, such spec change
> would make them non-conformant and is therefore not allowed. So I'm
> convinced now that this is the least-bad solution.
> 
> Regards, Peter

yes, i think i prefer this solution, one Iterable to rule them all.

First, it's not in the spirit of the Collection API to multiply the interfaces, 
by example, we have only one kind of Iterator and not an Iterator and a 
ReadOnlyIterator even if a lot of iterators doesn't implement remove. It's a 
central design of the Collection API, reduce the number of interfaces to ease 
the use even if it means that each interface may have a broader definition. The 
Collection API design has chosen his side between users and library writers 
(people that provides implementations) because from the library writer point of 
view you can not specify exactly the semantics you want.

Then from the user POV, what is point of IterableOnce ? I will not using it as 
parameter because using Iterable is a super-type (like i will use a List 
instead of an ArrayList as parameter) and if i using it as return type, codes 
that call that method can put it in an Iterable, this is exactly what the 
for-each-loop will do BTW, so it's seems useless.

Also as Peter said, there are already codes in the wild that create an Iterable 
that can only be iterated once, ASM has such class, if IterableOnce is added to 
the JDK, i will have people ask me to retrofit the ASM class to use 
IterableOnce just for the sake of having the right semantics ? So basically by 
introducing IterableOnce, all codes that were perfectly fine in term of 
semantics before introducing IterableOnce are now not quite right because they 
are not implementing the right interface ? Hum, i think i still not get why we 
need such interface.

Rémi


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread forax
yes,i should have been more specific, you can not *without* having some boxing 
in the middle.

Rémi

- Mail original -
> De: "Tagir Valeev" 
> À: "Remi Forax" 
> Cc: "Stuart Marks" , "core-libs-dev" 
> 
> Envoyé: Jeudi 7 Mars 2019 11:33:20
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> Hello, Remi!
> 
> It actually works thanks to auto-boxing/unboxing. E.g. this
> implementation works:
> 
> static Iterable range(int from, int to) {
>  return () -> new PrimitiveIterator.OfInt() {
>int cur = from;
> 
>@Override
>public int nextInt() {
>  if (cur >= to) {
>throw new NoSuchElementException();
>  }
>  return cur++;
>}
> 
>@Override
>public boolean hasNext() {
>  return cur < to;
>}
>  };
> }
> 
> public static void main(String[] args) {
>  for(int i : range(0, 100)) {
>System.out.println(i);
>  }
> }
> 
> It correctly compiles and prints numbers from 0 to 99. As IntStream
> extends BaseStream and BaseStream BaseStream> defines Iterator iterator(), it would be no
> problem with using IntStream.range in such code pattern were
> BaseStream extends IterableOnce.
> 
> Of course this produces unnecessary garbage, as I said.
> 
> With best regards,
> Tagir Valeev.
> 
> On Wed, Mar 6, 2019 at 7:37 PM Remi Forax  wrote:
>>
>> Hi Tagir,
>> try to do it now and you will see that you can't, because you can not write
>> Iterable yet.
>> Once we will get generics over value types, it will be a no-brainer.
>>
>> Rémi
>>
>> - Mail original -
>> > De: "Tagir Valeev" 
>> > À: "Stuart Marks" 
>> > Cc: "core-libs-dev" 
>> > Envoyé: Mercredi 6 Mars 2019 11:10:41
>> > Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow 
>> > Streams
>>
>> > Hello!
>> >
>> > By the way one of the painful code patterns in modern Java is `for(int
>> > i = 0; i> > newbies and prone to errors as the variable need to be repeated three
>> > times. Also the variable is not effectively final, despite it never
>> > changes within the loop body, so could have been considered as
>> > redeclared on every loop iteration (like in for-each). With the new
>> > proposal it's possible to write `for(int i : range(0, bound).boxed())`
>> > (assuming import static j.u.s.IntStream.range), which looks much
>> > better, though it has obvious performance drawback. Moving
>> > IterableOnce to BaseStream would enable to use `for(int i : range(0,
>> > bound))` which looks even better, though again we have plenty of
>> > garbage (but considerably less than in previous case!). I wonder
>> > whether Java could evolve to the point where such kind of code would
>> > be a recommended way to iterate over subsequent integer values without
>> > any performance handicap.
>> >
>> > With best regards,
>> > Tagir Valeev.
>> >
>> > On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks  
>> > wrote:
>> >>
>> >> Hi all,
>> >>
>> >> Please review and comment on this proposal to allow Stream instances to 
>> >> be used
>> >> in enhanced-for ("for-each") loops.
>> >>
>> >> Abstract
>> >>
>> >> Occasionally it's useful to iterate a Stream using a conventional loop. 
>> >> However,
>> >> the Stream interface doesn't implement Iterable, and therefore streams 
>> >> cannot be
>> >> used with the enhanced-for statement. This is a proposal to remedy that
>> >> situation by introducing a new interface IterableOnce that is a subtype of
>> >> Iterable, and then retrofitting the Stream interface to implement it. 
>> >> Other JDK
>> >> classes will also be retrofitted to implement IterableOnce.
>> >>
>> >> Full Proposal:
>> >>
>> >>  http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>> >>
>> >> Bug report:
>> >>
>> >>  https://bugs.openjdk.java.net/browse/JDK-8148917
>> >>
>> >> Webrev:
>> >>
>> >>  http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>> >>
>> >> Note, this changeset isn't ready to push yet. In particular, it has no 
>> >> tests
>> >> yet. However, the implementation is so simple that I figured I should 
>> >> include
>> >> it. Comments on the specification wording are also welcome.
>> >>
>> >> Thanks,
>> >>
> > > > s'marks


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Brian Goetz
> A new concern from me is that this change would allow Iterable and
> Stream to be used in foreach, but not Iterator. This seems like an
> odd/sharp conceptual edge.

Not actually a new concern — this was discussed back in JSR 335 as well.  

> Why not make `Iterator` implement `IterableOnce`? The default method
> would obviously just return `this`.

Such a default would not conform to the contract, as IO requires that 
subsequent calls throw.  

Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Peter Levart
Now that Brian put it so nicely, I'm convinced and I'd like to draw back 
and support this proposal as is...


On 3/14/19 3:50 PM, Stephen Colebourne wrote:

On Thu, 14 Mar 2019 at 14:21, Brian Goetz  wrote:

There's a reason it took as long as it did for Stuart to come up with
this proposal; all the options were known for years, they all have
problems, and the net benefit is still relatively narrow, which means we
don't have a lot of budget before the cost-benefit becomes negative.  I
think the option proposed is the least-worst, and people still seem to
really want to be able to foreach over streams.

The cost-benefit is definitely tight, but I think it is positive. As I
said here [1] there is still a case for control abstractions in Java
even when you have lambdas


A new concern from me is that this change would allow Iterable and
Stream to be used in foreach, but not Iterator. This seems like an
odd/sharp conceptual edge.


To be precise: this change would not allow Iterable and Stream to be 
used in foreach. foreach does not change. It would still alow only 
Iterable (and arrays). The Stream would become an Iterable with this change.




Why not make `Iterator` implement `IterableOnce`? The default method
would obviously just return `this`.


This would conflate concepts to much. Iterator *is* an external 
iteration state while Iterable[Once] is a (one-shot) factory for 
Iterator. The specification for IterableOnce could not say that the 
second and subsequent invocations of iterator() method are to throw 
exception if Iterator interface extended IterableOnce (interfaces have 
no state to model this in the default method).





It seems to me that if we are willing to countenance foreach over a
one-shot Stream, it is inappropriate to deny foreach to a one-shot
Iterator.


Again, this proposal does not do anything to foreach specification. It 
just makes Stream be Iterable. The concepts here are more to the point:


Iterable: factory for Iterator
Stream: one-shot factory for Spliterator
Stream extends IterableOnce: one-shot factory for Spliterator and 
one-shot factory for Iterator


I think this makes perfect sense.

Regards, Peter



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Stephen Colebourne
On Thu, 14 Mar 2019 at 14:21, Brian Goetz  wrote:
> There's a reason it took as long as it did for Stuart to come up with
> this proposal; all the options were known for years, they all have
> problems, and the net benefit is still relatively narrow, which means we
> don't have a lot of budget before the cost-benefit becomes negative.  I
> think the option proposed is the least-worst, and people still seem to
> really want to be able to foreach over streams.

The cost-benefit is definitely tight, but I think it is positive. As I
said here [1] there is still a case for control abstractions in Java
even when you have lambdas


A new concern from me is that this change would allow Iterable and
Stream to be used in foreach, but not Iterator. This seems like an
odd/sharp conceptual edge.

Why not make `Iterator` implement `IterableOnce`? The default method
would obviously just return `this`.

It seems to me that if we are willing to countenance foreach over a
one-shot Stream, it is inappropriate to deny foreach to a one-shot
Iterator.

thanks
Stephen

[1] https://mail.openjdk.java.net/pipermail/amber-dev/2019-March/004127.html


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Brian Goetz




It think this alternative is not given fair comparison. 1st this is an 
instance method, so the foreach loop should read:


for (T t : stream.asIterable()) {
    ...
}


Let's keep sight of the goal here, which is: people find it a gap that 
Stream does not play cleanly with foreach.  And the main knock against 
this approach (besides the chorus of "that's not what we wanted" we are 
sure to get) is that it is not really much more discoverable than some 
of the other workarounds (such as casting stream::iterator to Iterable.)





And now for something more controversial...

Is changing the language really out of the picture here?


Yes.  This issue was extensively litigated during JSR-335, and it was 
decided that one language-to-library tunnel (Iterable) here was all we 
wanted.  And there's been no new evidence since then that we want to 
change the language for this.


There's a reason it took as long as it did for Stuart to come up with 
this proposal; all the options were known for years, they all have 
problems, and the net benefit is still relatively narrow, which means we 
don't have a lot of budget before the cost-benefit becomes negative.  I 
think the option proposed is the least-worst, and people still seem to 
really want to be able to foreach over streams.




Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Peter Levart

Hi Stuart,

The Alternatives section of the proposal is very thorough and it 
mentions the following alternative:


"
    An alternative adapter is to add a default method to Stream:

    default Iterable asIterable() { return this::iterator; }

    for (T t : asIterable(stream)) {
        ...
    }

    This is slightly better at the call site, and it’s restricted to 
streams. But it’s still not as nice as IterableOnce and it facilitates 
creation of poorly-behaved Iterable instances.

"

It think this alternative is not given fair comparison. 1st this is an 
instance method, so the foreach loop should read:


for (T t : stream.asIterable()) {
    ...
}

...which is better in particular when creation of Stream and consumption 
is performed in single expression (not a good example, since it doesn't 
close the stream):


for (String line : Files.lines(path).asIterable()) {
    ...
}

In addition, why should this be restricted to streams? This could be 
combined with IterableOnce. The method could be called differently and 
specified as BaseStream's terminal operation, returning IterableOnce 
instead of Iterable:


    /**
 * Returns an {@link IterableOnce} for elements of this stream.
 *
 * This is a terminal
 * operation.
 *
 * @return iterable for elements of this stream that may be 
iterated only once

 */
    default IterableOnce iterate() {
    Iterator iterator = iterator();
    return () -> iterator; // this should be a proper IterableOnce 
with exception thrown etc.

    }

So IntStream and friends would get this too, although with necessary 
boxing, which should be easy for JIT to eliminate and without conflicts 
between IntStream.forEach and Iterable.forEach.



What do you think? Is this additional method invocation too much of a 
drawback?



And now for something more controversial...

Is changing the language really out of the picture here?

The Iterator interface has got a new default method called 
forEachRemaining in JDK 8. This method could be seen roughly equivalent 
to one-shot iteration directly from IterableOnce:


IterableOnce.forEach() vs. IterableOnce.iterator().forEachReamining()

Both of above can only be performed once. And once is enough for foreach 
loop. So if foreach loop was retrofitted to also act on Iterator(s) as a 
possible right hand side (arrays, Iterable(s), Iterator(s)), then we 
might not need this IterableOnce at all. You could then just write:


for (T t : stream.iterator()) ...

But that's probably not going to happen right?


Regards, Peter


On 3/12/19 10:59 PM, Stuart Marks wrote:

Hi Stephen,


My slight concern is that the terminal operation is hidden and not
immediately visible, which could be surprising. I do note that streams
throw a clear exception if a terminal operation is called a second
time which mitigates this to some degree.


Yes, this is certainly a possibility.

I'll note that even though my example does it, I think that storing a 
stream in a local variable is a bit of a code smell. It has to be done 
for try-with-resources, but storing the head of a pipeline in a local 
variable so that it can be iterated over does introduce the 
possibility of accidental reuse.


I suspect that most cases (aside from try-with-resources) will call a 
method returning a fresh Stream that's then iterated over. For example,


    for (var x : getSomeStream()) {
   // ...
    }

In this case there's no possibility of accidental reuse.


The IterableOnce class-level Javadoc contradicts the method-level
Javadoc. The class-level say "It is recommended that" whereas the
method-level Javadoc mandates.


Oh yes, thanks. I'll fix this to make the behavior mandatory.

s'marks





Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread forax
- Mail original -
> De: "Stuart Marks" 
> À: "Remi Forax" 
> Cc: "core-libs-dev" 
> Envoyé: Mardi 12 Mars 2019 22:45:12
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> Hi Remi,
> 
>> Stream.iterator() can be really really slow, it uses a pull semantics while
>> the whole Stream push values. When designing it, the lambda EG saw it as an
>> "escape hatch" in order to interropt with a legacy code than require an
>> Iterator and not more.
> 
> If Stream.iterator() is slow, then perhaps it needs to be optimized. Tagir had
> some ideas for how to do that. Of course, I don't know if that's exactly the
> right way; some additional investigation should be done. But poor performance
> relative to Spliterator.tryAdvance() or forEachRemaining() shouldn't be an
> argument against doing this. People repeatedly bump into the gap in the
> programming model between streams and the enhanced-for loop, and it's time to
> fill it in.

I'm talking about the perf difference between stream.forEach and for(var 
element: stream), forEachRemaining may be slower because for the VM the ideal 
case is to see the creation of the Stream and the call to the terminal 
operation inside the same inlining horizon so the creation of the Stream itself 
can be elided.

A bit of history: they have been several prototypes of how to implement the 
stream API before the current one, one of them (i think it's the first one) was 
based on iterators and iterators of iterators, one for each step of the Stream. 
The perf of that implementation was good until there was too many intermediary 
ops calls on the Stream and at that point perf were really bad. It's because 
the VM has two way to find the type of something in a generic code, it can 
build a profile by remembering what class was used for a method call or it can 
propagate the type of an argument to the type of the corresponding parameter. 
Because an iterator stores the element to return in a field, you are loosing 
the later way to optimize and the former only work if you have no more than 2 
different classes in the profile.
So while Stream.iterator() may be optimized, it's not that simple.

> 
>> This proposal has the side effect of making Stream more different from its
>> primitive counterpart IntStream, LongStream and DoubleStream which may be
>> problematic because we are trying to introduce reified generics as part of
>> Valhalla (there is a recent mail of Brian about not adding methods to
>> OptionalInt for the same reason).
> 
> Well, yes, I think that it means that Stream evolves somewhat independently of
> Int/Long/DoubleStream, but I don't see that this imposes an impediment on
> generic specialization in Valhalla. In that world, Stream should (mostly)
> just work. It may also be possible in a specialized world to add the specific
> things from IntStream (such as sum() and max()) to Stream.

We may want more here, like having Stream being a subtype of IntStream so 
there is only one implementation for IntStream and Stream.
Thus adding a method that make IntStream and Stream different just make 
this kind of retrofitting more unlikely. 

> 
>> And, the real issue is how to deal with checked exceptions inside the Stream
>> API, i would prefer to fix that issue instead of trying to find a way to
>> workaround it.
> 
> Well I'd like to have a solution for checked exceptions as well, but there
> doesn't appear to be one on the horizon. I mean, there are some ideas floating
> around, but nobody is working on them as far as I know.

as far as i know, there are two of them,
- one is to get ride of checked exception, even Kotlin which tout itself as a 
language that is more safe that Java doesn't have checked exception, basically 
Java is the only language that run of the JVM and have checked exception. 
- the other is to automatically wrap checked exceptions into a corresponding 
unchecked exception by letting the compiler generate the code that users 
currently write when the checked exception appear some context
  by example with the keyword autowrap,
  - you have the autowrap block (syntactically like a synchronized block)
  autowrap {
return Files.newInputStream(path);   // IOException is transformed to 
UncheckedIOException by calling IOException.wrap()
  }
  - you can use autowrap on a method declaration
 void foo(Path path) autowrap {
   return Files.newInputStream(path);   // IOException is transformed to 
UncheckedIOException by calling IOException.wrap()
 }
  - you can use autowrap with a functional interface
 void runBlock(autoWrap Consumer consumer) { ... }
 ...
 runblock(() -> {
   Files.newInputStream(path); // IOException is transformed to 
UncheckedIOException by calling IOException.wr

Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-13 Thread Ivan Gerasimov

Hi Stuart!


On 3/13/19 5:45 PM, Stuart Marks wrote:


On 3/12/19 4:32 PM, Ivan Gerasimov wrote:
If there were two new subtypes of Iterable introduced:  IterableOnce 
and IterableMultipleTimes, then all existing Iterables could be 
retrofitted to implement one of these.


It wouldn't *automatically* solve the problem of 3rd party API, which 
accepts an Iterable and iterates it multiple times, but would provide 
a means to change that API in a straightforward way.


Also, the static analysis tools could issue a suggestion for code 
that iterates Iterable more than once to switch to 
IterableMultipleTimes, so that it won't be possible to pass in an 
IterableOnce.


It would also allow to strengthen the specification for 
IterableMultipleTimes and state that all classes implementing it must 
make it possible to obtain multiple iterators.


Hi Ivan,

This would be a cleaner type hierarchy, in the sense that it would 
avoid the subtype-or-supertype issue that's being discussed in a 
parallel segment of this thread. But practically speaking I don't 
think it adds much value. Most Iterables can be iterated multiple 
times; it seems to be the exceptional case that an Iterable is 
once-only. (In the JDK, the examples of Scanner and Stream show that 
things with once-only behavior avoided implementing Iterable at all.)


Yes, I agree that IterableOnce will likely be rare, comparing to 
IterableMany.
While we could migrate the JDK classes to IterableMany (or whatever it 
would be called), I think it's unrealistic to expect that all the 
Iterable implementations out there in the wild would migrate. We'd 
thus be left with Iterable and IterableMany meaning more-or-less the 
same thing in perpetuity.


I'm thinking about a use case with a method that accepts Iterable and 
*needs* to traverse it multiple times.
(You actually gave such example in the proposal: assertThat(Iterable) 
from AssertJ).


With IterableMany it could be handled like this:

void foo(Iterable it) {
if (!(it instanceof IterableMany)) {
var list = new ArrayList();
it.forEach(list::add);
it = list;
}
for (Object x : it) { ... }
for (Object x : it) { ... }
}

On the other hand, checking if the argument is instanceof IterableOnce 
wouldn't help because the base class Iterable doesn't provide guaranties 
w.r.t number of traversals.


With kind regards,
Ivan



By introducing IterableOnce, we can attract retrofits by once-off 
things (like Scanner and Streams) and the uncommon occurrences of 
once-off things like DirectoryStream that currently implement Iterable 
can be migrated to IterableOnce. Everybody else can then just stay on 
Iterable.


s'marks



--
With kind regards,
Ivan Gerasimov



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-13 Thread Stuart Marks




On 3/5/19 12:43 PM, Mike Duigou wrote:
I don't believe that I would use the proposed enhancement to the for statement. 
For me there is cognitive load reduction in using a symmetrical method for all 
iterations even if they end up being a little more complicated for individual 
cases. Usually, for me, I use streams. Even the more complicated patterns such 
as the presented example will hopefully be familiar because they are repeated in 
many places throughout the code. Truly unusual or unique usages are hopefully 
very rare.


To choose between old style for, enhanced for, or streams based on which warts 
are to be avoided is just frustrating. Mixing idioms or required mixing of 
idioms produces the least satisfying result. Was the use of a particular idiom a 
necessity? A choice? Of no consequence? This gets harder to decode with a larger 
number of available idioms.


I suspect the cases for having mixed-idiom code, as in my example showing a 
stream head and a for-loop "tail", are somewhat rare. Maybe less than 5% of 
cases. But it least it's there when you need it, without requiring any subtle 
workarounds.


I think a more likely benefit is that this can reduce the pressure on the 
question of whether an API should return a Collection or a Stream. One 
consideration in making such a decision is whether the caller is more likely to 
iterate or stream the result. If it's likely the caller will want to iterate, 
this tips the decision toward returning a collection. But returning a collection 
usually requires creating and storing all the elements, losing laziness, so this 
is an uncomfortable tradeoff.


With the ability to iterate a Stream, it's possible for an API to return a 
Stream, while letting the caller choose between adding stream operations or 
iterating it, without losing laziness.


s'marks


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-13 Thread Stuart Marks




On 3/12/19 4:32 PM, Ivan Gerasimov wrote:
If there were two new subtypes of Iterable introduced:  IterableOnce and 
IterableMultipleTimes, then all existing Iterables could be retrofitted to 
implement one of these.


It wouldn't *automatically* solve the problem of 3rd party API, which accepts an 
Iterable and iterates it multiple times, but would provide a means to change 
that API in a straightforward way.


Also, the static analysis tools could issue a suggestion for code that iterates 
Iterable more than once to switch to IterableMultipleTimes, so that it won't be 
possible to pass in an IterableOnce.


It would also allow to strengthen the specification for IterableMultipleTimes 
and state that all classes implementing it must make it possible to obtain 
multiple iterators.


Hi Ivan,

This would be a cleaner type hierarchy, in the sense that it would avoid the 
subtype-or-supertype issue that's being discussed in a parallel segment of this 
thread. But practically speaking I don't think it adds much value. Most 
Iterables can be iterated multiple times; it seems to be the exceptional case 
that an Iterable is once-only. (In the JDK, the examples of Scanner and Stream 
show that things with once-only behavior avoided implementing Iterable at all.) 
While we could migrate the JDK classes to IterableMany (or whatever it would be 
called), I think it's unrealistic to expect that all the Iterable 
implementations out there in the wild would migrate. We'd thus be left with 
Iterable and IterableMany meaning more-or-less the same thing in perpetuity.


By introducing IterableOnce, we can attract retrofits by once-off things (like 
Scanner and Streams) and the uncommon occurrences of once-off things like 
DirectoryStream that currently implement Iterable can be migrated to 
IterableOnce. Everybody else can then just stay on Iterable.


s'marks


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-12 Thread Ivan Gerasimov

Hello!

Just an observation.

If there were two new subtypes of Iterable introduced:  IterableOnce and 
IterableMultipleTimes, then all existing Iterables could be retrofitted 
to implement one of these.


It wouldn't *automatically* solve the problem of 3rd party API, which 
accepts an Iterable and iterates it multiple times, but would provide a 
means to change that API in a straightforward way.


Also, the static analysis tools could issue a suggestion for code that 
iterates Iterable more than once to switch to IterableMultipleTimes, so 
that it won't be possible to pass in an IterableOnce.


It would also allow to strengthen the specification for 
IterableMultipleTimes and state that all classes implementing it must 
make it possible to obtain multiple iterators.


With kind regards,
Ivan


On 2/28/19 6:43 PM, Stuart Marks wrote:

Hi all,

Please review and comment on this proposal to allow Stream instances 
to be used in enhanced-for ("for-each") loops.


Abstract

Occasionally it's useful to iterate a Stream using a conventional 
loop. However, the Stream interface doesn't implement Iterable, and 
therefore streams cannot be used with the enhanced-for statement. This 
is a proposal to remedy that situation by introducing a new interface 
IterableOnce that is a subtype of Iterable, and then retrofitting the 
Stream interface to implement it. Other JDK classes will also be 
retrofitted to implement IterableOnce.


Full Proposal:

http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html

Bug report:

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

Webrev:

http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/

Note, this changeset isn't ready to push yet. In particular, it has no 
tests yet. However, the implementation is so simple that I figured I 
should include it. Comments on the specification wording are also 
welcome.


Thanks,

s'marks



--
With kind regards,
Ivan Gerasimov



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-12 Thread Stuart Marks
Due to how Iterable is specified now it does not promise multiple iterations, 
but due to how most (all that I know of except DirectoryStream or some 3rd 
party) Iterables are implemented now, the common expectation is that it does. 
By implementing more Iterable(s) that don't support multiple iteration 
(regardless of whether they implement this new sub-type or not), this 
expectation will be less and less honored. Perhaps this is not so bad. At 
various occasions the specification has been changed to accommodate the 
long-standing behavior or expectation of behavior, but this seems not to be 
one of them.


Correct.  But, with separate Iterable and IterableOnce, the spec for Iterable 
can say "If you don't support multiple iteration, you should implement IO."  
Which is OK, because at least those classes are being explicit.


Right. This sort of clarification should be done as part of JDK-8186220.

s'marks



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-12 Thread Stuart Marks

Hi Stephen,


My slight concern is that the terminal operation is hidden and not
immediately visible, which could be surprising. I do note that streams
throw a clear exception if a terminal operation is called a second
time which mitigates this to some degree.


Yes, this is certainly a possibility.

I'll note that even though my example does it, I think that storing a stream in 
a local variable is a bit of a code smell. It has to be done for 
try-with-resources, but storing the head of a pipeline in a local variable so 
that it can be iterated over does introduce the possibility of accidental reuse.


I suspect that most cases (aside from try-with-resources) will call a method 
returning a fresh Stream that's then iterated over. For example,


for (var x : getSomeStream()) {
   // ...
}

In this case there's no possibility of accidental reuse.


The IterableOnce class-level Javadoc contradicts the method-level
Javadoc. The class-level say "It is recommended that" whereas the
method-level Javadoc mandates.


Oh yes, thanks. I'll fix this to make the behavior mandatory.

s'marks



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-12 Thread Stuart Marks




Hi Remi,


Stream.iterator() can be really really slow, it uses a pull semantics while
the whole Stream push values. When designing it, the lambda EG saw it as an
"escape hatch" in order to interropt with a legacy code than require an
Iterator and not more.


If Stream.iterator() is slow, then perhaps it needs to be optimized. Tagir had 
some ideas for how to do that. Of course, I don't know if that's exactly the 
right way; some additional investigation should be done. But poor performance 
relative to Spliterator.tryAdvance() or forEachRemaining() shouldn't be an 
argument against doing this. People repeatedly bump into the gap in the 
programming model between streams and the enhanced-for loop, and it's time to 
fill it in.



This proposal has the side effect of making Stream more different from its
primitive counterpart IntStream, LongStream and DoubleStream which may be
problematic because we are trying to introduce reified generics as part of
Valhalla (there is a recent mail of Brian about not adding methods to
OptionalInt for the same reason).


Well, yes, I think that it means that Stream evolves somewhat independently of 
Int/Long/DoubleStream, but I don't see that this imposes an impediment on 
generic specialization in Valhalla. In that world, Stream should (mostly) 
just work. It may also be possible in a specialized world to add the specific 
things from IntStream (such as sum() and max()) to Stream.



And, the real issue is how to deal with checked exceptions inside the Stream
API, i would prefer to fix that issue instead of trying to find a way to
workaround it.


Well I'd like to have a solution for checked exceptions as well, but there 
doesn't appear to be one on the horizon. I mean, there are some ideas floating 
around, but nobody is working on them as far as I know.


But checked exceptions aren't the only reason to prefer iteration in some cases; 
loops offer more flexible control flow (break/continue) and easier handling of 
side effects. The Streams+IterableOnce feature benefits these cases as well as 
exception handling.


s'marks





Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-12 Thread Brian Goetz




Due to how Iterable is specified now it does not promise multiple 
iterations, but due to how most (all that I know of except 
DirectoryStream or some 3rd party) Iterables are implemented now, the 
common expectation is that it does. By implementing more Iterable(s) 
that don't support multiple iteration (regardless of whether they 
implement this new sub-type or not), this expectation will be less and 
less honored. Perhaps this is not so bad. At various occasions the 
specification has been changed to accommodate the long-standing 
behavior or expectation of behavior, but this seems not to be one of 
them.


Correct.  But, with separate Iterable and IterableOnce, the spec for 
Iterable can say "If you don't support multiple iteration, you should 
implement IO."  Which is OK, because at least those classes are being 
explicit.


Let's pretend for a moment that Iterable specification was changed to 
guarantee multi-pass iteration. In that case the IterableAtLeastOnce 
as a supertype of multi-pass Iterable would make much more sense, 
wouldn't it?


Then we couldn't use Stream in foreach without a language change, which 
is the whole point of this exercise.






Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-12 Thread Peter Levart




On 3/12/19 5:04 PM, Brian Goetz wrote:

No. You have the LSP backwards (though this is easy to do.)

IterableOnce means "*must throw* on subsequent use"; under this spec, 
an arbitrary Iterable is most certainly *not* an IterableOnce, and 
therefore an LSP violation.


It sounds like you are suggesting that we instead spec 
IterableAtLeastOnce, of which Iterable *would* be a credible subtype 
-- but due to how Iterable is specified now, the problem is that 
Iterable *already is* really "iterable at least once."  So there would 
be no point in introducing this type; we already have it.


Due to how Iterable is specified now it does not promise multiple 
iterations, but due to how most (all that I know of except 
DirectoryStream or some 3rd party) Iterables are implemented now, the 
common expectation is that it does. By implementing more Iterable(s) 
that don't support multiple iteration (regardless of whether they 
implement this new sub-type or not), this expectation will be less and 
less honored. Perhaps this is not so bad. At various occasions the 
specification has been changed to accommodate the long-standing behavior 
or expectation of behavior, but this seems not to be one of them.


Let's pretend for a moment that Iterable specification was changed to 
guarantee multi-pass iteration. In that case the IterableAtLeastOnce as 
a supertype of multi-pass Iterable would make much more sense, wouldn't it?


But as there could be Iterables out there that by current spec are 
perfectly valid and don't support multi-pass iteration, such spec change 
would make them non-conformant and is therefore not allowed. So I'm 
convinced now that this is the least-bad solution.


Regards, Peter




If we were doing this from scratch, we might choose a different path, 
but that option is not open to us.  As several have already noted, 
this proposal is not ideal, but due to the corner we're painted into 
by Iterable's current spec, there is not going to be an ideal outcome 
-- and the current strategy (do nothing) is also not ideal and makes 
many people unhappy.  So the game here is finding the least-bad 
solution that is compatible with the constraints we have, which I 
think Stuart has done exactly. (Also, he did a very nice job of 
writing up all the alternatives, to avoid (or at least reduce) the 
torrent of "have you thought about extensively>".)


On 3/6/2019 10:50 AM, Peter Levart wrote:


In this respect Iterable should be a subtype of IterableOnce and 
foreach loop should be retrofitted to work with IterableOnce.






Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-12 Thread Brian Goetz

No.  You have the LSP backwards (though this is easy to do.)

IterableOnce means "*must throw* on subsequent use"; under this spec, an 
arbitrary Iterable is most certainly *not* an IterableOnce, and 
therefore an LSP violation.


It sounds like you are suggesting that we instead spec 
IterableAtLeastOnce, of which Iterable *would* be a credible subtype -- 
but due to how Iterable is specified now, the problem is that Iterable 
*already is* really "iterable at least once."  So there would be no 
point in introducing this type; we already have it.


If we were doing this from scratch, we might choose a different path, 
but that option is not open to us.  As several have already noted, this 
proposal is not ideal, but due to the corner we're painted into by 
Iterable's current spec, there is not going to be an ideal outcome -- 
and the current strategy (do nothing) is also not ideal and makes many 
people unhappy.  So the game here is finding the least-bad solution that 
is compatible with the constraints we have, which I think Stuart has 
done exactly. (Also, he did a very nice job of writing up all the 
alternatives, to avoid (or at least reduce) the torrent of "have you 
thought about ".)


On 3/6/2019 10:50 AM, Peter Levart wrote:


In this respect Iterable should be a subtype of IterableOnce and 
foreach loop should be retrofitted to work with IterableOnce.




Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-12 Thread Remi Forax
- Mail original -
> De: "Peter Levart" 
> À: "John Rose" , "Tagir Valeev" 
> Cc: "core-libs-dev" 
> Envoyé: Mardi 12 Mars 2019 11:29:22
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> Hi John,
> 
> On 3/12/19 12:07 AM, John Rose wrote:
>>> public static void main(String[] args) {
>>>   for(int i : range(0, 100)) {
>>> System.out.println(i);
>>>   }
>>> }
>>>
>>> It correctly compiles and prints numbers from 0 to 99. As IntStream
>>> extends BaseStream and BaseStream>> BaseStream> defines Iterator iterator(), it would be no
>>> problem with using IntStream.range in such code pattern were
>>> BaseStream extends IterableOnce.
>>>
>>> Of course this produces unnecessary garbage, as I said.
>> This is a relatively simple kind of garbage to remove, because
>> it is made (by calls to Integer.valueOf) at the adapted boundaries
>> of the iterator, which are readily inlined into the loop.  The deeper
>> internal logic of the range function is box-free, as is the loop itself,
>> so the garbage is relatively easy to remove.
>>
>> That said, "out of the box" there is lots of garbage created unless
>> -XX:+AggressiveUnboxing is turned on.  I assume Graal does a good
>> job on it, even without this switch.
>>
>> If we ever succeed in suppressing the identity of java.lang.Integer,
>> and/or after making functions like range into reified generics, the
>> boxing will go away even more directly and simply.
>>
>> So, from the JIT engineering point of view, I would classify this
>> particular boxing problem as relatively short-lived.
>>
>> — John
> 
> What I have observed (some time ago) is that Integer instances obtained
> by Integer.valueOf() are never found to "not escape" the JIT compilation
> unit and are therefore never scalarized by JIT because of the "feature"
> that was designed to actually prevent the continuous allocation of some
> "values" of Integer(s) - namely the caching of Integer instances in the
> Integer.IntegerCache.cache array for values in range [-128, 127]. So the
> feature designed to prevent continuous allocation is actually working
> against the desired goal. The code of Integer.valueOf is:
> 
>     public static Integer valueOf(int i) {
>     if (i >= IntegerCache.low && i <= IntegerCache.high)
>     return IntegerCache.cache[i + (-IntegerCache.low)];
>     return new Integer(i);
>     }
> 
> ...so JIT would have to create two specializations of code: one for
> cached Integer instances which are always real objects and the other for
> scalarized Integer(s) created by constructor. Last time I experimented
> with this was in JDK 8. Is HotSpot in JDK 12+ smarter now and can do
> this? Perhaps the @HotSpotIntrinsicCandidate annotation on this method
> is a clue that it is treated in a special way by the JIT?

this has been fixed very recently
https://bugs.openjdk.java.net/browse/JDK-8075052

> 
> 
> Regards, Peter

regards,
Rémi


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-12 Thread Peter Levart

Hi John,

On 3/12/19 12:07 AM, John Rose wrote:

public static void main(String[] args) {
  for(int i : range(0, 100)) {
System.out.println(i);
  }
}

It correctly compiles and prints numbers from 0 to 99. As IntStream
extends BaseStream and BaseStream> defines Iterator iterator(), it would be no
problem with using IntStream.range in such code pattern were
BaseStream extends IterableOnce.

Of course this produces unnecessary garbage, as I said.

This is a relatively simple kind of garbage to remove, because
it is made (by calls to Integer.valueOf) at the adapted boundaries
of the iterator, which are readily inlined into the loop.  The deeper
internal logic of the range function is box-free, as is the loop itself,
so the garbage is relatively easy to remove.

That said, "out of the box" there is lots of garbage created unless
-XX:+AggressiveUnboxing is turned on.  I assume Graal does a good
job on it, even without this switch.

If we ever succeed in suppressing the identity of java.lang.Integer,
and/or after making functions like range into reified generics, the
boxing will go away even more directly and simply.

So, from the JIT engineering point of view, I would classify this
particular boxing problem as relatively short-lived.

— John


What I have observed (some time ago) is that Integer instances obtained 
by Integer.valueOf() are never found to "not escape" the JIT compilation 
unit and are therefore never scalarized by JIT because of the "feature" 
that was designed to actually prevent the continuous allocation of some 
"values" of Integer(s) - namely the caching of Integer instances in the 
Integer.IntegerCache.cache array for values in range [-128, 127]. So the 
feature designed to prevent continuous allocation is actually working 
against the desired goal. The code of Integer.valueOf is:


    public static Integer valueOf(int i) {
    if (i >= IntegerCache.low && i <= IntegerCache.high)
    return IntegerCache.cache[i + (-IntegerCache.low)];
    return new Integer(i);
    }

...so JIT would have to create two specializations of code: one for 
cached Integer instances which are always real objects and the other for 
scalarized Integer(s) created by constructor. Last time I experimented 
with this was in JDK 8. Is HotSpot in JDK 12+ smarter now and can do 
this? Perhaps the @HotSpotIntrinsicCandidate annotation on this method 
is a clue that it is treated in a special way by the JIT?



Regards, Peter



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-11 Thread Alan Snyder
Also, if the upper bound is an expression, the range() approach makes it clear 
that the upper bound expression is intended to be evaluated once.

> On Mar 11, 2019, at 4:07 PM, John Rose  > wrote:
> 
> P.S. I also saw the fiery objections to the range() idiom, and I have
> to disagree with those disagreers!  I prefer the range idiom to the
> multi-part for-init-test-step idiom, because counters are semantically
> *more complex* than ranges, from the viewpoints I typically use to
> reason about programs.  (There are times when a separate counter
> state *is* preferable to me, but usually when there is some sort of
> extra "i += skip" side effect in the loop.)  That's got to be a matter of
> taste.  I suppose some people habitually reason about programs
> in lower-level terms, like x86 instructions, and there a counter is no
> more complex than a range; for those people there's no reason to
> learn a different concept than a for-loop.  And surely there are other
> points of view that favor the good old for-loop-with-int-counter.
> 
> I'm more tolerant of new-fangled range-based notations because they
> let me avoid having to reason about side-effect-laden entities
> like counters, and that feels like a good thing to me.  Tastes vary.



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-11 Thread John Rose
On Mar 7, 2019, at 4:33 AM, Tagir Valeev  wrote:
> 
> Hello, Remi!
> 
> It actually works thanks to auto-boxing/unboxing. E.g. this
> implementation works:
> 
> static Iterable range(int from, int to) {
>  return () -> new PrimitiveIterator.OfInt() {
>int cur = from;
> 
>@Override
>public int nextInt() {
>  if (cur >= to) {
>throw new NoSuchElementException();
>  }
>  return cur++;
>}
> 
>@Override
>public boolean hasNext() {
>  return cur < to;
>}
>  };
> }
> 
> public static void main(String[] args) {
>  for(int i : range(0, 100)) {
>System.out.println(i);
>  }
> }
> 
> It correctly compiles and prints numbers from 0 to 99. As IntStream
> extends BaseStream and BaseStream BaseStream> defines Iterator iterator(), it would be no
> problem with using IntStream.range in such code pattern were
> BaseStream extends IterableOnce.
> 
> Of course this produces unnecessary garbage, as I said.

This is a relatively simple kind of garbage to remove, because
it is made (by calls to Integer.valueOf) at the adapted boundaries
of the iterator, which are readily inlined into the loop.  The deeper
internal logic of the range function is box-free, as is the loop itself,
so the garbage is relatively easy to remove.

That said, "out of the box" there is lots of garbage created unless
-XX:+AggressiveUnboxing is turned on.  I assume Graal does a good
job on it, even without this switch.

If we ever succeed in suppressing the identity of java.lang.Integer,
and/or after making functions like range into reified generics, the
boxing will go away even more directly and simply.

So, from the JIT engineering point of view, I would classify this
particular boxing problem as relatively short-lived.

— John

P.S. I also saw the fiery objections to the range() idiom, and I have
to disagree with those disagreers!  I prefer the range idiom to the
multi-part for-init-test-step idiom, because counters are semantically
*more complex* than ranges, from the viewpoints I typically use to
reason about programs.  (There are times when a separate counter
state *is* preferable to me, but usually when there is some sort of
extra "i += skip" side effect in the loop.)  That's got to be a matter of
taste.  I suppose some people habitually reason about programs
in lower-level terms, like x86 instructions, and there a counter is no
more complex than a range; for those people there's no reason to
learn a different concept than a for-loop.  And surely there are other
points of view that favor the good old for-loop-with-int-counter.

I'm more tolerant of new-fangled range-based notations because they
let me avoid having to reason about side-effect-laden entities
like counters, and that feels like a good thing to me.  Tastes vary.

> 
> With best regards,
> Tagir Valeev.
> 
> On Wed, Mar 6, 2019 at 7:37 PM Remi Forax  wrote:
>> 
>> Hi Tagir,
>> try to do it now and you will see that you can't, because you can not write 
>> Iterable yet.
>> Once we will get generics over value types, it will be a no-brainer.
>> 
>> Rémi
>> 
>> - Mail original -
>>> De: "Tagir Valeev" 
>>> À: "Stuart Marks" 
>>> Cc: "core-libs-dev" 
>>> Envoyé: Mercredi 6 Mars 2019 11:10:41
>>> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams
>> 
>>> Hello!
>>> 
>>> By the way one of the painful code patterns in modern Java is `for(int
>>> i = 0; i>> newbies and prone to errors as the variable need to be repeated three
>>> times. Also the variable is not effectively final, despite it never
>>> changes within the loop body, so could have been considered as
>>> redeclared on every loop iteration (like in for-each). With the new
>>> proposal it's possible to write `for(int i : range(0, bound).boxed())`
>>> (assuming import static j.u.s.IntStream.range), which looks much
>>> better, though it has obvious performance drawback. Moving
>>> IterableOnce to BaseStream would enable to use `for(int i : range(0,
>>> bound))` which looks even better, though again we have plenty of
>>> garbage (but considerably less than in previous case!). I wonder
>>> whether Java could evolve to the point where such kind of code would
>>> be a recommended way to iterate over subsequent integer values without
>>> any performance handicap.
>>> 
>>> With best regards,
>>> Tagir Valeev.
>>> 
>>> On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks  wrote:
>>>> 
>>>> Hi all,
>>>> 
>>>> Please review and comment on

Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-11 Thread David Holmes

On 7/03/2019 12:48 am, Scott Palmer wrote:

I don’t mean any offence, but I have to say, I strongly disagree with nearly 
everything you’ve written below. To me, the idea of making a stream of integers 
for a simple loop counter is hackish, confusing, verbose, and basically abusing 
the stream concept.  The only part I agree with is that it is an obvious 
performance drawback as well.  A counter and a stream of integers are 
completely different concepts and should not be confused in this manner.


I totally agree! The stream variant is just obfuscation at its worst - 
cleverness for cleverness sake.


My 2c.

David


Scott


On Mar 6, 2019, at 5:10 AM, Tagir Valeev  wrote:

Hello!

By the way one of the painful code patterns in modern Java is `for(int
i = 0; i wrote:


Hi all,

Please review and comment on this proposal to allow Stream instances to be used
in enhanced-for ("for-each") loops.

Abstract

Occasionally it's useful to iterate a Stream using a conventional loop. However,
the Stream interface doesn't implement Iterable, and therefore streams cannot be
used with the enhanced-for statement. This is a proposal to remedy that
situation by introducing a new interface IterableOnce that is a subtype of
Iterable, and then retrofitting the Stream interface to implement it. Other JDK
classes will also be retrofitted to implement IterableOnce.

Full Proposal:

 http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html

Bug report:

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

Webrev:

 http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/

Note, this changeset isn't ready to push yet. In particular, it has no tests
yet. However, the implementation is so simple that I figured I should include
it. Comments on the specification wording are also welcome.

Thanks,

s'marks




Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-07 Thread Tagir Valeev
Hello, Remi!

It actually works thanks to auto-boxing/unboxing. E.g. this
implementation works:

static Iterable range(int from, int to) {
  return () -> new PrimitiveIterator.OfInt() {
int cur = from;

@Override
public int nextInt() {
  if (cur >= to) {
throw new NoSuchElementException();
  }
  return cur++;
}

@Override
public boolean hasNext() {
  return cur < to;
}
  };
}

public static void main(String[] args) {
  for(int i : range(0, 100)) {
System.out.println(i);
  }
}

It correctly compiles and prints numbers from 0 to 99. As IntStream
extends BaseStream and BaseStream> defines Iterator iterator(), it would be no
problem with using IntStream.range in such code pattern were
BaseStream extends IterableOnce.

Of course this produces unnecessary garbage, as I said.

With best regards,
Tagir Valeev.

On Wed, Mar 6, 2019 at 7:37 PM Remi Forax  wrote:
>
> Hi Tagir,
> try to do it now and you will see that you can't, because you can not write 
> Iterable yet.
> Once we will get generics over value types, it will be a no-brainer.
>
> Rémi
>
> - Mail original -
> > De: "Tagir Valeev" 
> > À: "Stuart Marks" 
> > Cc: "core-libs-dev" 
> > Envoyé: Mercredi 6 Mars 2019 11:10:41
> > Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams
>
> > Hello!
> >
> > By the way one of the painful code patterns in modern Java is `for(int
> > i = 0; i > newbies and prone to errors as the variable need to be repeated three
> > times. Also the variable is not effectively final, despite it never
> > changes within the loop body, so could have been considered as
> > redeclared on every loop iteration (like in for-each). With the new
> > proposal it's possible to write `for(int i : range(0, bound).boxed())`
> > (assuming import static j.u.s.IntStream.range), which looks much
> > better, though it has obvious performance drawback. Moving
> > IterableOnce to BaseStream would enable to use `for(int i : range(0,
> > bound))` which looks even better, though again we have plenty of
> > garbage (but considerably less than in previous case!). I wonder
> > whether Java could evolve to the point where such kind of code would
> > be a recommended way to iterate over subsequent integer values without
> > any performance handicap.
> >
> > With best regards,
> > Tagir Valeev.
> >
> > On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks  wrote:
> >>
> >> Hi all,
> >>
> >> Please review and comment on this proposal to allow Stream instances to be 
> >> used
> >> in enhanced-for ("for-each") loops.
> >>
> >> Abstract
> >>
> >> Occasionally it's useful to iterate a Stream using a conventional loop. 
> >> However,
> >> the Stream interface doesn't implement Iterable, and therefore streams 
> >> cannot be
> >> used with the enhanced-for statement. This is a proposal to remedy that
> >> situation by introducing a new interface IterableOnce that is a subtype of
> >> Iterable, and then retrofitting the Stream interface to implement it. 
> >> Other JDK
> >> classes will also be retrofitted to implement IterableOnce.
> >>
> >> Full Proposal:
> >>
> >>  http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
> >>
> >> Bug report:
> >>
> >>  https://bugs.openjdk.java.net/browse/JDK-8148917
> >>
> >> Webrev:
> >>
> >>  http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
> >>
> >> Note, this changeset isn't ready to push yet. In particular, it has no 
> >> tests
> >> yet. However, the implementation is so simple that I figured I should 
> >> include
> >> it. Comments on the specification wording are also welcome.
> >>
> >> Thanks,
> >>
> > > s'marks


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-07 Thread Peter Levart




On 3/7/19 9:41 AM, Peter Levart wrote:
There is a benefit in the runtime though. The code can decide what to 
do with the passed-in Iterable depending on it implementing 
IterableOnce or not. Much like what RandomAccess interface does to 
List(s). The code can decide to dump the iterable into a List and 
iterate the List multiple times if the Iterable implements 
IterableOnce or do direct multiple iteration on the passed-in Iterable 
if it doesn't. 


Expanding on this further, there could be help for multi-pass iteration 
in the Iterable(Once) interfaces. For example:


public interface Iterable {

    Iterator iterator();

    default Iterable toMultipassIterable() {
    return this;
    }
}


and then:

public interface IterableOnce extends Iterable {

    @Override
    default Iterable toMultipassIterable() {
    List list = new ArrayList<>();
    for (T t : this) {
    list.add(t);
    }
    return list;
    }
}


... so any new code that needs multiple passes can do so easily.


Regards, Peter



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-07 Thread Peter Levart

Hi,

I see this is already discussed in the "Alternatives" section of the 
proposal (sorry for not reading this through before asking)...


But I don't quite understand the following part that talks about making 
IterableOnce a supertype of Iterable:


"However, doing so would require weakening the contract of IterableOnce. 
To allow Iterable to be a subtype, the semantics of IterableOnce would 
need to change to allow iteration once and possibly more, instead of the 
currently proposed at-most-once semantics. Having IterableOnce be a 
subtype allows the specification to make a much stronger assertion."


Why would that require "weakening the contract of IterableOnce" ? 
Because if IterableOnce was specified to throw exception on the 2nd and 
subsequent invocations, some instances (those implementing Iterable) 
would not respect that?


I see this as less evil than doing the other way around. How many 
situations will rely on IterableOnce to throw exception vs. how many 
situations will rely on Iterable to allow multiple invocations?


But I agree with assessment that changing language to retrofit foreach 
loop to work with a supertype of Iterable (although it seems a backwards 
compatible change) would be too costly for this feature.


And there are already other similar decisions made in API design of 
Java. For example Collections framework with immutable vs. mutable 
collections. This distinction is not even encoded in type(s). But this 
API was designed from the beginning with that in mind and therefore most 
code consuming collection types documents how it uses the passed-in 
collections (whether it only reads from them or also modifies them). 
Consumers of Iterable(s) did not have that requirement from the 
beginning. If they had, we would not need a special type to mark the 
distinction. The specification of Iterable could simply state that: 
"There are two kinds of Iterable(s) in this world: those that may be 
iterated only once and those that can be iterated multiple times"...


OTOH, having a separate type for only-once iterables would only help 
identify instances of them, but would not help in documenting the 
consumers. Consumers would still have to document this via javadoc. I 
would not recommend consumers to take parameters of type IterableOnce if 
IterableOnce was a subtype of Iterable, because they would unnecessarily 
restrict themselves to consume just the less capable instances.


In addition, introduction of IterableOnce as a subtype of Iterable does 
not help in (re)specifying the Iterable type. There will be two kinds of 
Iterable(s) in the new world regardless of that.


There is a benefit in the runtime though. The code can decide what to do 
with the passed-in Iterable depending on it implementing IterableOnce or 
not. Much like what RandomAccess interface does to List(s). The code can 
decide to dump the iterable into a List and iterate the List multiple 
times if the Iterable implements IterableOnce or do direct multiple 
iteration on the passed-in Iterable if it doesnt.


Regards, Peter

On 3/6/19 4:50 PM, Peter Levart wrote:

Hi Stuart,

According to Liskov substitution principle:

    Subtype Requirement: Let ϕ ( x ) be a property provable about 
objects x of type T. Then ϕ ( y ) should be true for objects y of type 
S where S is a subtype of T.



Let ϕ ( x ) for objects x of type Iterable be: "x.iterator() may be 
invoked multiple times, each time starting new iteration".


This clearly holds.

Does ϕ ( y ) hold for objects y of type IterableOnce? Clearly not.

In this respect Iterable should be a subtype of IterableOnce and 
foreach loop should be retrofitted to work with IterableOnce.


What do you think?

Regards, Peter

On 3/1/19 3:43 AM, Stuart Marks wrote:

Hi all,

Please review and comment on this proposal to allow Stream instances 
to be used in enhanced-for ("for-each") loops.


Abstract

Occasionally it's useful to iterate a Stream using a conventional 
loop. However, the Stream interface doesn't implement Iterable, and 
therefore streams cannot be used with the enhanced-for statement. 
This is a proposal to remedy that situation by introducing a new 
interface IterableOnce that is a subtype of Iterable, and then 
retrofitting the Stream interface to implement it. Other JDK classes 
will also be retrofitted to implement IterableOnce.


Full Proposal:

http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html

Bug report:

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

Webrev:

    http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/

Note, this changeset isn't ready to push yet. In particular, it has 
no tests yet. However, the implementation is so simple that I figured 
I should include it. Comments on the specification wording are also 
welcome.


Thanks,

s'marks






Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-06 Thread Peter Levart

Hi Stuart,

According to Liskov substitution principle:

    Subtype Requirement: Let ϕ ( x ) be a property provable about 
objects x of type T. Then ϕ ( y ) should be true for objects y of type S 
where S is a subtype of T.



Let ϕ ( x ) for objects x of type Iterable be: "x.iterator() may be 
invoked multiple times, each time starting new iteration".


This clearly holds.

Does ϕ ( y ) hold for objects y of type IterableOnce? Clearly not.

In this respect Iterable should be a subtype of IterableOnce and foreach 
loop should be retrofitted to work with IterableOnce.


What do you think?

Regards, Peter

On 3/1/19 3:43 AM, Stuart Marks wrote:

Hi all,

Please review and comment on this proposal to allow Stream instances 
to be used in enhanced-for ("for-each") loops.


Abstract

Occasionally it's useful to iterate a Stream using a conventional 
loop. However, the Stream interface doesn't implement Iterable, and 
therefore streams cannot be used with the enhanced-for statement. This 
is a proposal to remedy that situation by introducing a new interface 
IterableOnce that is a subtype of Iterable, and then retrofitting the 
Stream interface to implement it. Other JDK classes will also be 
retrofitted to implement IterableOnce.


Full Proposal:

http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html

Bug report:

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

Webrev:

    http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/

Note, this changeset isn't ready to push yet. In particular, it has no 
tests yet. However, the implementation is so simple that I figured I 
should include it. Comments on the specification wording are also 
welcome.


Thanks,

s'marks




Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-06 Thread Scott Palmer
I don’t mean any offence, but I have to say, I strongly disagree with nearly 
everything you’ve written below. To me, the idea of making a stream of integers 
for a simple loop counter is hackish, confusing, verbose, and basically abusing 
the stream concept.  The only part I agree with is that it is an obvious 
performance drawback as well.  A counter and a stream of integers are 
completely different concepts and should not be confused in this manner.

Scott

> On Mar 6, 2019, at 5:10 AM, Tagir Valeev  wrote:
> 
> Hello!
> 
> By the way one of the painful code patterns in modern Java is `for(int
> i = 0; i newbies and prone to errors as the variable need to be repeated three
> times. Also the variable is not effectively final, despite it never
> changes within the loop body, so could have been considered as
> redeclared on every loop iteration (like in for-each). With the new
> proposal it's possible to write `for(int i : range(0, bound).boxed())`
> (assuming import static j.u.s.IntStream.range), which looks much
> better, though it has obvious performance drawback. Moving
> IterableOnce to BaseStream would enable to use `for(int i : range(0,
> bound))` which looks even better, though again we have plenty of
> garbage (but considerably less than in previous case!). I wonder
> whether Java could evolve to the point where such kind of code would
> be a recommended way to iterate over subsequent integer values without
> any performance handicap.
> 
> With best regards,
> Tagir Valeev.
> 
> On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks  wrote:
>> 
>> Hi all,
>> 
>> Please review and comment on this proposal to allow Stream instances to be 
>> used
>> in enhanced-for ("for-each") loops.
>> 
>> Abstract
>> 
>> Occasionally it's useful to iterate a Stream using a conventional loop. 
>> However,
>> the Stream interface doesn't implement Iterable, and therefore streams 
>> cannot be
>> used with the enhanced-for statement. This is a proposal to remedy that
>> situation by introducing a new interface IterableOnce that is a subtype of
>> Iterable, and then retrofitting the Stream interface to implement it. Other 
>> JDK
>> classes will also be retrofitted to implement IterableOnce.
>> 
>> Full Proposal:
>> 
>> http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>> 
>> Bug report:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8148917
>> 
>> Webrev:
>> 
>> http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>> 
>> Note, this changeset isn't ready to push yet. In particular, it has no tests
>> yet. However, the implementation is so simple that I figured I should include
>> it. Comments on the specification wording are also welcome.
>> 
>> Thanks,
>> 
>> s'marks



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-06 Thread Remi Forax
Hi Tagir,
try to do it now and you will see that you can't, because you can not write 
Iterable yet.
Once we will get generics over value types, it will be a no-brainer.

Rémi

- Mail original -
> De: "Tagir Valeev" 
> À: "Stuart Marks" 
> Cc: "core-libs-dev" 
> Envoyé: Mercredi 6 Mars 2019 11:10:41
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> Hello!
> 
> By the way one of the painful code patterns in modern Java is `for(int
> i = 0; i newbies and prone to errors as the variable need to be repeated three
> times. Also the variable is not effectively final, despite it never
> changes within the loop body, so could have been considered as
> redeclared on every loop iteration (like in for-each). With the new
> proposal it's possible to write `for(int i : range(0, bound).boxed())`
> (assuming import static j.u.s.IntStream.range), which looks much
> better, though it has obvious performance drawback. Moving
> IterableOnce to BaseStream would enable to use `for(int i : range(0,
> bound))` which looks even better, though again we have plenty of
> garbage (but considerably less than in previous case!). I wonder
> whether Java could evolve to the point where such kind of code would
> be a recommended way to iterate over subsequent integer values without
> any performance handicap.
> 
> With best regards,
> Tagir Valeev.
> 
> On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks  wrote:
>>
>> Hi all,
>>
>> Please review and comment on this proposal to allow Stream instances to be 
>> used
>> in enhanced-for ("for-each") loops.
>>
>> Abstract
>>
>> Occasionally it's useful to iterate a Stream using a conventional loop. 
>> However,
>> the Stream interface doesn't implement Iterable, and therefore streams 
>> cannot be
>> used with the enhanced-for statement. This is a proposal to remedy that
>> situation by introducing a new interface IterableOnce that is a subtype of
>> Iterable, and then retrofitting the Stream interface to implement it. Other 
>> JDK
>> classes will also be retrofitted to implement IterableOnce.
>>
>> Full Proposal:
>>
>>  http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>>
>> Bug report:
>>
>>  https://bugs.openjdk.java.net/browse/JDK-8148917
>>
>> Webrev:
>>
>>  http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>>
>> Note, this changeset isn't ready to push yet. In particular, it has no tests
>> yet. However, the implementation is so simple that I figured I should include
>> it. Comments on the specification wording are also welcome.
>>
>> Thanks,
>>
> > s'marks


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-06 Thread Tagir Valeev
Hello!

By the way one of the painful code patterns in modern Java is `for(int
i = 0; i wrote:
>
> Hi all,
>
> Please review and comment on this proposal to allow Stream instances to be 
> used
> in enhanced-for ("for-each") loops.
>
> Abstract
>
> Occasionally it's useful to iterate a Stream using a conventional loop. 
> However,
> the Stream interface doesn't implement Iterable, and therefore streams cannot 
> be
> used with the enhanced-for statement. This is a proposal to remedy that
> situation by introducing a new interface IterableOnce that is a subtype of
> Iterable, and then retrofitting the Stream interface to implement it. Other 
> JDK
> classes will also be retrofitted to implement IterableOnce.
>
> Full Proposal:
>
>  http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>
> Bug report:
>
>  https://bugs.openjdk.java.net/browse/JDK-8148917
>
> Webrev:
>
>  http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>
> Note, this changeset isn't ready to push yet. In particular, it has no tests
> yet. However, the implementation is so simple that I figured I should include
> it. Comments on the specification wording are also welcome.
>
> Thanks,
>
> s'marks


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-05 Thread Mike Duigou
I don't believe that I would use the proposed enhancement to the for 
statement. For me there is cognitive load reduction in using a 
symmetrical method for all iterations even if they end up being a little 
more complicated for individual cases. Usually, for me, I use streams. 
Even the more complicated patterns such as the presented example will 
hopefully be familiar because they are repeated in many places 
throughout the code. Truly unusual or unique usages are hopefully very 
rare.


To choose between old style for, enhanced for, or streams based on which 
warts are to be avoided is just frustrating. Mixing idioms or required 
mixing of idioms produces the least satisfying result. Was the use of a 
particular idiom a necessity? A choice? Of no consequence? This gets 
harder to decode with a larger number of available idioms.



I've seen library functions (usually statically imported) used for the 
"(Iterble) stream::iterator" construction that presented it as 
"iterable(stream)". Not something I would use but it seemed clean enough 
for those who wanted it.


Cheers,

Mike


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-04 Thread Stephen Colebourne
On Fri, 1 Mar 2019 at 02:46, Stuart Marks  wrote:
> Please review and comment on this proposal to allow Stream instances to be 
> used
> in enhanced-for ("for-each") loops.

This all seems reasonable enough though of course not ideal.

My slight concern is that the terminal operation is hidden and not
immediately visible, which could be surprising. I do note that streams
throw a clear exception if a terminal operation is called a second
time which mitigates this to some degree.

The IterableOnce class-level Javadoc contradicts the method-level
Javadoc. The class-level say "It is recommended that" whereas the
method-level Javadoc mandates.

Stephen



> Abstract
>
> Occasionally it's useful to iterate a Stream using a conventional loop. 
> However,
> the Stream interface doesn't implement Iterable, and therefore streams cannot 
> be
> used with the enhanced-for statement. This is a proposal to remedy that
> situation by introducing a new interface IterableOnce that is a subtype of
> Iterable, and then retrofitting the Stream interface to implement it. Other 
> JDK
> classes will also be retrofitted to implement IterableOnce.
>
> Full Proposal:
>
>  http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>
> Bug report:
>
>  https://bugs.openjdk.java.net/browse/JDK-8148917
>
> Webrev:
>
>  http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>
> Note, this changeset isn't ready to push yet. In particular, it has no tests
> yet. However, the implementation is so simple that I figured I should include
> it. Comments on the specification wording are also welcome.
>
> Thanks,
>
> s'marks


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-02 Thread Remi Forax
Hi Stuart,
Stream.iterator() can be really really slow, it uses a pull semantics while the 
whole Stream push values. When designing it, the lambda EG saw it as an "escape 
hatch" in order to interropt with a legacy code than require an Iterator and 
not more. 

This proposal has the side effect of making Stream more different from its 
primitive counterpart IntStream, LongStream and DoubleStream which may be 
problematic because we are trying to introduce reified generics as part of 
Valhalla (there is a recent mail of Brian about not adding methods to 
OptionalInt for the same reason).

And, the real issue is how to deal with checked exceptions inside the Stream 
API, i would prefer to fix that issue instead of trying to find a way to 
workaround it.

regards,
Rémi


- Mail original -
> De: "Stuart Marks" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 1 Mars 2019 03:43:44
> Objet: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> Hi all,
> 
> Please review and comment on this proposal to allow Stream instances to be 
> used
> in enhanced-for ("for-each") loops.
> 
> Abstract
> 
> Occasionally it's useful to iterate a Stream using a conventional loop. 
> However,
> the Stream interface doesn't implement Iterable, and therefore streams cannot 
> be
> used with the enhanced-for statement. This is a proposal to remedy that
> situation by introducing a new interface IterableOnce that is a subtype of
> Iterable, and then retrofitting the Stream interface to implement it. Other 
> JDK
> classes will also be retrofitted to implement IterableOnce.
> 
> Full Proposal:
> 
> http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
> 
> Bug report:
> 
> https://bugs.openjdk.java.net/browse/JDK-8148917
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
> 
> Note, this changeset isn't ready to push yet. In particular, it has no tests
> yet. However, the implementation is so simple that I figured I should include
> it. Comments on the specification wording are also welcome.
> 
> Thanks,
> 
> s'marks


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-01 Thread Stuart Marks

The proposal looks good to me. In particular it's very nice to have
this properly (ability to iterate only once) to be encoded in the type
system. This could be helpful for static analysis tools to warn when
IterableOnce is reused.


Thanks! It would be good to see what you can come up with respect to static 
analysis. I had speculated about this some, but you're much closer to this than 
I am.



Scanner case looks funny. In general such pattern could be applied to
every Iterator implementor.


Yes, Scanner is rather an outlier with respect to Iterator. There are of course 
a lot of Iterator implementations, but (at least in the JDK) most of them are 
internal classes that can only be instantiated by calling a method -- typically 
but not always the iterator() method. There are only a few cases where an API 
*class* implements Iterator and where it's likely that calling code would get an 
instance of Iterator separately from iterating over it. Scanner was the only one 
I found in the JDK where it seemed reasonable to want to iterate it using an 
enhanced-for loop.



As for Streams, I worry a little that people would prefer for(T t :
stream) {...} over stream.forEach(t -> ...) even when forEach works
perfectly.


Possibly. Of all the stream operations, forEach() is the one that's the easiest 
to learn but among the least useful. There's a certain kind of error where 
someone new to streams will try to put too much work into a forEach() operation 
and be frustrated because they can't modify local variables, break early, etc. 
The usual remedy is to recast such tasks to use reduction instead. I guess the 
risk is that the person would instead switch to a for-loop and then conclude, 
Why bother with this streams stuff?


I think at this point there are enough streams tutorials and Stack Overflow Q 
on this topic that this won't be a very big problem.



The Stream.iterator() implementation produces some amount
of unnecessary garbage. At least it would be nice to add special case
into Spliterators#iterator(java.util.Spliterator) to
reduce number of wrappers if stream was directly created from an
iterator or collection: ...
Of course this could be done later as separate enhancement.


Yes, I think this would be good to investigate and optimize if it proves to be a 
problem.


On 2/28/19 9:54 PM, Tagir Valeev wrote:

One more alternative which could be considered (though it requires a
language change) is to allow the inference of Iterable type for
enhanced for when iteration value is a function expression and
iteration parameter type is explicitly specified as T. In this case
for(T t : stream::iterator) {} would be a valid syntax. Also people
could use enhanced for with any iterator they already have like for(T
t : () -> iterator) {}. Instead of for (; it.hasNext() ; ) { T t =
it.next(); ... }. It looks better than allowing to specify iterator
directly (like for(T t : stream.iterator())) as this could break some
programs where the same class implements Iterator and Iterable.


This was noticed and discussed fairly early on (way back in 2012!) and it turns 
out to be more complicated than it appears at first glance. See these threads 
from lambda-dev:


http://mail.openjdk.java.net/pipermail/lambda-dev/2012-August/005717.html

http://mail.openjdk.java.net/pipermail/lambda-dev/2012-September/005749.html

See in particular the messages from Maurizio. There is also reference to the 
original Java 5 design discussions that ruled out Iterator in the RHS of the 
enhanced-for loop. TL;DR the reason is that they didn't want the RHS expression 
to have a side effect of draining the Iterator.


s'marks



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-02-28 Thread Tagir Valeev
One more alternative which could be considered (though it requires a
language change) is to allow the inference of Iterable type for
enhanced for when iteration value is a function expression and
iteration parameter type is explicitly specified as T. In this case
for(T t : stream::iterator) {} would be a valid syntax. Also people
could use enhanced for with any iterator they already have like for(T
t : () -> iterator) {}. Instead of for (; it.hasNext() ; ) { T t =
it.next(); ... }. It looks better than allowing to specify iterator
directly (like for(T t : stream.iterator())) as this could break some
programs where the same class implements Iterator and Iterable.

With best regards,
Tagir Valeev.

On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks  wrote:
>
> Hi all,
>
> Please review and comment on this proposal to allow Stream instances to be 
> used
> in enhanced-for ("for-each") loops.
>
> Abstract
>
> Occasionally it's useful to iterate a Stream using a conventional loop. 
> However,
> the Stream interface doesn't implement Iterable, and therefore streams cannot 
> be
> used with the enhanced-for statement. This is a proposal to remedy that
> situation by introducing a new interface IterableOnce that is a subtype of
> Iterable, and then retrofitting the Stream interface to implement it. Other 
> JDK
> classes will also be retrofitted to implement IterableOnce.
>
> Full Proposal:
>
>  http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>
> Bug report:
>
>  https://bugs.openjdk.java.net/browse/JDK-8148917
>
> Webrev:
>
>  http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>
> Note, this changeset isn't ready to push yet. In particular, it has no tests
> yet. However, the implementation is so simple that I figured I should include
> it. Comments on the specification wording are also welcome.
>
> Thanks,
>
> s'marks


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-02-28 Thread Tagir Valeev
Hello!

The proposal looks good to me. In particular it's very nice to have
this properly (ability to iterate only once) to be encoded in the type
system. This could be helpful for static analysis tools to warn when
IterableOnce is reused.

Scanner case looks funny. In general such pattern could be applied to
every Iterator implementor.

As for Streams, I worry a little that people would prefer for(T t :
stream) {...} over stream.forEach(t -> ...) even when forEach works
perfectly. The Stream.iterator() implementation produces some amount
of unnecessary garbage. At least it would be nice to add special case
into Spliterators#iterator(java.util.Spliterator) to
reduce number of wrappers if stream was directly created from an
iterator or collection:

--- src/java.base/share/classes/java/util/Spliterators.java (revision
53626:e2fc434b410a35b28ab433c29863c8a26e4e813a)
+++ src/java.base/share/classes/java/util/Spliterators.java (date 1551417931182)
@@ -665,6 +665,24 @@
  */
 public static Iterator iterator(Spliterator
spliterator) {
 Objects.requireNonNull(spliterator);
+if (spliterator instanceof IteratorSpliterator) {
+@SuppressWarnings("unchecked")
+IteratorSpliterator wrapper =
(IteratorSpliterator) spliterator;
+wrapper.estimateSize(); // force initialization
+Iterator it = Objects.requireNonNull(wrapper.it);
+// Avoid exposing the remove() method of original
iterator; too bad there's no way to avoid this if remove() is not
implemented :(
+return new Iterator<>() {
+@Override
+public boolean hasNext() {
+return it.hasNext();
+}
+
+@Override
+public T next() {
+return it.next();
+}
+};
+}
 class Adapter implements Iterator, Consumer {
 boolean valueReady = false;
 T nextElement;

Of course this could be done later as separate enhancement.

On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks  wrote:
>
> Hi all,
>
> Please review and comment on this proposal to allow Stream instances to be 
> used
> in enhanced-for ("for-each") loops.
>
> Abstract
>
> Occasionally it's useful to iterate a Stream using a conventional loop. 
> However,
> the Stream interface doesn't implement Iterable, and therefore streams cannot 
> be
> used with the enhanced-for statement. This is a proposal to remedy that
> situation by introducing a new interface IterableOnce that is a subtype of
> Iterable, and then retrofitting the Stream interface to implement it. Other 
> JDK
> classes will also be retrofitted to implement IterableOnce.
>
> Full Proposal:
>
>  http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>
> Bug report:
>
>  https://bugs.openjdk.java.net/browse/JDK-8148917
>
> Webrev:
>
>  http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>
> Note, this changeset isn't ready to push yet. In particular, it has no tests
> yet. However, the implementation is so simple that I figured I should include
> it. Comments on the specification wording are also welcome.
>
> Thanks,
>
> s'marks


Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-02-28 Thread Stuart Marks

Hi all,

Please review and comment on this proposal to allow Stream instances to be used 
in enhanced-for ("for-each") loops.


Abstract

Occasionally it's useful to iterate a Stream using a conventional loop. However, 
the Stream interface doesn't implement Iterable, and therefore streams cannot be 
used with the enhanced-for statement. This is a proposal to remedy that 
situation by introducing a new interface IterableOnce that is a subtype of 
Iterable, and then retrofitting the Stream interface to implement it. Other JDK 
classes will also be retrofitted to implement IterableOnce.


Full Proposal:

http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html

Bug report:

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

Webrev:

http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/

Note, this changeset isn't ready to push yet. In particular, it has no tests 
yet. However, the implementation is so simple that I figured I should include 
it. Comments on the specification wording are also welcome.


Thanks,

s'marks