Re: RFR: 8279986: methods Math::asXExact for safely checked primitive casts

[Brian Goetz]

Another way to evaluate answers here is: are we inventing new relations, 
or merely clarifying old ones?  The latter is often more desirable.


For example, we might say that x:X can be converted exactly to Y, for 
primitive X and Y, iff:


    eq( (X) (Y) x, x )

where `eq` is `==` for non-floating primitive types, and derived from 
Float::compare and Double::compare for floating point. This means we are 
not inventing any new conversions or comparisons, but merely combining 
ones we already have in the language/platform in a convenient way.


Do the toXExact methods you've defined have this characteristic?

(Though, while this is often the best starting place, it is not always a 
slam-dunk answer; sometimes there are good reasons to depart from 
existing relations, but we should be explicit about what those are.)




On 5/25/2022 9:46 AM, Raffaello Giulietti wrote:


Themotivation behind this PR isnot driven by pattern matching:John 
Rose (the reporter of the JBS issue) is concerned about having safer 
casts that, in addition, can be JITted to efficient code.


But I get your point that having non-throwing testsbetter serves 
pattern matching.However, I’m not sure that pattern matching alone 
would remove the more general need for the proposed methods.


As for the controversial question about -0.0, as you note any proposal 
will kind of suck. With “safer” cast methods we can have two (or even 
more) variants.


In the context of primitive pattern matching (including the relevant 
material in the JLS), however, it would be probably much simpler to 
allow for matches between integral types on one side and for matches 
between floating-point types on the other side, but not a mix. The 
nuisance with -0.0 and other special values would disappear altogether.


Thus, while examples like

    if (anIntExpression instanceof byte b)

and

    if (aDoubleExpression instanceof float f)

make perfect sense, would an example like
    if (aDoubleExpression instanceof short s)

be pragmatically reasonable?

IIRC, the discussions about “Primitive type patterns” and “Evolving 
past reference type patterns” in the amber-spec-experts mailing list 
of April 2022 don’t even mention the mixed integral/floating-point case.


Greetings
Raffaello

*From: *core-libs-dev  on behalf 
of Brian Goetz 

*Date: *Tuesday, 24 May 2022 at 00:09
*To: *Raffaello Giulietti , 
core-libs-dev@openjdk.java.net 
*Subject: *Re: RFR: 8279986: methods Math::asXExact for safely checked 
primitive casts


This work is quite timely as we are now paving the way for primitive
type patterns over in Project Amber, and also has a nontrivial
connection with Valhalla.  If you'll pardon a brief digression...

Instanceof and casting work together in a familiar way: before you cast,
you first ask instanceof -- but this is restricted currently to
reference types.  But the pattern is obvious: instanceof is the
precondition check for casting, which asks: "If I made this cast, would
I like the answer."  There are currently two reasons to not like the
answer: a CCE, or the operand is null (because, even though the cast
would succeed, if you tried to use it as a member of that type, you
wouldn't like the answer then.)

If we wanted to extend `instanceof` to primitive types, we are asking
the same question: if I were to cast to this type, would I like the
answer.  And casts involving primitives give us two more reasons to not
like the answer: an NPE (due to unboxing), or loss of precision.  Which
means that we have to specify in JLS 5.1 what cast with loss of
precision means.  At the very least, we will want to align this work
with that; the asXExact should be able to say "throws if the cast would
lose precision", where "lose precision" is precisely defined by the JLS.

Separately, Project Valhalla will let us define new primitive-like
types, such as HalfFloat and SuperLong. Conversions between primitives
are currently specified in a complex table in JLS 5.1. But surely we
will want to support primitive widening conversions between HalfFloat
and float (somehow; how we do this is a separate discussion.)  Which
brings us back to pattern matching; narrowing casts are inherently
partial, and declared patterns bring partiality into the "return type",
and are the natural way (when we have it) to express things like "cast,
but fail if you can't do so safely". This is preferable to throwing
(which currently is the our choice.)  So it might be a little
unfortunate to introduce throwing toXExactly now and then have to
introduce separate patterns which signal precision loss by match
failure.  (Though that's not the end of the world if there is some
duplication.)

What this says is that the current proposal of toXExact is not the
primitive, because we probably wouldn't want to implement a pattern in
terms of the throwing version.

Converting from float/double to integral types is particularly tricky
with -0.0.  Both an

Re: RFR: 8279986: methods Math::asXExact for safely checked primitive casts

[Brian Goetz]

Themotivation behind this PR isnot driven by pattern matching:John 
Rose (the reporter of the JBS issue) is concerned about having safer 
casts that, in addition, can be JITted to efficient code.




Yes, of course.  But also, as the platform evolves, it often happens 
that the same issues arises in separate places.  It would be terrible if 
asXExact had a different opinion of what could be converted exactly to 
an int, from the language's `instanceof int`. And similarly, it would be 
unfortunate if the semantics were driven by nothing more than "who got 
there first."  So we frequently discover things that we thought were 
independent increments of functionality, that turn out to want to be 
co-designed with other things, even those not known at the time we 
started.  This is just how this game works.


But I get your point that having non-throwing testsbetter serves 
pattern matching.However, I’m not sure that pattern matching alone 
would remove the more general need for the proposed methods.




Yes, not sure either.  The game here is "find the primitive"; our first 
move in this game is not always the right one.  So let's figure it out.


As for the controversial question about -0.0, as you note any proposal 
will kind of suck. With “safer” cast methods we can have two (or even 
more) variants.




Also, small changes in terminology can subtly bias our answer. Words 
like "exact", "information-preserving", "loss of precision", "safe", 
etc, all seem to circle the same concept, but the exact choice of 
terminology -- often made at random at the beginning of an investigation 
-- can bias us one way or the other.  (Concrete illustration: converting 
-0.0 to 0 seems questionable when your rule is "no loss of information", 
but seems more ambiguous when your rule is "safe".)


In the context of primitive pattern matching (including the relevant 
material in the JLS), however, it would be probably much simpler to 
allow for matches between integral types on one side and for matches 
between floating-point types on the other side, but not a mix. The 
nuisance with -0.0 and other special values would disappear altogether.




That was my first thought as well, but then I had second thoughts, as 
this seemed like wishful thinking.  If we are willing to say:


    long x = 7;
    if (x instanceof int) { /* yes, it is */ }

then it may seem quite odd to not also say

    double x = 7.0;
    if (x instanceof int) { /* this too? */ }

Because, the natural way to interpret the first is "is the value on the 
left representable in the type on the right."  Clearly 7 is 
representable as an int.  But so is 7.0; we lose nothing going from 
double 7.0 -> int 7 -> double 7.0.  And whoosh, now we're sucked into 
belly of the machine, staring down -0.0 and NaN other abominations, 
questioning our life choices.


That's not to say that your initial thought is wrong, just that it will 
be surprising if we go that way.  Maybe surprises are inevitable; maybe 
this is the least bad of possible surprises.  We should eliminate the 
"maybes" from this analysis first, though, before deciding.



Thus, while examples like

    if (anIntExpression instanceof byte b)

and

    if (aDoubleExpression instanceof float f)

make perfect sense, would an example like
    if (aDoubleExpression instanceof short s)

be pragmatically reasonable?

IIRC, the discussions about “Primitive type patterns” and “Evolving 
past reference type patterns” in the amber-spec-experts mailing list 
of April 2022 don’t even mention the mixed integral/floating-point case.




Correct, we hadn't gotten there yet, we were still trying to wrap our 
heads around how it should work in the easier cases.

Re: RFR: 8279986: methods Math::asXExact for safely checked primitive casts

[Brian Goetz]

This work is quite timely as we are now paving the way for primitive 
type patterns over in Project Amber, and also has a nontrivial 
connection with Valhalla.  If you'll pardon a brief digression...


Instanceof and casting work together in a familiar way: before you cast, 
you first ask instanceof -- but this is restricted currently to 
reference types.  But the pattern is obvious: instanceof is the 
precondition check for casting, which asks: "If I made this cast, would 
I like the answer."  There are currently two reasons to not like the 
answer: a CCE, or the operand is null (because, even though the cast 
would succeed, if you tried to use it as a member of that type, you 
wouldn't like the answer then.)


If we wanted to extend `instanceof` to primitive types, we are asking 
the same question: if I were to cast to this type, would I like the 
answer.  And casts involving primitives give us two more reasons to not 
like the answer: an NPE (due to unboxing), or loss of precision.  Which 
means that we have to specify in JLS 5.1 what cast with loss of 
precision means.  At the very least, we will want to align this work 
with that; the asXExact should be able to say "throws if the cast would 
lose precision", where "lose precision" is precisely defined by the JLS.


Separately, Project Valhalla will let us define new primitive-like 
types, such as HalfFloat and SuperLong. Conversions between primitives 
are currently specified in a complex table in JLS 5.1.  But surely we 
will want to support primitive widening conversions between HalfFloat 
and float (somehow; how we do this is a separate discussion.)  Which 
brings us back to pattern matching; narrowing casts are inherently 
partial, and declared patterns bring partiality into the "return type", 
and are the natural way (when we have it) to express things like "cast, 
but fail if you can't do so safely". This is preferable to throwing 
(which currently is the our choice.)  So it might be a little 
unfortunate to introduce throwing toXExactly now and then have to 
introduce separate patterns which signal precision loss by match 
failure.  (Though that's not the end of the world if there is some 
duplication.)


What this says is that the current proposal of toXExact is not the 
primitive, because we probably wouldn't want to implement a pattern in 
terms of the throwing version.


Converting from float/double to integral types is particularly tricky 
with -0.0.  Both answers kind of suck.  (This is a familiar situation, 
and these can be very difficult to resolve, as for each position, 
*someone* has decided the other position is untenable.)  I understand 
the rationale behind Michael H's "but its not exact", but let's not 
pretend one answer is good and the other sucks -- they both suck, and 
therefore the decision can be made on other factors.


So I have a few new wrinkles to add to the story:

 - We should wait until we have candidate JLS text for "cast conversion 
without loss of precision", and ensure the two are consistent, before 
pushing;
 - I not quite comfortable with settling the -0.0 issue just yet, there 
are some other explorations to complete first;
 - We should be prepared for the fact that we will, sometime soon, have 
to implement this whole set again as patterns that do not throw.









On 5/5/2022 6:18 AM, Raffaello Giulietti wrote:

Add a family of "safe" cast methods.

-

Commit messages:
  - 8279986: methods Math::asXExact for safely checked primitive casts
  - 8279986: methods Math::asXExact for safely checked primitive casts
  - 8279986: methods Math::asXExact for safely checked primitive casts

Changes:https://git.openjdk.java.net/jdk/pull/8548/files
  Webrev:https://webrevs.openjdk.java.net/?repo=jdk=8548=00
   Issue:https://bugs.openjdk.java.net/browse/JDK-8279986
   Stats: 615 lines in 2 files changed: 609 ins; 0 del; 6 mod
   Patch:https://git.openjdk.java.net/jdk/pull/8548.diff
   Fetch: git fetchhttps://git.openjdk.java.net/jdk  pull/8548/head:pull/8548

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

Re: Stream.fromForEach(Consumer>)

[Brian Goetz]

This is a nice use of `mapMulti` to create a stream whose contents are 
dynamically generated.  It is one step short of generators (which we 
hope Loom will eventually give us), in that it is restricted to a 
generator method that generates all of the elements in one invocation.  
This is still quite expressive.  (In hindsight, the name `generate` was 
wasted on the current `Stream::generate`, which is not all that useful 
and taking up a good name.)


I agree that the naming needs work, and people may well need help 
navigating the type `Consumer>`. Push is evocative, but 
there's more than just push; it's more like you have to kick something 
into pushing.  (Perhaps it is like the mythical Pushmi-Pullyu beast.)


Ideally, what is captured in the naming is that the outer `consumer` 
method is called exactly once, and that it should call the inner 
consumer to push elements.  (Of course that's a lot to capture.)  
Something that evokes "build by push", perhaps.  Stream::fromPush is 
about the best I've got right now.


Are there any useful Consumer> in the wild, that are 
actually typed like that?  I doubt it (there are many things convertible 
to it, though.)  Which suggests it might be fruitful to define a FI:


    interface PushSource {
    void accept(Consumer> pusher);
    }

    static Stream fromPush(PushSource source) { ... }

and Iterable::forEachRemaining and Optional::ifPresent will convert to it.


On 5/21/2022 6:54 PM, Remi Forax wrote:

Hi all,
a stream is kind of push iterator so it can be created from any object that has 
a method like forEach(Consumer),
but sadly there is no static method to create a Stream from a Consumer of 
Consumer so people usually miss that creating a Stream from events pushed to a 
consumer is easy.

By example, let say i've a very simple parser like this, it calls the 
listener/callback for each tokens

   record Parser(String text) {
 void parse(Consumer listener) {
   for(var token: text.split(" ")) {
  listener.accept(token);
   }
 }
   }

Using the method Stream.fromForEach, we can create a Stream from the sequence 
of tokens that are pushed through the listener
   var parser = new Parser("1 2");
   var stream = Stream.fromForEach(parser::parse);

It can also works with an iterable, an optional or even a collection
   Iterable iterable = ...
   var stream = Stream.fromForEach(iterable::forEachRemaning);

   Optional optional = ...
   var stream = Stream.fromForEach(optional::ifPresent);

   List list = ...
   var stream = Stream.fromForEach(list::forEach);

I known the last two examples already have their own method stream(), it's just 
to explain how Stream.fromForEach is supposed to work.

In term of implementation, Stream.fromForEach() is equivalent to creating a 
stream using a mapMulti(), so it can be implemented like this

   static  Stream fromForEach(Consumer> forEach) {
 return Stream.of((T) null).mapMulti((__, consumer) -> 
forEach.accept(consumer));
   }

but i think that Stream.fromForEach(iterable::forEachRemaning) is more readable 
than Stream.of(iterable).mapMult(Iterable::forEachRemaining).

The name fromForEach is not great and i'm sure someone will come with a better 
one.

regards,
Rémi

Re: RFR: JDK-8282798 java.lang.runtime.Carrier

[Brian Goetz]

MethodType is a useful shape for describing a pattern, in reverse; the 
“parameter” types are really the pattern bindings, and the “return” type is 
really the (minimal) type of the target.  With such a description, the 
MethodType for a pattern is the right input to get the carrier for the pattern. 
 



> On Mar 8, 2022, at 3:10 PM, Jim Laskey  wrote:
> 
> On Tue, 8 Mar 2022 16:00:48 GMT, Maurizio Cimadamore 
>  wrote:
> 
>>> We propose to provide a runtime anonymous carrier class object generator; 
>>> java.lang.runtime.Carrier. This generator class is designed to share 
>>> anonymous classes when shapes are similar. For example, if several clients 
>>> require objects containing two integer fields, then Carrier will ensure 
>>> that each client generates carrier objects using the same underlying 
>>> anonymous class. 
>>> 
>>> See JBS for details.
>> 
>> src/java.base/share/classes/java/lang/runtime/Carrier.java line 852:
>> 
>>> 850:  * @throws IllegalArgumentException if number of component slots 
>>> exceeds maximum
>>> 851:  */
>>> 852: public static MethodHandle constructor(MethodType methodType) {
>> 
>> What happens to the methodType return type? (both here and in the components 
>> method). Should javadoc say anything?
> 
> The return type is ignored. Going back to John's notion of just passing in 
> the parameterTypes would work okay. I think when Brian wrote the original 
> code he may have been thinking about keying on the MethodType (interned). 
> I'll add a note about the return type.
> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/7744

Re: RFR: JDK-8282798 java.lang.runtime.Carrier

[Brian Goetz]

It is a base assumption that the specific representation — bits in an int[], a 
class per shape, a cache of common shapes, etc — is a pure implementation 
detail that can be changed at will, based on various tradeoffs like 
footprint-vs-startup that can be made at any time.  The only assumption is that 
when you pass a shape to the constructor() method, the same (hidden) 
representation will be used when you pass the same shape in the same JVM 
invocation to the component() methods.  

Having a List view fo the components may help because of the internal 
@Stable annotation.  

For pattern matching, we envision storing the accessor MHs in condy at the use 
site.  String templates probably has a different expected user model.  

> On Mar 8, 2022, at 11:04 AM, Maurizio Cimadamore 
>  wrote:
> 
> On Tue, 8 Mar 2022 14:02:39 GMT, Jim Laskey  wrote:
> 
>> We propose to provide a runtime anonymous carrier class object generator; 
>> java.lang.runtime.Carrier. This generator class is designed to share 
>> anonymous classes when shapes are similar. For example, if several clients 
>> require objects containing two integer fields, then Carrier will ensure that 
>> each client generates carrier objects using the same underlying anonymous 
>> class. 
>> 
>> See JBS for details.
> 
> Looks good.
> I've left some minor doc-related comments.
> When reading (as discussed online) I wondered if using Unsafe to access 
> values inside a byte[] or Object[] would simplify the carrier implementation 
> somehow.
> 
> src/java.base/share/classes/java/lang/runtime/Carrier.java line 48:
> 
>> 46: 
>> 47: /**
>> 48:  * This  class is used to create objects that have number and types of
> 
> The class javadoc seems a bit on the thin side. I would say something on the 
> fact that the shape of the carrier is determined by a MethodType, etc, and 
> add an example to illustrate basic usage.
> 
> src/java.base/share/classes/java/lang/runtime/Carrier.java line 129:
> 
>> 127: 
>> 128: /**
>> 129:  * Factory for array based carrier. Array wrapped in object to 
>> provide
> 
> I found this comment hard to parse - or unhelpful to understand how the 
> factory really worked. IIUC, this factory captures all parameters into an 
> Object[] (using a collector MH) and returns that Object[] (which acts as the 
> carrier object). I'm not sure I see anything here that would prevent the user 
> from building a carrier and casting to Object[] ?
> 
> src/java.base/share/classes/java/lang/runtime/Carrier.java line 432:
> 
>> 430:  * Construct a new object carrier class based on shape.
>> 431:  *
>> 432:  * @param carrierShape  shape of carrier
> 
> Suggestion:
> 
> * @param carrierShape shape of carrier
> 
> src/java.base/share/classes/java/lang/runtime/Carrier.java line 633:
> 
>> 631:  * getter.
>> 632:  *
>> 633:  * @param i  index of component to get
> 
> Suggestion:
> 
> * @param i index of component to get
> 
> src/java.base/share/classes/java/lang/runtime/Carrier.java line 656:
> 
>> 654: 
>> 655: /**
>> 656:  * Find or create carrier class for a carrioer shape.
> 
> Suggestion:
> 
> * Find or create carrier class for a carrier shape.
> 
> src/java.base/share/classes/java/lang/runtime/Carrier.java line 852:
> 
>> 850:  * @throws IllegalArgumentException if number of component slots 
>> exceeds maximum
>> 851:  */
>> 852: public static MethodHandle constructor(MethodType methodType) {
> 
> What happens to the methodType return type? (both here and in the components 
> method). Should javadoc say anything?
> 
> -
> 
> Marked as reviewed by mcimadamore (Reviewer).
> 
> PR: https://git.openjdk.java.net/jdk/pull/7744

Re: RFR: JDK-8282798 java.lang.runtime.Carrier

[Brian Goetz]

You keep saying “what we need”.  But really, what I think you mean is “what the 
scheme I have in mind needs.”  Can we try and separate these out?  I don’t 
think any of these are *needed*, but maybe you want to make the argument that 
you have a better scheme in mind, and we should choose that scheme, and 
therefore then we might need them.  

> On Mar 8, 2022, at 10:04 AM, Remi Forax  wrote:
> 
> Hi Jim,
> I believe that there is a mismatch about what is needed for the pattern 
> matching and the API you propose.
> The Carrier API allows to map one tuple of types to one storage 
> representation based on ints, longs and references.
> 
> But what we need is to have several shapes for the same storage, mapping is 
> not one to one but many to one, more like how unions are mapped in C.
> For a switch, if we have several cases, each one need it's own shape to store 
> the bindings but at the same time we also need one field to be the same for 
> all shapes (the field of type int indicating which case match) so one storage 
> but many shapes.
> 
> Rémi
> 
> - Original Message -
>> From: "Jim Laskey" 
>> To: "core-libs-dev" 
>> Sent: Tuesday, March 8, 2022 3:11:39 PM
>> Subject: RFR: JDK-8282798 java.lang.runtime.Carrier
> 
>> We propose to provide a runtime anonymous carrier class object generator;
>> java.lang.runtime.Carrier. This generator class is designed to share 
>> anonymous
>> classes when shapes are similar. For example, if several clients require
>> objects containing two integer fields, then Carrier will ensure that each
>> client generates carrier objects using the same underlying anonymous class.
>> 
>> See JBS for details.
>> 
>> -
>> 
>> Commit messages:
>> - Introduce java.lang.runtime.Carrier
>> 
>> Changes: https://git.openjdk.java.net/jdk/pull/7744/files
>> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7744=00
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8282798
>> Stats: 1085 lines in 2 files changed: 1085 ins; 0 del; 0 mod
>> Patch: https://git.openjdk.java.net/jdk/pull/7744.diff
>> Fetch: git fetch https://git.openjdk.java.net/jdk pull/7744/head:pull/7744
>> 
>> PR: https://git.openjdk.java.net/jdk/pull/7744

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v6]

[Brian Goetz]

Yes, and ...

There are words of flags elsewhere scattered through the JDK, such the 
InnerClasses attribute, module flags in the Module attribute, and flags 
on the "requires" entries in the Module attribute.  Having one 
abstraction to cover all these locations, even though reflection doesn't 
currently serve up them all, would be a sensible thing.




On 2/17/2022 11:34 AM, Roger Riggs wrote:

On Thu, 17 Feb 2022 01:38:42 GMT, Joe Darcy  wrote:


This is an early review of changes to better model JVM access flags, that is 
"modifiers" like public, protected, etc. but explicitly at a VM level.

Language level modifiers and JVM level access flags are closely related, but 
distinct. There are concepts that overlap in the two domains (public, private, 
etc.), others that only have a language-level modifier (sealed), and still 
others that only have an access flag (synthetic).

The existing java.lang.reflect.Modifier class is inadequate to model these subtleties. For example, 
the bit positions used by access flags on different kinds of elements overlap (such as 
"volatile" for fields and "bridge" for methods. Just having a raw integer does 
not provide sufficient context to decode the corresponding language-level string. Methods like 
Modifier.methodModifiers() were introduced to cope with this situation.

With additional modifiers and flags on the horizon with projects like Valhalla, 
addressing the existent modeling deficiency now ahead of time is reasonable 
before further strain is introduced.

This PR in its current form is meant to give the overall shape of the API. It 
is missing implementations to map from, say, method modifiers to access flags, 
taking into account overlaps in bit positions.

The CSRhttps://bugs.openjdk.java.net/browse/JDK-8281660  will be filled in once 
the API is further along.

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains ten additional commits since the 
last revision:

  - Update JVMS references.
  - Merge branch 'master' into JDK-8266670
  - Reorder constants by mask value per review feedback.
  - Merge branch 'master' into JDK-8266670
  - Respond to more review feedback.
  - Respond to review feedback explicitly stating returned sets are immutable.
  - Fix typo from review feedback.
  - Merge branch 'master' into JDK-8266670
  - JDK-8266670: Better modeling of modifiers in core reflection

The jvms also has `access_flags` for parameters.
[4.7.24. The MethodParameters 
Attribute](https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.7.24)

Even if java.lang.reflect.Parameter is not a "Member", it would be useful for 
Parameter to have an `accessFlags()` method and loosen up the spec for AccessFlag to 
allow its use in Parameter.
Its access flags have the same underlying bit patterns and definitions.

-

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

Re: [External] : Sequenced Collections

[Brian Goetz]

It's a time-honored tradition in naming bikesheds to pick and choose the 
precedents you want to be "consistent" with, and I think that's what's 
going on here with your preference for "Ordered" over "Sequenced".  But 
in an ecosystem as large as Java, you can always find conflicting 
precedents to be "consistent" with, which makes "for consistency" 
arguments generally weak.


There's a reason that we didn't propose "ordered", and it's not because 
it didn't occur to us; it is already heavily used to indicate ordering 
of the elements.  Any mention of Comparator or Comparable will quickly 
use the word "order" to mean ordering of the elements (after the 
mathematical terms "partial order" and "total order".)  From Comparator 
(first lines):


 * A comparison function, which imposes a total ordering on
 * some collection of objects.  This ordering is referred to as the 
class's natural

 * ordering

From Comparable (first lines):

 * Compares this object with the specified object for order.

From TreeSet:

 * Constructs a new, empty tree set, sorted according to the
 * natural ordering of its elements.

From Collections::sort:

 * Sorts the specified list into ascending order, according to the
 * {@linkplain Comparable natural ordering} of its elements.

And there are also helper methods like Collections::reverseOrder and 
Comparator::naturalOrder.



So, we can find precedent for "ordered" meaning element order, and also 
for it meaning has-an-encounter-order.  So how do we choose?  Well, 
we're talking about enhancing *collections*, and throughout Collections, 
"ordered" primarily means "element order".  So while you could be mad at 
Streams for having used "ordered" to mean "has an encounter order", it's 
still not the case that both terms have an equal claim; context matters.








On 2/15/2022 2:38 AM, Glavo wrote:

On the one hand, I am very opposed to using the name ` SequencedCollection
`. In the JVM ecosystem, several widely used libraries are using the term
Seq,
Typical examples are Scala and kotlin's standard libraries, Scala uses
`Seq` as a similar correspondence to a `List`, while Kotlin's `Sequence`
represents something with semantics between `Iterable` and `Stream`.
Introducing things with similar names may lead to more confusion.

On the other hand, we have defined the meaning of "ordered".
Refer to `Stream::forEachOrdered` and `Spliterator.ORDERED`. Their
semantics for known collections are similar to the one you discussed.
Direct reuse is not only shorter, but also more harmonious and unified in
semantics.
So I think `OrderedCollection` is a good choice.


On Tue, Feb 15, 2022 at 2:45 PM Stuart Marks
wrote:



On 2/11/22 7:24 PM, Tagir Valeev wrote:

Of course, I strongly support this initiative and am happy that my

proposal got some

love and is moving forward. In general, I like the JEP in the way it is.

I have only

two slight concerns:
1. I'm not sure that having addition methods (addFirst, addLast,

putFirst, putLast)

is a good idea, as not every mutable implementation may support them.

While this

adds some API unification, it's quite a rare case when this could be

necessary. I

think most real world applications of Sequenced* types would be around

querying, or

maybe draining (so removal operations are ok). Probably it would be

enough to add

addFirst, addLast, putFirst, putLast directly to the compatible
implementations/subinterfaces like List, LinkedHashSet, and

LinkedHashMap removing

them from the Sequenced* interfaces. In this case, SortedSet interface

will not be

polluted with operations that can never be implemented. Well my opinion

is not very

strong here.

Hi Tagir, thanks for looking at this.

Yes, this particular issue involves some tradeoffs. As you noted,
addFirst/addLast
can't be implemented by SortedSet and so they throw UOE. This is an
unfortunate
asymmetry. If these were omitted, the design would be cleaner in the sense
that
there would be fewer things that throw UOE.

But there are costs to not having those methods, which I think outweigh
the
asymmetry around SortedSet.

The other collections have interfaces corresponding to common
implementations:
ArrayList has List, ArrayDeque has Deque, TreeSet has SortedSet, etc., and
Java
style tends to encourage "programming to the interface." But there's no
interface
that corresponds to LinkedHashSet.

Over the years we've mostly just put up with this gap. But it's really
noticeable
when you add the reversed view. The reversed view of a List is a List, the
reversed
view of a Deque is a Deque, the reversed view of a SortedSet is a
SortedSet, and the
reversed view of a LinkedHashSet is a ... what? SequencedSet is the answer
here.

We also want the reversed view to be equivalent in power to the forward
view. If the
addFirst/addLast methods were only on LinkedHashSet, it would be possible
to add at
either end of a LinkedHashSet but not its reversed view. This is a big
hole. So the
addFirst/addLast methods need to 

Re: RFR: 8280915: Better parallelization for AbstractSpliterator and IteratorSpliterator when size is unknown

[Brian Goetz]

Are you proposing dropping SIZED from the spliterator for arrays?  This 
would undermine all the array-based optimizations (e.g., toArray), which 
seems a bad trade.  I realize the splitting heuristics are frustrating 
for a number of use cases, but this seems like throwing the baby out 
with the bathwater.


(Loom is coming, and that is a good time to revisit streams support for 
blocking operations, which is a big part of what people complain about 
with parallel streams.)


On 1/29/2022 11:38 AM, Tagir F.Valeev wrote:

See the bug description for details.

I propose a simple solution. Let's allow ArraySpliterator to be non-SIZED and 
report artificial estimatedSize(), much bigger than the real one. This will allow 
AbstractSpliterator and IteratorSpliterator to produce prefix whose size is 
comparable to Long.MAX_VALUE (say, starting with Long.MAX_VALUE/2), and this will 
enable further splitting of the prefix. This change will drastically improve 
parallel streaming for affected streams of size <= 1024 and significantly 
improve for streams of size 1025..2. The cost is higher-grained splitting for 
huge streams of unknown size. This might add a minor overhead for such scenarios 
which, I believe, is completely tolerable.

No public API changes are necessary, sequential processing should not be 
affected, except an extra field in ArraySpliterator which increases a footprint 
by 8 bytes.

I added a simple test to ensure that at least two threads are actually used 
when parallelizing Stream.iterate source. More testing ideas are welcome.

-

Commit messages:
  - JDK-8280915 Better parallelization for AbstractSpliterator and 
IteratorSpliterator when size is unknown

Changes:https://git.openjdk.java.net/jdk/pull/7279/files
  Webrev:https://webrevs.openjdk.java.net/?repo=jdk=7279=00
   Issue:https://bugs.openjdk.java.net/browse/JDK-8280915
   Stats: 128 lines in 2 files changed: 96 ins; 0 del; 32 mod
   Patch:https://git.openjdk.java.net/jdk/pull/7279.diff
   Fetch: git fetchhttps://git.openjdk.java.net/jdk  pull/7279/head:pull/7279

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

Re: a quick question about String

[Brian Goetz]

As the language currently stands, new means new; it is guaranteed to create a 
new identity.  (When Valhalla comes along, instances of certain classes will be 
identity-free, so the meaning of new will change somewhat, but String seems 
likely to stay as it is.)  

The language is allowed (in some cases required) to intern string _literals_, 
so the expression

“foo” == “foo”

can be true.  That’s OK because no one said “new”.  

In your example, the two instances would not be ==, but would be .equals.  But 
since “equivalent” is not a precise term, we can have an angels-on-pins debate 
about whether they are indeed equivalent.  IMO equivalent here means “for 
practical purposes”; locking on an arbitrary string which you did not allocate 
is a silly thing to do, so it is reasonable that the doc opted for a 
common-sense interpretation of equivalent, rather than a more precise one.  



> On Dec 24, 2021, at 11:12 AM, Alan Snyder  wrote:
> 
> Just when I thought the answer was simple, now it seems more complex.
> 
> Are you saying that new would always create a new object but the GC might 
> merge multiple instances of String into a single instance?
> 
> Also, if new String() always creates a new instance, then it seems that this 
> statement (from String.java) is not quite right:
> 
> Because String objects are immutable they can be shared. For example:
> 
> String str = "abc";
> is equivalent to:
> 
> char data[] = {'a', 'b', 'c'};
> String str = new String(data);
> 
> 
> 
> 
>> On Dec 23, 2021, at 2:55 PM, Simon Roberts  
>> wrote:
>> 
>> I think there are two things at stake here, one is that as I understand it,
>> "new means new", in every case. This is at least partly why the
>> constructors on soon-to-be value objects are deprecated; they become
>> meaningless. The other is that if the presumption is that we should
>> always intern new Strings, I must disagree. Pooling takes time and memory
>> to manage, and if there are very few duplicates, it's a waste of both. I
>> believe it should be up to the programmer to decide if this is appropriate
>> in their situation. Of course, the GC system seems to be capable of
>> stepping in in some incarnations, which adds something of a counterexample,
>> but that is, if I recall, configurable.
>> 
>> 
>> On Thu, Dec 23, 2021 at 2:53 PM Xeno Amess  wrote:
>> 
>>> never should，as Object can be use as lock.
>>> 
>>> XenoAmess
>>> 
>>> From: core-libs-dev  on behalf of
>>> Bernd Eckenfels 
>>> Sent: Friday, December 24, 2021 5:51:55 AM
>>> To: alan Snyder ; core-libs-dev <
>>> core-libs-dev@openjdk.java.net>
>>> Subject: Re: a quick question about String
>>> 
>>> new String() always creates a new instance.
>>> 
>>> Gruss
>>> Bernd
>>> --
>>> http://bernd.eckenfels.net
>>> 
>>> Von: core-libs-dev  im Auftrag von
>>> Alan Snyder 
>>> Gesendet: Thursday, December 23, 2021 6:59:18 PM
>>> An: core-libs-dev 
>>> Betreff: a quick question about String
>>> 
>>> Do the public constructors of String actually do what their documentation
>>> says (allocate a new instance), or is there some kind of compiler magic
>>> that might avoid allocation?
>>> 
>>> 
>> 
>> -- 
>> Simon Roberts
>> (303) 249 3613
>> 
> 

Re: Adding an @Immutable annotation to Java

[Brian Goetz]

I don’t see how a restricted reference by itself is useful, if you cannot 
depend upon the object not being mutated via other references.

Agree; this may help you do local proofs of correctness, and may conceivably 
help the JIT (though, its pretty smart about figuring this stuff out on its 
own), but the benefit doesn’t rise to the level of the complexity here.

The main value of an @Immutable designation is that it is safe to share a 
reference without additional coordination (both sharing with untrusted code, 
and sharing across threads).  But, this is only useful if the designation is 
(a) true all the way down, and (b) cannot be subverted by unsafe casts.

One needs look no farther than String to realize the near-hopelessness of this 
task.  A String is logically immutable, but its representation includes (a) a 
mutable cache of its hashCode, and (b) a reference to a mutable array of bytes 
or chars.  Even if we were willing to throw (a) overboard, (b) would require 
some sort of @TrustMe, which we could conceivably do for jl.String, but 
couldn’t possibly expose more broadly without driving the value of the 
mechanism to near-zero.  Treat this as an existence proof that mutability is 
just too pervasive to contain, at least for now.

There are efforts underway to chip away at some of the untamed mutability; 
frozen arrays are on the drawing board (addressing (b)), and Valhalla will join 
records in defining _shallowly_ immutable aggregates.  (It could conceivably go 
farther, but not until we can at least bring String into the fold.)  But we’d 
need to make a lot more progress before anyone could consider believing an 
@Immutable designation.  And if it can’t be believed, it doesn’t have enough 
value to put in the language.  (No problem with privately using annotations to 
capture design intent in documentation, but the bar for the language is higher 
than that.)

There are other impediments too; much ink has been spilled on the challenges of 
capturing immutability in a type hierarchy as complex as Collections.  But my 
main point is that while this is something that initially seems desirable, when 
you start pulling on the string, you realize it is not as useful as it 
initially seems in an environment of pervasive mutability.

Re: Feature request: custom default annotation attributes

[Brian Goetz]

While I'm the first to admit that the design of annotations incorporated 
a great deal of optimism about its sufficiency, I think the 
return-on-complexity for such things is not good enough to warrant 
working on this.


Now I was thinking: wouldn't it be nice to be able to define a custom 
default attribute? That could be done using an annotation, let's call 
it @DefaultAttribute. 


Actually it can't.  The cardinal design principle of annotations is: 
they do not affect language semantics.  So you'd need actual language 
support here (such as marking the default attribute with a keyword like 
"default-attribute".)  That's not impossible, but there's clearly a host 
of conditions and interactions to deal with. Yes, they're mostly 
trivial, but there's always more of them than one imagines at first.  
But if we do this feature (which seems trivial, but no language feature 
is trivial), we're implicitly deciding to delay or not do some other 
feature.  And the other features on the menu all offer a much greater 
return.


(Such features are like single-purpose kitchen appliances; a countertop 
hot-dog cooker might be the best way to make hot dogs, but single-use 
appliances usually offer a pretty poor return on counter space.  (Except 
the rice cooker; that's the one single-purpose appliance we have in our 
house, and I wouldn't give it up.))


So, given an infinite budget for implementation, language surface, and 
complexity, its not something I'd rule out because its terrible 
(clearing this bar is actually pretty good), but realistically I don't 
see it coming near the top of the list of features we'd want to invest in.

Re: RFR: 8276904: Optional.toString() is unnecessarily expensive

[Brian Goetz]

I would suggest that we hold this patch until the string interpolation 
JEP lands.  It will offer both better readability *and* better 
performance, and probably better to have one round of "replace all the 
toString implementations" than two?


On 11/10/2021 1:04 PM, Eamonn McManus wrote:

Use string concatenation instead of `String.format`.

-

Commit messages:
  - 8276904: Optional.toString() is unnecessarily expensive

Changes:https://git.openjdk.java.net/jdk/pull/6337/files
  Webrev:https://webrevs.openjdk.java.net/?repo=jdk=6337=00
   Issue:https://bugs.openjdk.java.net/browse/JDK-8276904
   Stats: 4 lines in 4 files changed: 0 ins; 0 del; 4 mod
   Patch:https://git.openjdk.java.net/jdk/pull/6337.diff
   Fetch: git fetchhttps://git.openjdk.java.net/jdk  pull/6337/head:pull/6337

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

Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources

[Brian Goetz]

I am not really sure we’ve gotten the right idiom here yet.  I’d like to slow 
down a bit before making an API decision.  

What id suggest is capturing the proposed api and spec on list, and let’s let 
this sit and bake for a bit longer.  My sense is that something better may well 
emerge if we do. 

Sent from my MacBook Wheel

> On Oct 9, 2021, at 5:41 AM, Tagir F.Valeev  wrote:
> 
> On Sun, 3 Oct 2021 11:00:25 GMT, Tagir F. Valeev  wrote:
> 
>> Currently, when the stream holds a resource, it's necessary to wrap it with 
>> try-with-resources. This undermines the compact and fluent style of stream 
>> API calls. For example, if we want to get the `List` of files inside the 
>> directory and timely close the underlying filehandle, we should use 
>> something like this:
>> 
>> 
>> List paths;
>> try (Stream stream = Files.list(Path.of("/etc"))) {
>>paths = stream.toList();
>> }
>> // use paths
>> 
>> 
>> I suggest to add a new default method to Stream interface named 
>> `consumeAndClose`, which allows performing terminal stream operation and 
>> closing the stream at the same time. It may look like this:
>> 
>> 
>>default  R consumeAndClose(Function, ? extends R> 
>> function) {
>>Objects.requireNonNull(function);
>>try(this) {
>>return function.apply(this);
>>}
>>}
>> 
>> 
>> Now, it will be possible to get the list of the files in the fluent manner:
>> 
>> 
>> List list = 
>> Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
> 
> CSR is [posted](https://bugs.openjdk.java.net/browse/JDK-8274994), please 
> review!
> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/5796

Re: Discussion: easier Stream closing

[Brian Goetz]

In Java 8, I think we were reluctant to lean on the idiom of "pass me a 
lambda and I'll pass it the confined data"), because Java developers 
were already struggling to understand lambdas.  But now that we're 
mostly over that hurdle, API points that accept Consumer are 
a powerful way to gain confinement (as we've seen in StackWalker, and 
also in the explorations of ScopeLocal in Loom.)  So I have no objection 
to this idiom.


I have some concern that putting these side-by-side with existing 
Files.walk(...) methods will be a source of confusion, creating two 
different ways to do the same thing.


I would rather not add anything new to BaseStream; it was a mistake to 
expose at all, and I'd rather see it deprecated than add more to it.  
However, adding it individually to the Stream implementations is 
equivalent, and doesn't have this problem.


The consumeAndClose approach is clever, in that it adds one API point 
that works for all streams, rather than having to add a new API point 
for every factory of a closeable stream; on the other hand, it is 
dramatically less discoverable, and so requires more education to get 
people to use it (and, as you say, has the exception problem.)


On 9/26/2021 5:27 AM, Tagir Valeev wrote:

Hello!

With current NIO file API, a very simple problem to get the list of
all files in the directory requires some ceremony:

List paths;
try (Stream stream = Files.list(Path.of("/etc"))) {
 paths = stream.toList();
}

If we skip try-with-resources, we may experience OS file handles leak,
so it's desired to keep it. Yet, it requires doing boring stuff. In
this sense, classic new File("/etc").list() was somewhat more
convenient (despite its awful error handling).

I like how this problem is solved in StackWalker API [1]: it limits
the lifetime of the Stream by requiring a user-specified function to
consume it. After the function is applied, the stream is closed
automatically. We could add a similar overload to the Files API. As an
additional feature, we could also translate all UncheckedIOException's
back to IOException for more uniform exception processing:

/**
  * @param dir  The path to the directory
  * @param function  function to apply to the stream of directory files
  * @param  type of the result
  * @return result of the function
  * @throws IOException if an I/O error occurs when opening the directory, or
  * UncheckedIOException is thrown by the supplied function.
  */
public static  T list(Path dir, Function, ?
extends T> function) throws IOException {
 try (Stream stream = Files.list(dir)) {
 return function.apply(stream);
 }
 catch (UncheckedIOException exception) {
 throw exception.getCause();
 }
}

In this case, getting the List of all files in the directory will be
as simple as

List paths = Files.list(Path.of("/etc"), Stream::toList);
This doesn't limit the flexibility. For example, if we need only file
names instead of full paths, we can do this:
List paths = Files.list(Path.of("/etc"), s ->
s.map(Path::getFileName).toList());

Alternatively, we could enhance the BaseStream interface in a similar
way. It won't allow us to translate exceptions, but could be useful
for every stream that must be closed after consumption:

// in BaseStream:

/**
  * Apply a given function to this stream, then close the stream.
  * No further operation on the stream will be possible after that.
  *
  * @param function function to apply
  * @param  type of the function result
  * @return result of the function
  * @see #close()
  */
default  R consumeAndClose(Function function) {
 try(@SuppressWarnings("unchecked") S s = (S) this) {
 return function.apply(s);
 }
}

The usage would be a little bit longer but still more pleasant than
explicit try-with-resources:

List list = Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);

On the other hand, in this case, we are free to put intermediate
operations outside of consumeAndClose, so we won't need to nest
everything inside the function. Only terminal operation should be
placed inside the consumeAndClose. E.g., if we need file names only,
like above, we can do this:

List list =
Files.list(Path.of("/etc")).map(Path::getFileName).consumeAndClose(Stream::toList);

What do you think?


[1] 
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#walk(java.util.function.Function)

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null

[Brian Goetz]

Actually, it will not NPE if `names` is null and you have selected 
equals/hashCode as the name.  Might be better to do requiresNonNull() up 
front for all the arguments, just to make such analysis simpler:


requireNonNull(methodName);
requireNonNull(type);
requireNonNull(recordClass);
requireNonNull(names);
    requireNonNull(getters);

On 8/23/2021 4:04 PM, Brian Goetz wrote:

+1

On 8/23/2021 2:22 PM, Vicente Romero wrote:
Please review this simple PR along with the associated CSR. The PR is 
basically adding a line the the specification of method 
`java.lang.runtime.ObjectMethods::bootstrap` stating under what 
conditions a NPE will be thrown.


TIA

link to the [CSR](https://bugs.openjdk.java.net/browse/JDK-8272852)

-

Commit messages:
  - 8272347: ObjectMethods::bootstrap should specify NPE if any 
argument except lookup is null


Changes: https://git.openjdk.java.net/jdk/pull/5226/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5226=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8272347
   Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
   Patch: https://git.openjdk.java.net/jdk/pull/5226.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk 
pull/5226/head:pull/5226


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

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null

[Brian Goetz]

+1

On 8/23/2021 2:22 PM, Vicente Romero wrote:

Please review this simple PR along with the associated CSR. The PR is basically 
adding a line the the specification of method 
`java.lang.runtime.ObjectMethods::bootstrap` stating under what conditions a 
NPE will be thrown.

TIA

link to the [CSR](https://bugs.openjdk.java.net/browse/JDK-8272852)

-

Commit messages:
  - 8272347: ObjectMethods::bootstrap should specify NPE if any argument except 
lookup is null

Changes: https://git.openjdk.java.net/jdk/pull/5226/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5226=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8272347
   Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
   Patch: https://git.openjdk.java.net/jdk/pull/5226.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk pull/5226/head:pull/5226

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

Re: Implementing MethodHandleProxies#asInterfaceInstance with hidden classes

[Brian Goetz]

This was an early attempt at the functionality provided by LambdaMetafactory.  
It could probably be reimplemented on top of that, but probably could be 
deprecated in favor of LMF as well. 

Sent from my iPad

> On Aug 22, 2021, at 10:08 PM, liangchenb...@gmail.com wrote:
> 
> Currently, java.lang.invoke.MethodHandleProxies#asInterfaceInstance [1] is
> implemented with java.lang.reflect.Proxy. After looking at its public API,
> including Javadoc, it seems that MethodHandleProxies would benefit from a
> hidden class implementation without changing its public API definition
> (including Javadocs).
> 
> Recently, there is JEP 416 [2] for reimplementing reflection based on
> method handles. This implementation utilizes hidden classes with method
> handles passed in classdata and retrieved to condy, which allows generic
> method handles (beyond regular constable ones) to be optimized in method
> calls. Similarly, for MethodHandleProxies, hidden classes allow the
> implementations to detach from classloaders and be freely recyclable; they
> can use class data to store the adapted method handles (which can
> significantly speed up invocations); and it can allow it to support
> implementing single-abstract-method abstract classes in the future (as
> discussed in its Javadoc) as well, with only minor tweaks.
> 
> Does this sound feasible? I want to ensure it is a good idea before any
> implementation is attempted. If there is any issue with this vision, please
> don't hesitate to point it out. Feel free to comment, too! If this looks
> good, I hope an issue can be created for it.
> 
> Best
> 
> [1]
> https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/invoke/MethodHandleProxies.html
> [2] https://openjdk.java.net/jeps/416

Re: RFR: 8272137: Make Collection and Optional classes streamable

[Brian Goetz]

For the third time: This discussion illustrates why the PR was 
premature; the design was not agreed upon first.  High-level design 
discussions (i.e., "is this a good design", "is this a good idea at 
all", "are we solving the right problem", "does it need to be solved in 
the JDK") should happen first on the discussion lists; if they happen on 
a PR like this, that's clear evidence that important steps have been 
skipped.




On 8/14/2021 10:48 PM, CC007 wrote:

On Mon, 9 Aug 2021 12:28:23 GMT, CC007  
wrote:


create Streamable and ParallelStreamable interface and use them in Collection 
and Optional

I read through [these 
posts](http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/2013-June/thread.html#1910)
 and while I did see good reasons for not moving stream to Iterable, I didn't see any 
mention of a separate Streamable-like interface, so could you elaborate on that it 
"did not carry its weight"?

-

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

Re: RFR: 8272137: Make Collection and Optional classes streamable

[Brian Goetz]

On Mon, 9 Aug 2021 12:28:23 GMT, CC007  
wrote:

> create Streamable and ParallelStreamable interface and use them in Collection 
> and Optional

To reiterate: These issues were explored in the JSR 335 EG and it was agreed 
that this abstraction did not carry its weight. In any case, it is premature to 
bring a PR for a significant API change that has not been discussed first on 
corelibs-dev. Please withdraw the PR, and if you want to continue the 
discussion, bring it to corelibs-dev.

Sent from my iPad

> On Aug 14, 2021, at 9:10 PM, CC007 ***@***.***> wrote:
> 
> 
> I understand what you are proposing. I do not believe Streamable carries its 
> weight.
> 
> That argument seems subjective. Could you elaborate on that in an objective 
> manner?
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or unsubscribe.
> Triage notifications on the go with GitHub Mobile for iOS or Android.

-

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

Re: RFR: 8272137: Make Collection and Optional classes streamable

[Brian Goetz]

On Mon, 9 Aug 2021 12:28:23 GMT, CC007  
wrote:

> create Streamable and ParallelStreamable interface and use them in Collection 
> and Optional

I understand what you are proposing. I do not believe Streamable carries its 
weight.  



Sent from my iPad

> On Aug 14, 2021, at 8:53 PM, CC007 ***@***.***> wrote:
> 
> 
> I object to this change. These issues were explored in the JSR 335 EG and it 
> was agreed that this abstraction did not carry its weight. In any case, it is 
> premature to bring a PR for a significant API change that has not been 
> discussed first on corelibs-dev. Please withdraw the PR, and if you want to 
> continue the discussion, bring it to corelibs-dev.
> 
> I agree with the spliterator performance issues and the ability to be able to 
> create IntStreams from Iterable. Therefore option 1 and 2 from JDK-8272137 
> seem fairly unfeasible to implement. This was already hinted at there, but 
> for different reasons.
> 
> This PR however implements option 4: an option that doesn't actually make the 
> Iterable class have a stream method, but instead abstracts the stream method 
> to an interface. This method is then implemented by Collection and Optional. 
> It also allows other code to implement this interface.
> 
> Using this implementation, you don't have the issues that the JSR 335 EG 
> specified that would be present if Iterable itself were to be made 
> Streamable. You wouldn't want that anyway, because streamability and 
> iterability are two similar but very separate concerns in Java.
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or unsubscribe.
> Triage notifications on the go with GitHub Mobile for iOS or Android.

-

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

Re: RFR: 8272137: Make Iterable classes streamable

[Brian Goetz]

On Mon, 9 Aug 2021 12:28:23 GMT, CC007  
wrote:

> create Streamable and ParallelStreamable interface and use them in Collection 
> and Optional

I object to this change.  These issues were explored in the JSR 335 EG and it 
was agreed that this abstraction did not carry its weight.  In any case, it is 
premature to bring a PR for a significant API change that has not been 
discussed first on corelibs-dev.  Please withdraw the PR, and if you want to 
continue the discussion, bring it to corelibs-dev.

-

Changes requested by briangoetz (Reviewer).

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

Re: RFR: 8272137: Make Iterable classes streamable

[Brian Goetz]

Starting with a PR is not the way to add significant API surface to the JDK.   
You have to first build consensus on the mailing list for the design concept 
before we start talking about code.  

FTR, Interfaces like these were already discussed and rejected in the JSR 335 
design discussions.  

Sent from my iPad

> On Aug 14, 2021, at 6:00 AM, Rémi Forax  wrote:
> 
> On Mon, 9 Aug 2021 12:28:23 GMT, CC007 
>  wrote:
> 
>> create Streamable and ParallelStreamable interface and use them in 
>> Collection and Optional
> 
> Hi Rick,
> I do not think that such interfaces are a good addition to the JDK,
> we do not want to promote the fact that you can create a Stream from an 
> Iterable because the resulting Stream is not efficient (or is not as 
> efficient as it could be) and we can already uses a Supplier if we 
> want to to represent a factory of streams
> 
> See my comment on the bug for more details
> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/5050

Re: JNI WeakGlobalRefs

[Brian Goetz]

You might also consider bringing this to panama-dev, since that will 
eventually be the recommended replacement for most uses of JNI.


On 7/21/2021 6:25 PM, David Holmes wrote:

Hi Hans,

On 22/07/2021 7:54 am, Hans Boehm wrote:

Is this an appropriate list to discuss JNI?


No - hotspot-dev (to get runtime and GC folk) is the place to discuss 
JNI.


Thanks,
David


I'm concerned that the current semantics of JNI WeakGlobalRefs are still
dangerous in a very subtle way that is hidden in the spec. The current
(14+) spec says:

“Weak global references are related to Java phantom references
(java.lang.ref.PhantomReference). A weak global reference to a specific
object is treated as a phantom reference referring to that object when
determining whether the object is phantom reachable (see java.lang.ref).
---> Such a weak global reference will become functionally equivalent to
NULL at the same time as a PhantomReference referring to that same 
object

would be cleared by the garbage collector. <---”

(This was the result of JDK-8220617, and is IMO a large improvement over
the prior version, but ...)

Consider what happens if I have a WeakGlobalRef W that refers to a Java
object A which, possibly indirectly, relies on an object F, where F is
finalizable, i.e.

W - - -> A -> ... -> F

Assume that F becomes invalid once it is finalized, e.g. because the
finalizer deallocates a native object that F relies on. This seems to 
be a

very common case. We are then exposed to the following scenario:

0) At some point, there are no longer any other references to A or F.
1) F is enqueued for finalization.
2) W is dereferenced by Thread 1, yielding a strong reference to A and
transitively to F.
3) F is finalized.
4) Thread 1 uses A and F, accessing F, which is no longer valid.
5) Crash, or possibly memory corruption followed by a later crash 
elsewhere.


(3) and (4) actually race, so there is some synchronization effort 
and cost
required to prevent F from corrupting memory. Commonly the 
implementer of W

will have no idea that F even exists.

I believe that typically there is no way to prevent this scenario, 
unless
the developer adding W actually knows how every class that A could 
possibly

rely on, including those in the Java standard library, are implemented.

This is reminiscent of finalizer ordering issues. But it seems to be 
worse,

in that there isn't even a semi-plausible workaround.

I believe all of this is exactly the reason PhantomReference.get() 
always

returns null, while WeakReference provides significantly different
semantics, and WeakReferences are enqueued when an object is enqueued 
for

finalization.

The situation improves, but the problem doesn't fully disappear, in a
hypothetical world without finalizers. It's still possible to use
WeakGlobalRef to get a strong reference to A after a WeakReference to 
A has
been cleared and enqueued. I think the problem does go away if all 
cleanup

code were to use PhantomReference-based Cleaners.

AFAICT, backward-compatibility aside, the obvious solution here is to 
have
WeakGlobalRefs behave like WeakReferences. My impression is that this 
would
fix significantly more broken clients than it would break correct 
ones, so

it is arguably still a viable option.

There is a case in which the current semantics are actually the desired
ones, namely when implementing, say, a String intern table. In this case
it's important the reference not be cleared even if the referent is, at
some point, only reachable via a finalizer. But this use case again 
relies
on the programmer knowing that no part of the referent is invalidated 
by a

finalizer. That's a reasonable assumption for the
Java-implementation-provided String intern table. But I'm not sure it's
reasonable for any user-written code.

There seem to be two ways forward here:

1) Make WeakGlobalRefs behave like WeakReferences instead of
PhantomReferences, or
2) Add strong warnings to the spec that basically suggest using a strong
GlobalRef to a WeakReference instead.

Has there been prior discussion of this? Are there reasonable use 
cases for

the current semantics? Is there something else that I'm overlooking? If
not, what's the best way forward here?

(I found some discussion from JDK-8220617, including a message I posted.
Unfortunately, it seems to me that all of us overlooked this issue?)

Hans

Re: [jdk17] RFR: 8268766: Desugaring of pattern matching enum switch should be improved

[Brian Goetz]

Bootstrap methods are cheap. If you have one that is semantically 
different, better to write a separate bootstrap than to try and cram two 
sets of functionality into one.  So +1 for "make a new bootstrap for enums."


On 6/16/2021 11:25 AM, Jan Lahoda wrote:

Currently, an enum switch with patterns is desugared in a very non-standard, 
and potentially slow, way. It would be better to use the standard `typeSwitch` 
bootstrap to classify the enum constants. The bootstrap needs to accept enum 
constants as labels in order to allow this. A complication is that if an enum 
constant is missing, that is not an incompatible change for the switch, and the 
switch should simply work as if the case for the missing constant didn't exist. 
So, the proposed solution is to have a new bootstrap `enumConstant` that 
converts the enum constant name to the enum constant, returning `null`, if the 
constant does not exist. It delegates to `ConstantBootstraps.enumConstant` to 
do the actual conversion. And `typeSwitch` accepts `null`s as padding.

How does this look?

-

Commit messages:
  - Adding and fixing test.
  - Merging master.
  - 8268766: Desugaring of pattern matching enum switch should be improved

Changes: https://git.openjdk.java.net/jdk17/pull/81/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=81=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8268766
   Stats: 313 lines in 6 files changed: 165 ins; 76 del; 72 mod
   Patch: https://git.openjdk.java.net/jdk17/pull/81.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/81/head:pull/81

PR: https://git.openjdk.java.net/jdk17/pull/81

Re: case null vs case dominance

[Brian Goetz]

On 6/7/2021 5:51 AM, Remi Forax wrote:

Hi all,
the first part of the message is about javac error message that could be 
improved,
the second part is about the current spec being not very logical.

With this code

 Object o = null;
 var value = switch(o) {
   //case null -> 0;
   case Object __ -> 0;
   case null -> 0;
 };
 System.out.println(value);

The error message is
   PatternMatching101.java:70: error: this case label is dominated by a 
preceding case label
   case null -> 0;
   ^

The error message is wrong here, because it's 'case null' and you can put a 
case null where you want but below a total pattern, so the error mesage should 
reflect that.


But the case null *is* dominated by the total pattern, and therefore dead.


Here is an example that compiles showing that case null can be below a case 
String, which it dominates

 Object o = null;
 var value = switch(o) {
   case String s -> 0;
   case null -> 0;
   default -> 0;
 };


In this case, the String pattern is not total, so it does not dominate null.

Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[Brian Goetz]

It seems pretty clear that this "feature" is a leftover from an early 
implementation, doesn't clearly say what it is supposed to do, is more 
complicated than it looks, and is buggily implemented.  While I 
understand the temptation to "fix" it, at this point we'd realistically 
be adding a basically entirely new reflection feature that hasn't gone 
through any sort of design analysis -- we'd just be guessing about what 
the original intent of this vestigial feature is.  It seems the 
reasonable options are to either leave it alone, deprecate it, or engage 
in a much more deliberate redesign.  (And given the fact that I can 
think of at least 1,000 things that are higher priority, I'd not be 
inclined to pursue the third option.)


On 6/2/2021 11:06 PM, Jaroslav Tulach wrote:

On Tue, 1 Jun 2021 09:30:40 GMT, Jaroslav Tulach 
 wrote:


There doesn't seem to be much support for the complete changes in #4245. To get 
at least something useful from that endeavor I have extracted the test for 
existing behavior of `-XX:+PreserveAllAnnotations` and I am offering it in this 
pull request without any changes to the JVM behavior.

FYI: The current test is a "mirror" copy of 
[AnnotationTypeRuntimeAssumptionTest.java](https://github.com/openjdk/jdk/blob/0f689ea8f29be113cc8a917a86fd9e0f85f921fc/test/jdk/java/lang/annotation/AnnotationType/AnnotationTypeRuntimeAssumptionTest.java)
 with adjustments to cover the `CLASS`->`RUNTIME` migration.


What I would do is to add a patch for the parser bug

That's an interesting discovery! I see a patch and I can include it as your 
commit in this PR, if there is a test...


and then extend the test in 3 dimensions:

...or just take my PR and expand it. Also the checkbox _"Allow edits and access to 
secrets by maintainers"_ is on - e.g. this PR is open for modifications.

-

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

Re: RFR: 8265130: Make ConstantDesc class hierarchy sealed [v2]

[Brian Goetz]

The motivation here is that we wanted to preserve the ability to 
describe "special" indy sites with special objects.  The standard 
implementation can describe any indy site (bootstrap, static args, 
invocation name and type), but some indy sites (e.g., lambda capture) 
are "special".  It would be reasonable for a classfile parser to 
"uplevel" such sites, so that (a) parsers could provide implementations 
that interpret the semantics of the bootstrap argument list relative to 
the known bootstrap method, and (b) so that class generators (including 
compilers) could provide higher-level descriptions of the indy.  This 
allows us to abstract away from the details of compiler translation, by 
"unlowering" translation-by-indy for known bootstraps.




On 5/20/2021 7:43 PM, Mandy Chung wrote:

On Thu, 20 May 2021 22:35:38 GMT, Thiago Henrique Hüpner 
 wrote:


I should have made it clear.  I was expecting `DynamicConstantDesc` should be 
`sealed`.  Do you expect non-JDK implementation class extending 
`DynamicConstantDesc`?

 From the ConstantDesc Javadoc:

  * Non-platform classes should not implement {@linkplain ConstantDesc} 
directly.
  * Instead, they should extend {@link DynamicConstantDesc} (as {@link EnumDesc}
  * and {@link VarHandleDesc} do.)

Thanks.  I missed this javadoc.

-

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

Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[Brian Goetz]

Since the discussion happened over the holiday weekend, I didn't get a 
chance to respond until now, but I think that this came to a good 
outcome.  As Alan's archaeology discovered, this flag appears to be a 
leftover from the original implementation, and I could find no signs of 
its usage.  We might consider deprecating it (though, we might also 
leave sleeping dogs alone), but its good to have a test for the current 
behavior.


Allow me to make a few meta-comments about how we got here, though, 
before we wrap up.



they are not visible via java.lang.reflect API. I assume that's just an 
omission.


While omissions do sometimes happen, it is usually not the best first 
theory in a situation like this.  More often than not, there is a good, 
but often non-obvious, explanation.


More importantly, though, the PR-first approach is not how we like to 
approach such a change, especially when it involves the behaviors of 
such fundamental APIs such as reflection, because it jumps over the most 
important steps:


 - What problem are we trying to solve?
 - Is it really a problem that needs to be solved?
 - Is it really a problem that needs to be solved in the JDK?
 - What solutions are there, besides the obvious one?
 - What are the pros, cons, and tradeoffs of the various solutions?
 - Of the proposed solution, what future downsides and risks could we 
imagine?


Going straight to the code of a specific solution is likely to be 
unsatisfying in both the short term (because its the wrong conversation) 
or the long term (because it might not be the right solution to the 
right problem.)  Instead, starting with a discussion of the problem, and 
of potential solutions and tradeoffs, is likely to yield a better result 
on both counts.


(As an example of the last one, suppose we committed this patch.  One 
could easily imagine some library later saying "must be run with 
-Xx:PreserveAllAnnotations" because that's what the author had to do to 
make it work.  But a behavior change like non-runtime annotations 
showing up unexpectedly in reflection could confuse existing code, which 
might not work as expected with the new behavior.  Which might then call 
for "can we please make the PreserveAllAnnotations flag more selective 
(e.g., per-class/module/package)" or calls for new flags or ... yuck.  
We try to avoid today's solutions becoming tomorrow's problems.  This is 
not an exact science, of course, but this is the sort of stewardship we 
strive for.)






On 6/1/2021 5:39 AM, Jaroslav Tulach wrote:

There doesn't seem to be much support for the complete changes in #4245. To get 
at least something useful from that endeavor I have extracted the test for 
existing behavior of `-XX:+PreserveAllAnnotations` and I am offering it in this 
pull request without any changes to the JVM behavior.

-

Commit messages:
  - Test long time existing behavior of -XX:+PreserveAllAnnotations

Changes: https://git.openjdk.java.net/jdk/pull/4280/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4280=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8267936
   Stats: 172 lines in 1 file changed: 172 ins; 0 del; 0 mod
   Patch: https://git.openjdk.java.net/jdk/pull/4280.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk pull/4280/head:pull/4280

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

Re: RFR: 8267587: Update java.util to use enhanced switch [v5]

[Brian Goetz]

On Tue, 25 May 2021 11:49:18 GMT, Tagir F. Valeev  wrote:

>> Inspired by PR#4088. Most of the changes are done automatically using 
>> IntelliJ IDEA refactoring. Some manual adjustments are also performed, 
>> including indentations, moving comments, extracting common cast out of 
>> switch expression branches, etc.
>> 
>> I also noticed that there are some switches having one branch only in 
>> JapaneseImperialCalendar.java. Probably it would be better to replace them 
>> with `if` statement?
>
> Tagir F. Valeev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   More vertical alignment

src/java.base/share/classes/java/util/JapaneseImperialCalendar.java line 1124:

> 1122: return Math.max(LEAST_MAX_VALUES[YEAR], d.getYear());
> 1123: }
> 1124: }

A switch with one element here is kind of weird.  I would turn this into 
"return switch (field) { ... }", with two cases, YEAR and default.

src/java.base/share/classes/java/util/JapaneseImperialCalendar.java line 1371:

> 1369: }
> 1370: }
> 1371: }

Pull value assignment out of switch?

-

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

Re: RFR: 8267587: Update java.util to use enhanced switch [v5]

[Brian Goetz]

On Tue, 25 May 2021 11:49:18 GMT, Tagir F. Valeev  wrote:

>> Inspired by PR#4088. Most of the changes are done automatically using 
>> IntelliJ IDEA refactoring. Some manual adjustments are also performed, 
>> including indentations, moving comments, extracting common cast out of 
>> switch expression branches, etc.
>> 
>> I also noticed that there are some switches having one branch only in 
>> JapaneseImperialCalendar.java. Probably it would be better to replace them 
>> with `if` statement?
>
> Tagir F. Valeev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   More vertical alignment

src/java.base/share/classes/java/util/Calendar.java line 1507:

> 1505: }
> 1506: case "japanese" -> cal = new 
> JapaneseImperialCalendar(zone, locale, true);
> 1507: default -> throw new IllegalArgumentException("unknown 
> calendar type: " + type);

Agree with Chris' suggestion here.

-

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

Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions

[Brian Goetz]

In the last hunk, you convert

case Collator.IDENTICAL: toAddTo.append('='); break;
case Collator.TERTIARY:  toAddTo.append(','); break;
case Collator.SECONDARY: toAddTo.append(';'); break;
case Collator.PRIMARY:   toAddTo.append('<'); break;
case RESET: toAddTo.append('&'); break;
case UNSET: toAddTo.append('?'); break;


to

case Collator.IDENTICAL -> toAddTo.append('=');
case Collator.TERTIARY  -> toAddTo.append(',');
case Collator.SECONDARY -> toAddTo.append(';');
case Collator.PRIMARY   -> toAddTo.append('<');
case RESET  -> toAddTo.append('&');
case UNSET  -> toAddTo.append('?');

But, you can go further, pulling the toAddTo.append() call out of the switch.  
This was one of the benefits we anticipated with expression switches; that it 
would expose more opportunities to push the conditional logic farther down 
towards the leaves.  I suspect there are other opportunities for this in this 
patch too.

> On May 25, 2021, at 7:57 AM, Patrick Concannon  
> wrote:
> 
> Hi,
> 
> Could someone please review my code for updating the code in the `java.io`, 
> `java.math`, and `java.text` packages to make use of the switch expressions?
> 
> Kind regards,
> Patrick
> 
> -
> 
> Commit messages:
> - 8267670: Update java.io, java.math, and java.text to use switch expressions
> 
> Changes: https://git.openjdk.java.net/jdk/pull/4182/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4182=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8267670
>  Stats: 328 lines in 11 files changed: 1 ins; 187 del; 140 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/4182.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182
> 
> PR: https://git.openjdk.java.net/jdk/pull/4182

Re: Builder pattern for Java records

[Brian Goetz]

As Rafaello mentioned, these seem well below the threshold for “convenience” 
methods in the JDK.  The bar here is pretty high.  This seems something better 
suited to Apache Commons?

Sent from my iPad

> On May 22, 2021, at 5:33 PM, Alberto Otero Rodríguez  
> wrote:
> 
> Understood Brian, great answer.
> 
> Could you please answer to my other proposal?:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077955.html
> 
> Regards,
> 
> Alberto.
> ________
> De: Brian Goetz 
> Enviado: sábado, 22 de mayo de 2021 22:35
> Para: Alberto Otero Rodríguez 
> Cc: core-libs-dev@openjdk.java.net 
> Asunto: Re: Builder pattern for Java records
> 
> There is no end to patterns of code that could be generated by tools, and for 
> each of them, one can imagine situations where they would be useful.  But in 
> addition to the other reply about how builders are really only called for 
> when there are a large number of _optional_ components, the premise of the 
> question makes some incorrect assumptions about records.
> 
> Records are not a “macro generator”; to think of records in terms of “the 
> compiler generates XYZ pattern of code”, while technically accurate, is 
> missing the main point.  They are a semantic feature, that happens to 
> dispense with some boilerplate.
> 
> Records are best understood as _nominal tuples_; they are the language’s 
> mechanism for representing product types.  Because Java is an OO language, we 
> can mediate the construction process to reject invalid states or normalize 
> state to its canonical form, and we derive the semantics for accessors, 
> equals, hashCode, etc, from the semantics of tuples.
> 
> If some project out there wants to have code generators for patterns that are 
> sometimes useful for records, that’s great — but that’s not where the 
> language should be focusing.
> 
>> On May 21, 2021, at 11:37 AM, Alberto Otero Rodríguez 
>>  wrote:
>> 
>> Hi, I have found this project on GitHub which creates a Builder for Java 
>> records:
>> https://github.com/Randgalt/record-builder
>> [https://opengraph.githubassets.com/a4e3a7b3c7b16b51e0854011fe4b031577bcc09919058baef412b03613295d20/Randgalt/record-builder]<https://github.com/Randgalt/record-builder>
>> GitHub - Randgalt/record-builder: Record builder generator for Java 
>> records<https://github.com/Randgalt/record-builder>
>> The target package for generation is the same as the package that contains 
>> the "Include" annotation. Use packagePattern to change this (see Javadoc for 
>> details).. Usage Maven. Add the dependency that contains the @RecordBuilder 
>> annotation. < dependency > < groupId >io.soabase.record-builder 
>> < artifactId >record-builder-core < version 
>> >set-version-here> github.com
>> 
>> And I was wondering if this could be made by default in all Java Records.
>> 
>> Technically a Builder is only used when creating the Object so I think is 
>> possible and, if Java creates it by default, would be less error prone than 
>> creating it manually.
>> 
>> Regards,
>> 
>> Alberto.
> 

Re: Builder pattern for Java records

[Brian Goetz]

There is no end to patterns of code that could be generated by tools, and for 
each of them, one can imagine situations where they would be useful.  But in 
addition to the other reply about how builders are really only called for when 
there are a large number of _optional_ components, the premise of the question 
makes some incorrect assumptions about records.

Records are not a “macro generator”; to think of records in terms of “the 
compiler generates XYZ pattern of code”, while technically accurate, is missing 
the main point.  They are a semantic feature, that happens to dispense with 
some boilerplate.  

Records are best understood as _nominal tuples_; they are the language’s 
mechanism for representing product types.  Because Java is an OO language, we 
can mediate the construction process to reject invalid states or normalize 
state to its canonical form, and we derive the semantics for accessors, equals, 
hashCode, etc, from the semantics of tuples.  

If some project out there wants to have code generators for patterns that are 
sometimes useful for records, that’s great — but that’s not where the language 
should be focusing.  

> On May 21, 2021, at 11:37 AM, Alberto Otero Rodríguez  
> wrote:
> 
> Hi, I have found this project on GitHub which creates a Builder for Java 
> records:
> https://github.com/Randgalt/record-builder
> [https://opengraph.githubassets.com/a4e3a7b3c7b16b51e0854011fe4b031577bcc09919058baef412b03613295d20/Randgalt/record-builder]
> GitHub - Randgalt/record-builder: Record builder generator for Java 
> records
> The target package for generation is the same as the package that contains 
> the "Include" annotation. Use packagePattern to change this (see Javadoc for 
> details).. Usage Maven. Add the dependency that contains the @RecordBuilder 
> annotation. < dependency > < groupId >io.soabase.record-builder < 
> artifactId >record-builder-core < version >set-version-here version ...
> github.com
> 
> And I was wondering if this could be made by default in all Java Records.
> 
> Technically a Builder is only used when creating the Object so I think is 
> possible and, if Java creates it by default, would be less error prone than 
> creating it manually.
> 
> Regards,
> 
> Alberto.

Re: [External] : Re: ReversibleCollection proposal

[Brian Goetz]

Has it ever been conceived to create an entire new API like how it was
done for Dates and Times? Obviously this would be a massive
undertaking, but it seems to me we've just about reached the limits of
how far we can stretch the current API.


"This code is a mess, we should throw it away and rewrite it"

   -- every developer

Don't confuse the volume of "I would rather do it this way" replies with 
the complexity of this issue; I think that's just the nature of the game 
("Being the biggest crime of the last 50 years, and everybody wanted to 
get in the newspaper story about it.")  Stuart's proposal is a measured, 
responsible, carefully-thought-through way to extend the framework we have.


In addition to "massive undertaking", and in addition to "if you think 
you can't get people to agree on something the size of a golf ball, try 
to get them to agree on something the size of Montana", there's another 
massive problem here: migration.  It's not just a matter of having a 
"better" Collections library; the collection interfaces we have 
(Collection, List, Map, Set) have found their way into the API of nearly 
every Java library.  Moving to Collections II would then force a 
migration on every one of those libraries (or consumer of those libraries.)

Re: [External] : Re: JEP draft: Scope Locals

[Brian Goetz]

Let's try again: why is that important?   What decisions would you
make differently if you had this information?  What benefit would
come from those decisions?

Threading is hard.


Gee, thanks MIke for pointing that out to me, obviously I'm new to the 
topic :)


Scoped variables are also hard to get right. Combining the two without 
an explicit api ignores the complexity that will arise as scoped 
variables are passed to other threads.


OK, now we're getting to the actual question; you obviously have some 
assumptions about the degree of sharing expected, that might be skipping 
a few steps (and then skipping some more steps by jumping to a 
solution).  So to ask again: what patterns of access / sharing are you 
assuming, what risks are you anticipating, etc?

Re: [External] : Re: JEP draft: Scope Locals

[Brian Goetz]

Let's back up a lot of steps; you're deep in proposing a solution
when you've not even explained what problem you think you're
solving.  So control question, which I hope will start to expose
the assumptions:

   Why do you think its important to know that a snapshot of a
variable has occurred?


From the JEP:
"In addition, a |Snapshot()| operation that captures the current set 
of inheritable scope locals is provided. This allows context 
information to be shared with asynchronous computations."


As the provider of the scoped variable, I'd certainly like to know 
that my scoped variable is being passed into a new scope.


Let's try again: why is that important?   What decisions would you make 
differently if you had this information?  What benefit would come from 
those decisions?

Re: [External] : Re: JEP draft: Scope Locals

[Brian Goetz]

How so? How do I know if a snapshot of my variable has occurred?


Let's back up a lot of steps; you're deep in proposing a solution when 
you've not even explained what problem you think you're solving.  So 
control question, which I hope will start to expose the assumptions:


   Why do you think its important to know that a snapshot of a variable 
has occurred?

Re: JEP draft: Scope Locals

[Brian Goetz]

The lifecycle is bounded by the duration of the Runnable passed to the 
`run()` method.


In the simple case, with no inheritance and no snapshotting:

    ScopedLocal.where(x, xExpr).run(r)

the lifecycle of the binding starts when we enter r, and ends when we 
return from r.


With snapshotting, the answer is the same, just less obviously so.  If I 
take a snapshot of a set of variables, we're not in a scope yet; we'll 
enter a new scope when we use that snapshot to run a task, possibly 
multiple times.


With inheritance, it is possible that the child thread can live longer 
than the lifetime of the task that spawned it; this is something we hope 
we can avoid through _structured concurrency_ -- that the inheritance is 
restricted to cases where we can prove the lifetime of the parent is 
longer than the lifetime of the child.  There's more work to be done here.


To be explicit, the two assumptions we need in order to avoid people 
shooting their feet are:


 - ScopeLocal values are effectively final "all the way down". We can't 
enforce this, but if users had to synchronize when accessing the state 
of scoped locals, then someone made a big mistake.  (This joins a very 
long list of mutability bugs we can't prevent but for which we have to 
communicate to users what they shouldn't be doing, not unlike "don't 
modify the source of the stream while traversing it.")


 - When inheriting, the parent must outlive the child.

When these assumptions hold, understanding the lifecycle is trivial.



On 5/14/2021 3:19 PM, Mike Rettig wrote:

I don't see a way to capture the lifecycle of the scoped variable. I think
for framework designers it's going to be important to know the life cycle
of the scoped variable especially with snapshotting and inheritance.  The
lack of a defined life cycle for thread locals is one of the many reasons
why it should be avoided. If scoped locals have a well defined and
comprehensive life cycle then it will make working with them much easier
(especially when many threads are involved).

interface ScopedVariableLifecycle {
ScopedVariable begin();
}

interface ScopedVariable {
T get();
void end();

Optional> createForChildScope();
Optional> createSnapshot();
}

static final ScopeLocal x = ScopeLocal.forType(MyType.class);

//most scoped variables will probably use the standard singleton
implementations.

ScopedLocal.where(x, new Singleton(new MyType()));

ScopedLocal.where(x, new InheritableSingleton(new MyType()));


//a custom lifecycle (e.g. database connection that is opened on first
access and closed at the end of the scope).

ScopedLocal.where(x, new MyScopedVariableLifecycle());


On Wed, May 12, 2021 at 10:15 AM Andrew Haley  wrote:


There's been considerable discussion about scope locals on the loom-dev
list,
and it's now time to open this to a wider audience. This subject is
important
because. although scope locals were motivated by the the needs of Loom,
they
have many potential applications outside that project.

The draft JEP is at

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

I've already received some very helpful suggestions for enhancements to
the API, and it'll take me a while to work through them all. In particular,
Paul Sandoz has suggested that I unify the classes Snapshot and Carrier,
and it will take me some time to understand the consequences of that.

In the meantime, please have a look at the JEP and comment here.


For reference, earlier discussions are at

https://mail.openjdk.java.net/pipermail/loom-dev/2021-March/002268.html
https://mail.openjdk.java.net/pipermail/loom-dev/2021-April/002287.html
https://mail.openjdk.java.net/pipermail/loom-dev/2021-May/002427.html

--
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: JEP draft: Scope Locals

[Brian Goetz]

Its simpler than you're making it.  Think of the motivating use cases 
for ThreadLocal, such as when a container calls into user code, and the 
user code calls back into the container, and we need to keep track of 
{transaction, security, etc} context, and it is impractical to pass them 
all through as parameters, or enumerate the possible callbacks.  The 
motivation here is the same; while the model is constrained, which 
eliminates some of the ways TLs are used today, these are "better TLs 
for better threads".  (If you believe that the motivation here is 
"poorly written libraries", then you believe that all libraries and 
frameworks that use TL are also poorly written.)


Where I think the JEP draft (the exposition, not the feature design) 
could be improved is to make the relationship to TL more clear.  IMO, 
this is a "more modern" TL design; it addresses the same primary use 
cases, without the unconstrained mutability of TLs.  The safety that 
comes from the immutability enables some new concurrent patterns (e.g., 
structured concurrency) and some nice optimizations.


On 5/14/2021 8:51 AM, Alan Snyder wrote:

The essence of the example seems to be that the callback is supporting multiple 
client contexts and when called needs to act relative to the specific client 
context.

The callback does not get a parameter identifying the client context because 
the library that calls it does not know anything about these client contexts.

Effectively, the callback uses the calling thread as a way to identify the 
client context.

That is not new. The ability to shadow/snapshot appears to be new, but it is 
still based on the thread as the ultimate context identifier.

Did I get it right?

If so, perhaps it would be less confusing if “Scope Locals” instead were called 
“Scoped Thread Locals”.






On May 14, 2021, at 1:28 AM, Andrew Haley  wrote:

On 5/13/21 4:59 PM, Alan Snyder wrote:



On May 13, 2021, at 2:03 AM, Andrew Haley  wrote:

On 5/12/21 8:12 PM, Alan Snyder wrote:

 From the motivation section:


So you want to invoke a method |X| in a library that later calls back into your 
code. In your callback you need some context, perhaps a transaction ID or some 
|File| instances. However, |X| provides no way to pass a reference through 
their code into your callback. What are you to do?

When I read this, my first thought was… pass a callback that is bound to the 
context.

I suppose the example you have in mind has a fixed callback.

Fixed? It's a callback, of some form.

What I meant was that the callback has a larger scope than the method call. 
Otherwise, you could bind the context to the callback object.

I don't know quite what you mean by "larger scope," but clearly, if you
can explicitly pass a callback created by a lambda, that's a more explicit
solution.


Is this a common practice? Doesn’t it point to a deficiency in the library API?

Not necessarily. For example, say you want to give access to a particular
resource (a log stream, say) to trusted callees. Nobody AFAIK passes a log
stream as an explicit argument through business logic because that's just
too much clutter.

That sounds interesting, but I’m not following how scope locals would be used 
to solve this problem.

In the outer scope you bind a logger scope local, and all transitive callees
can use that.

--
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: 8243287: Removal of Unsafe::defineAnonymousClass

[Brian Goetz]

Thanks for checking.  I thought I remembered something like this 
somewhere, but it may be that you already fixed such tests when you did 
hidden classes?  In any case, seems like we're all good now.


Cheers,
-Brian

On 5/13/2021 2:50 PM, Mandy Chung wrote:
I did a search on java.base and the tests on `String::indexOf` and 
`String::contains` of a slash and don't spot any such test.  The JDK 
has no use of VM anonymous class. If the test is trying to determine 
if it's lambda proxy class, it should be converted to call 
`Class::isHidden` but testing of the class name containing a slash is 
still valid (I haven't found any of such test though).


Mandy

On 5/11/21 6:20 AM, Brian Goetz wrote:

There may be some JDK code that checks for anon classes by comparing the name 
to see if it contains a slash, especially tests, but which don’t say 
“anonymous”.  Did you do a search for these idioms too, which are now dead 
tests?

Sent from my iPad


On May 11, 2021, at 8:59 AM, Harold Seigel  wrote:

Please review this large change to remove Unsafe::defineAnonymousClass().  The 
change removes dAC relevant code and changes a lot of tests.  Many of the 
changed tests need renaming.  I hope to do this in a follow up RFE.  Some of 
the tests were modified to use hidden classes, others were deleted because 
either similar hidden classes tests already exist or they tested dAC specific 
functionality, such as host classes.

This change was tested with Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and 
Mach5 tiers 3-7 on Linux x64.

Thanks, Harold

-

Commit messages:
- 8243287: Removal of Unsafe::defineAnonymousClass

Changes:https://git.openjdk.java.net/jdk/pull/3974/files
Webrev:https://webrevs.openjdk.java.net/?repo=jdk=3974=00
  Issue:https://bugs.openjdk.java.net/browse/JDK-8243287
  Stats: 3516 lines in 116 files changed: 69 ins; 3181 del; 266 mod
  Patch:https://git.openjdk.java.net/jdk/pull/3974.diff
  Fetch: git fetchhttps://git.openjdk.java.net/jdk  pull/3974/head:pull/3974

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

Re: JEP draft: Scope Locals

[Brian Goetz]

Scope locals have come together nicely.

I have some vague thoughts on the presentation of the JEP draft.  There 
are (at least) three intertwined things in the motivation:


 - ThreadLocal (and ITL especially) were always compromises, and with 
the advent of Loom, have become untenable -- but the need for implicit 
parameters has not gone away
 - Scoped locals, because of their effective finality and dynamic 
scoping, offer a programming model that is a better fit for virtual 
threads, but, even in the absence of virtual threads, are an enabler for 
structured concurrency
 - The programming model constraints enable a better-performing 
implementation


In reading the draft, these separate motivations seem somewhat tangled.  
All the words make sense, but a reader has a hard time coming away with 
a clear picture of "so, why did we do this exactly, besides that its 
cool and faster?"


A possible way to untangle this is:

 - the underlying use cases: various forms of implicit context 
(transaction context, implicit parameters, leaving breadcrumbs for your 
future self.)
 - the existing solution: thread locals.  ThreadLocals are effectively 
mutable per-thread globals.  The unconstrained mutability makes them 
hard to optimize.  ThreadLocals interact poorly with pooled threads.
 - Here comes Loom!  We no longer need to pool threads.  So, why are 
TLs not good enough?
 - The more constrained programming model of SLs enables two big new 
benefits:
   - structured concurrency, which is applicable to virtual and 
non-virtual threads alike

   - better optimization of inheritance and get operations







On 5/12/2021 10:42 AM, Andrew Haley wrote:

There's been considerable discussion about scope locals on the loom-dev list,
and it's now time to open this to a wider audience. This subject is important
because. although scope locals were motivated by the the needs of Loom, they
have many potential applications outside that project.

The draft JEP is at

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

I've already received some very helpful suggestions for enhancements to
the API, and it'll take me a while to work through them all. In particular,
Paul Sandoz has suggested that I unify the classes Snapshot and Carrier,
and it will take me some time to understand the consequences of that.

In the meantime, please have a look at the JEP and comment here.


For reference, earlier discussions are at

https://mail.openjdk.java.net/pipermail/loom-dev/2021-March/002268.html
https://mail.openjdk.java.net/pipermail/loom-dev/2021-April/002287.html
https://mail.openjdk.java.net/pipermail/loom-dev/2021-May/002427.html

Re: Draft JEP: Reimplement Core Reflection on Method Handles

[Brian Goetz]

Yes, please!

To add to the list of motivations/things to remove: the current 
implementation relies on the special `MagicAccessorImpl` to relax 
accessibility.  The notes in this class are frightening; getting rid of 
it would be a mercy.




On 5/11/2021 4:42 PM, Mandy Chung wrote:
This draft JEP is a proposal to reimplement core reflection on top of 
method handles:

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

Feedback is welcome.  The prototype is at [1].

Mandy
[1] https://github.com/mlchung/jdk/tree/reimplement-method-invoke

Re: RFR: 8243287: Removal of Unsafe::defineAnonymousClass

[Brian Goetz]

There may be some JDK code that checks for anon classes by comparing the name 
to see if it contains a slash, especially tests, but which don’t say 
“anonymous”.  Did you do a search for these idioms too, which are now dead 
tests?

Sent from my iPad

> On May 11, 2021, at 8:59 AM, Harold Seigel  wrote:
> 
> Please review this large change to remove Unsafe::defineAnonymousClass().  
> The change removes dAC relevant code and changes a lot of tests.  Many of the 
> changed tests need renaming.  I hope to do this in a follow up RFE.  Some of 
> the tests were modified to use hidden classes, others were deleted because 
> either similar hidden classes tests already exist or they tested dAC specific 
> functionality, such as host classes.
> 
> This change was tested with Mach5 tiers 1-2 on Linux, Mac OS, and Windows, 
> and Mach5 tiers 3-7 on Linux x64.
> 
> Thanks, Harold
> 
> -
> 
> Commit messages:
> - 8243287: Removal of Unsafe::defineAnonymousClass
> 
> Changes: https://git.openjdk.java.net/jdk/pull/3974/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3974=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8243287
>  Stats: 3516 lines in 116 files changed: 69 ins; 3181 del; 266 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/3974.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3974/head:pull/3974
> 
> PR: https://git.openjdk.java.net/jdk/pull/3974

Re: Would lambda expressions be meaningful annotation properties?

[Brian Goetz]

Yes, this has been considered at some length.  The summary verdict is:

 - Method references for static/unbound methods seem like reasonable 
constant literals to put in annotations, not unlike class literals.

 - Lambdas, on the other hand: absolutely, positively, not.

To actually get to method refs in annotations, there's a lot of tedious 
but not terribly hard work at a number of levels: JVMS, JLS, reflection, 
plus of course JVM runtime and javac.  Given the relatively broad cost, 
it hasn't been too high on the priority list, but it is a 
reasonable-to-have feature.


On 5/10/2021 5:40 PM, Rafael Winterhalter wrote:

Hello, I was wondering if there was ever any consideration of allowing
(stateless) lambda expressions as annotation members. As an example, this
would make the following lambda expression possible:

@interface MyAnnotation {
   Supplier value();
}

In many enterprise applications, this would be a very helpful addition and
replace a rather complicated pattern with little type-safety. For example,
in Spring, beans can be registered conditionally. Today, this requires
declaring a class that implements the Condition interface which then can be
supplied as an argument to the Conditional annotation:

public class MyCondition implements Condition {
   @Override
   public boolean matches(ConditionContext context, AnnotatedTypeMetadata
metadata) {
 return MyCode.isEnabled(context, metadata)
   }
}

which is then used in:

@Conditional(MyCondition.class) @Bean
public MyBean myBean() { ... }

By allowing stateless lambda expressions as annotation members, this could
be simplified to:

@Conditional(MyCode::isEnabled) @Bean
public MyBean myBean() { ... }

Another example where this would improve code readability a lot would be
bean validation frameworks, where custom constraints could be moved
directly to a property:

class MyBean {
   @Valid(value -> value != null && MyCode.validate(value))
   String property;
}

I observe such patterns regularly and therefore, I was curious if such a
feature was ever discussed or if this would be considered for a future
version. I understand that the intention of annotations is to provide
declarative metadata which can be processed also for unloaded code, but I
still feel like this would be a useful extension. The lambda expression
would be implicitly stateless, but of course they represent code that
requires class loading and would therefore be not necessarily meaningful to
annotation processors or other static code processing tools. If this would
be a feature to consider and only not a priority, I would be happy to
contribute, given I could get some help around the formalities of such a
process.

Thanks, Rafael

Re: Collection::getAny discussion

[Brian Goetz]

While I agree that we should be careful, let's not paint ourselves into 
an either/or strawman.  The choice is not "never add anything to 
Collection" vs "let's dump every silly idea that comes to anyone's mind 
into Collection"; it is, as always, going to involve tradeoffs between 
stability and evolution.


We cannot constrain ourselves so hard that we cannot evolve the core 
libraries because it might collide with someone else's subclass.  That's 
not reasonable, nor is that good for Java.


On the other hand, we must continue to exercise care in many dimensions 
when adding to libraries that are widely used and implemented -- which 
we already do (so much so, in fact, that people are often frustrated by 
our seeming conservatism.)








On 4/30/2021 4:02 PM, Donald Raab wrote:

There is a default method getAny defined on the RichIterable interface in 
Eclipse Collections. Adding a getAny with the same signature to Collection is 
bound to cause a break similar to CharSequence.isEmpty did with JDK 15 but 
possibly more extensive since RichIterable is the parent interface for all 
collection types in Eclipse Collections. Adding it with a different signature 
(returns Optional) could cause extensive damage.

https://www.eclipse.org/collections/javadoc/10.4.0/org/eclipse/collections/api/RichIterable.html#getAny()

I highly recommend we stop looking to add new zero-argument default methods to 
20+ year Collection interfaces and hope that we don’t break anything. There 
seems to be desire to breathe life into the old Collection interfaces. IMO, we 
should just start planning and focusing on a Collections 2.0 design.



On Apr 30, 2021, at 2:49 PM, Stuart Marks  wrote:

Hi Henri,

I've changed the subject of this thread because I think it's out of scope of the 
ReversibleCollection proposal. I don't mean to say that we can't talk about it, but I 
would like it to be decoupled from ReversibleCollection. I'm somewhat arbitrarily calling 
it "Collection::getAny" because something similar to that was mentioned by both 
Remi and Peter elsewhere in this thread. There is also a bunch of history in the bug 
database that contains related ideas.

Before we dive in, I want to explain why this is separate from ReversibleCollection. Most of the ideas, including yours, involve 
an implementation that does something like `iterator().next()`, in other words, getting the "first" element from an 
Iterator. Hey, there's getFirst() in ReversibleCollection, let's use that! No. The "first" element of an iterator is in 
general an arbitrary element, which is different from the "first" element in the structural ordering of elements 
provided by a ReversibleCollection. The "arbitrary" notion is captured by "getAny" so that's what I'll use as 
a working term. (Of course any actual API we might add would have a better name if we can find one.)

For a historical perspective, let's dig into the bug database and take a look 
at this bug:

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

This requests a method Collection.get(Object). This searches the collection for 
an element that equals() the argument and returns the element, or it returns 
null if the element isn't found. (Recall in those days it was impossible to add 
a method to an interface without breaking compatibility, so it also proposes 
various workarounds that are no longer necessary.)

There's a comment from Josh Bloch saying that Collection should have had a 
get() method as well as a no-arg remove() method. Well ok then! And he points 
to the then-new Queue interface that was delivered in Java SE 5. Queue adds the 
following methods that seem relevant to this discussion:

* E element() -- gets the head element, throws NSEE if empty
* E remove() -- removes and returns the head element, throws NSEE if empty

(It also adds peek() and poll(), which are similar to the above except they 
return null if empty.)

This is kind of odd, in that none of these methods satisfy what the bug's 
submitter was requesting, which is a one-arg get() method. Also, these methods 
are on Queue, which doesn't really help with collections in general.

You're asking for something that's somewhat different, which you called the "find 
the first element when there is only one" problem. Here, there's a precondition that 
the collection have a single element. (It's not clear to me what should happen if the 
collection has zero or more than one element.)

To throw a couple more variations into the mix, Guava has a couple Collectors 
(for streams) that do interesting things. The class is MoreCollectors:

https://guava.dev/releases/30.1.1-jre/api/docs/com/google/common/collect/MoreCollectors.html

and the collectors are:

* onlyElement -- if source has 1 element, returns it; throws NSEE if empty, IAE if 
> 1
* toOptional -- if source has 0 or 1 elements, returns an Optional; otherwise 
throws

These apply to streams, but I think you can see the applicability to Collection 
as well. In particular, your 

Re: New Collections interface - Sized

[Brian Goetz]

This is basically Spliterator, an iterator + a size, with the iterator is "push" instead 
of "pull" because it's more efficient.

In details a Spliterator is either
- an Iterable (with no SIZED characteristic)
- an Iterable + size (if SIZED and estimateSize() != Long.MAX_VALUE)
- an Iterable + comparator (if SORTED and comparator != null)
- an Iterable + split (if trySplit != null)

and all combinations of the above.


So, you're asking for Spliterable, no?   :)

Re: New Collections interface - Sized

[Brian Goetz]

Is there a compelling example of where this would be used by clients?

Here are some examples:
https://stackoverflow.com/questions/10988634/java-global-isempty-method
Without passing judgment on the sort of dynamically typed programs that 
need a method like this, or wondering what monstrosities such code uses 
to actually _get_ the data out of this weird ad-hoc union, note that 
this code becomes dramatically simpler with pattern matching:


    return switch (c) {
    case null -> true;
    case CharSequence cs -> cs.length() == 0;
    case Collection c -> c.isEmpty();
    case Object[] os -> os.length == 0;
    default -> { weird reflective thingy }
    }

Note also that a Sized abstraction only addresses two of these five 
cases -- CharSequence and Collection.  The others would still require 
such ad-hoc treatment.


And while it may seem like I have pattern matching on the brain, there's 
a point to bringing up pattern matching -- which is that real code often 
ends up dealing with ad-hoc unions, and a linguistic mechanism for 
abstracting computations over ad-hoc unions is often a better solution 
if you find yourself in ad-hoc land than trying to partially unify it.


But again, if you are treating these things as containers, then a Sized 
doesn't get you very far, because if you conclude the thing isn't empty, 
you're going to want to get stuff out, and Sized has no methods for 
that.  So presumably there's some companion to Sized for accessing 
elements by index:


    interface HasStuff extends Sized {
    T get(int index);
    }

And note that in order for HasStuff to be useful, it has to extend 
Sized, because, no intersection types.  Which suggests Sized is not the 
abstraction you are looking for.


And again, pattern matching:

    Object getElement(Object thingWithStuff, int index) {
    return switch (thingWithStuff) {
    case null -> null;
    case Object[] os -> os[index];
    case Collection c -> c.get(index);
    ... more if you can think of them ...
   };

Re: New Collections interface - Sized

[Brian Goetz]

This has come up before.  For example, during an early iteration of the 
Stream design, before parallelism entered the picture.  The first 
scrawled-on-a-napkin prototype for streams was based on Iterator, and it 
took about a minute to realize that we could do a much better job if we 
had a slightly broader interface to work with, essentially Iterator+Sized.


When you pull on this string, you end up with a lot of new interfaces, 
such as SizedIterator, SizedIterable, etc, in part because ... we have 
no intersection types.  Having lots of fine-grained interfaces for "has 
X" and "has Y" is nice from a "bucket of lego bricks" library-design 
perspective, but when the user goes to express "I need an aggregate that 
has sizes, iterators, and encounter order", you end up with code like:


    > void foo(X x) { ... }

and then you run into the wall of "but I can only use intersection types 
in these places in the language."  The idiom of having fine-grained 
mix-in-ish interfaces really wants a language with intersection types.


Additionally, I am having a hard time imagining how Sized would be 
usable by a client; no method will *take* a Sized (it's just not broad 
enough), and I can't immediately imagine what would even *return* a 
Sized.  If the type is not suitable for use by clients, then it serves 
only to organize the library itself, and that's a weaker motivation.


Is there a compelling example of where this would be used by clients?


On 4/23/2021 5:23 AM, Stephen Colebourne wrote:

Hi all,
While a discussion on ReversibleCollection is going on, I'd like to
raise the interface I've always found most missing from the framework
- Sized

  public interface Sized {
int size();
default boolean isEmpty() {
  return size() == 0;
}
default boolean isNotEmpty() {
  return !isEmpty();
}
}

This would be applied to Collection and Map, providing a missing
unification. Anything else that has a size could also be retrofitted,
such as String. Ideally arrays too, but that could come later. Note
that Iterable would not implement it.

WDYT?

Stephen

Re: ReversibleCollection proposal

[Brian Goetz]

Adding a REVERSIBLE characteristic to spliterators is easy enough, and 
as you say, many useful sources can easily provide an efficient reverse 
operation.  Filtering and mapping can preserve reversibility. The real 
question is what to do if someone calls reverse() on a non-reversible 
stream.  (Finite streams that are not easily reversible can still be 
reversed by draining the stream into a buffer, but that's likely 
inefficient; infinite streams have no chance.)  Should reverse() just 
throw when it encounters a source not designed for reversibility?  I 
don't particularly like the idea of throwing based on a dynamic source 
property, but trying to thread ReversibleStream through the static types 
is probably worse.


In the "drain" it case, the user can always simulate this manually: call 
toArray and get a reversed stream from that (since array-based streams 
are trivially reversible.)


I'm not particularly convinced of the value of folding from both 
directions.  The main value of foldr over foldl in Haskell is that 
because of laziness, foldr on infinite lists can work with 
short-circuiting combiners, whereas foldl cannot.  In strict languages, 
this benefit is not present, making the marginal value of foldr over 
foldl much smaller.


But, Stuart's proposal gives us much of this with less fuss.  IF a 
collection is reversible, you can just say


    c.reversed().stream()

and off you go, plus a similar method for arrays 
(Arrays.reversedStream).  Given that, what is the value of pushing this 
further into stream?


On 4/17/2021 7:49 AM, Tagir Valeev wrote:

Great proposal, thanks!

It has most functionality from my previous proposal, except the
ability to iterate LinkedHashSet starting from a specific element.
Well, probably we can skip this for now.

Some people really want to reverse Streams as well. E. g., the
corresponding StackOverflow question [1] has 171 upvotes and 200k+
views. Also, if the stream were reversible, it would be possible to
implement efficient foldRight/reduceRight operation. See the
discussion at [2]. It would be nice to explore the possibility of
extending Stream API to take into account the source reversibility.
Some existing ordered sources that have no underlying collection (e.g.
IntStream.range, or Arrays.stream) can be easily reversed as well. Map
and filter operations preserve reversibility and flatMap is reversible
if the supplied stream is reversible as well.

With best regards,
Tagir Valeev.

[1] https://stackoverflow.com/q/24010109/4856258
[2] https://bugs.openjdk.java.net/browse/JDK-8133680

On Sat, Apr 17, 2021 at 12:41 AM Stuart Marks  wrote:

This is a proposal to add a ReversibleCollection interface to the Collections
Framework. I'm looking for comments on overall design before I work on detailed
specifications and tests. Please send such comments as replies on this email 
thread.

Here's a link to a draft PR that contains the code diffs. It's prototype 
quality,
but it should be good enough to build and try out:

  https://github.com/openjdk/jdk/pull/3533

And here's a link to a class diagram showing the proposed additions:


https://cr.openjdk.java.net/~smarks/ReversibleCollection/ReversibleCollectionDiagram.pdf

Thanks,

s'marks


# Ordering and Reversibility


A long-standing concept that's been missing from collections is that of the
positioning, sequencing, or arrangement of elements as a structural property of 
a
collection. (This is sometimes called the "iteration order" of a collection.) 
For
example, a HashSet is not ordered, but a List is. This concept is mostly not
manifested in the collections API.

Iterating a collection produces elements one after another in *some* sequence. 
The
concept of "ordered" determines whether this sequence is defined or whether 
it's a
coincidence of implementation. What does "having an order" mean? It implies that
there is a first element and that each element has a successor. Since 
collections
have a finite number of elements, it further implies that there is a last 
element
that has no successor. However, it is difficult to discern whether a collection 
has
a defined order. HashSet generally iterates its elements in the same undefined
order, and you can't actually tell that it's not a defined order.

Streams do have a notion of ordering ("encounter order") and this is preserved,
where appropriate, through the stream pipeline. It's possible to detect this by
testing whether its spliterator has the ORDERED characteristic. Any collection 
with
a defined order will have a spliterator with this characteristic. However, this 
is
quite a roundabout way to determine whether a collection has a defined order.
Furthermore, knowing this doesn't enable any additional operations. It only 
provides
constraints on the stream's implementations (keeping the elements in order) and
provides stronger semantics for certain operations. For example, findFirst() on 
an
unordered stream is the same as findAny(), but actually finds 

Re: Garbage Free Check

[Brian Goetz]

Project Valhalla will allow Instant to be migrated to a primitive class, 
which would address your problem.


On 4/2/2021 7:47 PM, Ralph Goers wrote:

Log4j 2 supports the notion of a PreciseClock - one that can be initialized to 
something more precise than a millisecond. At the same time it also supports 
running with no heap allocations in certain circumstances. I am in the process 
of moving our master branch to require Java 11 as the minimum. In doing so I am 
encountering unit test errors while verifying that logging is garbage free. 
They all occur allocating an Instant.

The code we have simply does

public void init(MutableInstant mutableInstant) {
 Instant instant = java.time.Clock.systemUTC().instant();
 mutableInstant.initFromEpochSecond(instant.getEpochSecond(), 
instant.getNano());
}
In our previous tests we had thought the allocation was being eliminated due to 
escape analysis since the data is being extracted from the Instant and not 
passed along. However, after upgrading the Google test library and the JDK 
version it appears that is not the case.
Ideally we would really like something like

public void init(MutableInstant mutableInstant) {
java.time.Clock.systemUTC().initInstant(mutableInstant);
}

where Mutable instant would implement an interface that has the two set 
methods.The method would execute the same logic that is in the instant() method 
but instead of creating a new Instant it would call the set methods for the 
provided object.

This would allow us to either have the MutableInstants in ThreadLocals or some 
other mechanism to ensure they are thread safe and have no heap allocations. As 
it stands now I see no way to gain access to the higher precision clock without 
memory allocation.

Do you know of another way to do this? Am I missing something?

Ralph

Re: Proposal for Decimal64 and Decimal128 value-based classes

[Brian Goetz]

They'll find a natural home in JDBC, since SQL has a native decimal type.

On 3/30/2021 7:05 PM, Raffaello Giulietti wrote:


As far as I can tell, scientific computation will make use of binary 
floating point numbers for a long time. Decimal floating point numbers 
are still limited to biz and fin applications. 

Re: Proposal for Decimal64 and Decimal128 value-based classes

[Brian Goetz]

Overall, I'd be happy to see Decimal types that are aimed at "reasonable 
precision" in addition to the infinite precision that BD offers.  (After 
Valhalla, of course.)




On 3/29/2021 4:14 PM, Raffaello Giulietti wrote:

Hello,

I'm experimenting with an implementation of Decimal64 and Decimal128, 
two standard IEEE 754-2019 formats supporting, well, decimal floating 
point numbers of up to 16 resp 34 decimal digits.


While BigDecimal can simulate most finite number computations on these 
formats by passing MathContext.DECIMAL64 and MathContext.DECIMAL128, 
resp., to the arithmetic operations, this approach has some 
shortcomings wrt to the standard:
* Like the binary formats (float and double in Java), the standard 
mandates signed zeroes, signed infinities and NaNs for the decimal 
formats as well, while BigDecimal has neither.
* BigDecimal is not a final class, so is not (and can never be) 
value-based.
* BigDecimal needs support from BigInteger (another non final, non 
value-based class).


On the other hand, classes Decimal64 and Decimal128:
* Are value-based and can be declared to be primitive in a future 
version of OpenJDK, once Valhalla is completed.
* Require 12 and 20 bytes, resp, of storage in the Valhalla model of 
primitive objects and have no references to other identity or 
primitive objects.

* Support the signed zeroes, the infinities and the NaNs.
* Currently support the 4 fundamental operations (+ - * /) according 
to the standard and will support remainder, square root, comparisons, 
etc. and the conversions listed in the standard.
* Could be extended to support the (rather cumbersome) exceptions 
handling of the standard.
* There's no plan to support transcendental functions, as the aim is 
to have numerical classes for business applications, not scientific or 
engineering purposes.
* Are faster than the (incomplete) simulation with BigDecimal, 1x-5x 
in the current incarnations.



I would be glad to contribute the code to the OpenJDK provided there 
is a genuine interest from the community and a reviewer willing to 
sponsor my effort. (I guess the best candidate for this role would be 
Joe Darcy from what I can read in this mailing list.)



Greetings
Raffaello

Re: RFR: 8263358: Update java.lang to use instanceof pattern variable

[Brian Goetz]

These patches are obviously minimally correct.  However, for equals 
methods at least, I would take them one step further, from:


    if (!(o instanceof Key that)) return false;
    //noinspection StringEquality (guaranteed interned String(s))
    return name == that.name &&
   Arrays.equals(ptypes, that.ptypes);

to

    return (o instanceof Key that)
    && name == that.name
    && Arrays.equals(ptypes, that.ptypes);

The use of "if it's not, return false" is a holdover from when we 
couldn't express this as a single expression (which is almost always 
preferable), which means we had to fall back to control flow.  Now we 
don't have to.






On 3/10/2021 8:04 AM, Patrick Concannon wrote:

Hi,

Could someone please review my code for updating the code in the `java.lang` 
package to make use of the `instanceof` pattern variable?

Kind regards,
Patrick

-

Commit messages:
  - 8263358: Update java.lang to use instanceof pattern variable

Changes: https://git.openjdk.java.net/jdk/pull/2913/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2913=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8263358
   Stats: 62 lines in 18 files changed: 0 ins; 31 del; 31 mod
   Patch: https://git.openjdk.java.net/jdk/pull/2913.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk pull/2913/head:pull/2913

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

Re: RFR: 8263190: Update java.io, java.math, and java.text to use instanceof pattern variable

[Brian Goetz]

Looks good!  Glad to see the Amber features finding their way into the 
codebase.


On 3/8/2021 1:53 PM, Patrick Concannon wrote:

Hi,

Could someone please review my code for updating the code in the `java.io`, 
`java.math`, and `java.text` packages to make use of the `instanceof` pattern 
variable?

Kind regards,
Patrick

-

Commit messages:
  - 8263190: Update java.io, java.math, and java.text to use instanceof pattern 
variable

Changes: https://git.openjdk.java.net/jdk/pull/2879/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2879=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8263190
   Stats: 60 lines in 15 files changed: 0 ins; 27 del; 33 mod
   Patch: https://git.openjdk.java.net/jdk/pull/2879.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk pull/2879/head:pull/2879

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

Re: RFR: 8180352: Add Stream.toList() method

[Brian Goetz]

I have been reading previous threads, the original bug request, and exploring 
the javadoc and implementation of toList() on Stream in JDK 16. I don’t want to 
waste time rehashing previous discussions, but I want to understand and 
prioritize the motivation of this change, and propose what I believe is a safer 
alternative name for this method based on the current implementation: 
Stream.toUnmodifiableList().


Big +1 to "let's not rehash previous discussions, but help us understand 
the motivation."  Stewarding the core libraries is a complex task, and 
there are rarely hard-and-fast rules for doing the Right Thing.


Your question seems to have two main aspects:

 - Why this method, why not others, and why now
 - Why take such a strong anti-mutability position with this method

The desire for a Stream::toList method has a long history; when we first 
did streams, it was one of the first convenience methods to be 
"requested".  We resisted then, for good reasons, but we knew this saga 
was not over.


"Convenience" methods are a constant challenge in the JDK.  On the one 
hand, they are, well, convenient, and we want Java to be easy and 
pleasant to program in.  On the other, the number of potentially-useful 
imaginable convenience methods is infinite, and the widespread 
perception is that they are so easy, that all that is needed is for 
someone to propose the idea.  (The (admittedly soft) criteria we use for 
judging whether a convenience method meets the bar is an interesting 
one, which we can have separately.)


There are basically two stable points with respect to convenience 
methods in API design; zero tolerance, and "don't worry, be happy". In 
the former, the methods of an API are like a basis (ideally, an 
orthonormal one) of a vector space; the minimum number of API points 
from which you can derive all possible usages.  At the other extreme, 
every reasonable combination of methods gets its own special form of 
expression.  Of course, both are extremes (Stream::count and 
IntStream::sum are conveniences for reduce, and even Haskell's Monad has 
multiple ways to represent bind), but APIs tend to align themselves in 
one direction or another.  And, as the JDK APIs go, Streams treats 
sparsity and orthogonality as virtues to be striven for.


Eclipse Collections chooses a different (and also valid!) philosophy: 
completeness, and it walks the walk.  (Having 81 (template-generated) 
implementations of HashMap is proof.) Similarly, Tagir's StreamEx is an 
example of an extension to Stream that takes the other approach.  And 
both are great!  But also, they are not how the JDK rolls.  Which is 
fine; it's a big ecosystem, and there's room for multiple philosophies, 
and each can find its fans and detractors.


The calls for a convenience for Stream::toList have come pretty much 
continuously since we first resisted it (but, we knew even then that if 
we had a lifetime budget for just one convenience method, it would end 
up being toList.)  We knew then that there would be questions to ask 
about what the ideal dial settings would be for toList, and were not yet 
ready to confront the question, nor did we want to add fuel to the 
demands for more convenience methods ("No toSet?  Inconsistent!")


When an API is new, and all things are possible, we tend to be in 
"imagine everything we could put into it" mode, and streams was no 
different.  It is wise to resist this temptation -- and maybe even 
over-rotate in the other direction -- to allow for some time for the 
spirit of what you've built to make itself clear; even creators are not 
always immediately clear on the nuances of their creation.  So we tried 
hard to resist the calls for unnecessary methods, knowing that they 
could always be added, but not taken away, and also, allowing for the 
true gaps to emerge from usage.  (The first method to be added, 
takeWhile(), was the very opposite of a convenience; it represented a 
reasonable use case that the original design didn't support.)


So, why toList now?  Well, a number of reasons.  Collecting to a list is 
one of the most common terminal operations, so any small irritant (like 
a clumsy locution) adds up.  And, as has been pointed out, it can be 
more efficient if it is brought into the stream core rather than held at 
arm's length through Collector.  So if we're going to compromise our 
principles in one place, after thinking about it for a long time, this 
seemed a worthy candidate.  (And still, we hesitate, because we knew it 
would be firing the starting gun for the "But where's toSet?" arguments.)


So yes, there are lots of good reasons to continue to Just Say No to 
conveniences, but, there are also reasonable times to make exceptions -- 
especially when it is not purely about convenience. And, data suggests 
that toList is 5-10x more popular than the next most popular collector, 
so there's a clear argument to say that toList is pretty special, and we 
can stop there.



List 

Re: 'Find' method for Iterable

[Brian Goetz]

On 9/21/2020 4:08 AM, Michael Kuhlmann wrote:
But after thinking about it, I'm now convinced that it would be a bad 
idea. Because it extends the scope of this small, tiny Iterable 
interface to something bigger which it shouldn't be. 


This response captures the essence of the problem.  You may think you 
are asking for "just one more method on Iterable", but really, what you 
are asking for is to turn Iterable into something it is not.  Iterable 
exists not to be "smallest collection", but as the interface to the 
foreach-loop.  Yes, we could have chosen a different design center for 
Iterable (Eclipse Collections did, see RichIterable), but we didn't.  
Adding this method merely sends the signal that we want to extend 
Iterable to be more than it is, which opens the floodgate to requests 
(and eventually demands) for more methods.



I can ask about Iterable#forEach - is it there only because it was there to
begin with? Would it have been a bad idea to add one if we had streams
already?


While you can make an argument that forEach is excessive, the fact that 
you make this argument illustrates the essential problem with this 
proposal.  Your argument wrt forEach is "If that makes sense, then so 
does find."  If you pull on that string, then this method forms the 
basis of tomorrow's assumption: "You have forEach and find, it is 
unreasonable to not add ".


So, Iterable should stay simple.  (The other arguments against it on 
this thread, are also valid, but this is the most important one.)

Re: RFR: 8180352: Add Stream.toList() method

[Brian Goetz]

I wonder if we should not do the same with toList(), having toList() to be equivalent to 
collect(Collectors.toUnmodifiableList()) and toList(IntFunction>) 
allowing to use a null friendly List implementation like ArrayList.


If you pull on this string all the way, we end up with "let's just not 
do anything."


Adding conveniences only begets demands for more conveniences, and the 
endpoint there is a class with hundreds of trivial methods. This is why 
we held the line for so long.  Over that time, it emerged that the _one_ 
convenience method we could justify was toList(), since 
collect(toList()) is single most commonly used terminal op -- but only 
if we could justify stopping there.  Which I think we can, so we're 
stopping there.  (Maybe in five years we'll add another one.)

Re: RFR: 8180352: Add Stream.toList() method

[Brian Goetz]

As for nullity topic, I really welcome that the proposed toList() is
null-tolerant but it worth mentioning that existing terminal
operations like findFirst(), findAny(), min() and max() are already
null-hostile.
The min() and max() methods are not null hostile.  If you pass a 
null-friendly comparator (such as returned by the `nullsFirst()` and 
`nullsLast()` comparator combinators), nulls are not a problem.  The 
null hostility comes not from streams, but from the behaviors that 
clients pass to stream methods, whether they be min() or max() or 
map().  If the behaviors passed to _any_ stream method are null-hostile, 
then (usually) so will be that stream pipeline.  But this is something 
entirely under the control of the user; users should ensure that their 
domain and the behaviors that operate on that domain are consistent.


What we're talking about is what _streams_ should do.  Streams should 
not gratutiously narrow the domain.  Since `toList()` will be built into 
streams, we have to define this clearly, and there's no justification 
for making this method null-hostile.  The arguments made so far that 
toList() should somehow be null-hostile appear to be nothing more than 
weak wrappers around "I hate nulls, so let's make new methods 
null-hostile."


Remi say:

You know that you can not change the implementation of 
Collectors.toList(), and you also know that if there is a method 
Stream.toList(), people will replace the calls to 
.collect(Collectors.toList()) by a call to Stream.toList() for the 
very same reason but you want the semantics of Stream.toList() to be 
different from the semantics of Collectors.toList().


This is what I call a "for consistency" argument.  Now, we all know that 
consistency is a good starting point, but almost invariably, when 
someone says "you should do X because for consistency with Y", that "for 
consistency" argument turns out to be little more than a thin wrapper 
around "I prefer Y, and I found a precedent for it." Yes, people will be 
tempted to make this assumption -- at first. (And most of the time, that 
will be fine -- the most common thing people do after collecting to a 
list is iterate the list.)   But it is a complete inversion to say that 
the built-in method must be consistent with any given existing 
Collector, even if that collector has a similar name.  The built-in 
method should provide sensible default to-list behavior, and if you want 
_any other_ to-list behavior -- a mutable list, a different type of 
list, a null-hostile list, a list that drops prime-numbered elements, 
whatever -- you use the more general tool, which is collect(), which 
lets you do whatever you want, and comes with a variety of 
pre-configured options.  And this is the most sensible default behavior 
for a built-in to-list operation.


firstFirst/findAny are indeed sad corner cases, because we didn't have a 
good way of representing "maybe absent nullable value."  (If that case 
were more important, we might have done more work to support it.)  But I 
think it would be a poor move to try and extrapolate from this behavior; 
this behavior is merely a hostage to circumstance.

Re: RFR: 8180352: Add Stream.toList() method

[Brian Goetz]

There is no value in making users remember which stream methods are
null-hostile and which are null-tolerant; this is just more accidental
complexity.

I think that ship has sailed a long ago.
Some collectors are null hostile, some are not.
You can make a point that a Collector is technically not the Stream API per se.


Yes, and this is no mere "technical argument".  A Collector is a 
collection of behaviors, governed by its own spec.  The behaviors passed 
to stream operations, whether they be lambdas passed by the user 
(`.map(x -> x.hopeXIsntNull())`) or collector objects or comparators, 
are under the control of the user, and it is the user's responsibility 
to pass behaviors which are compatible with the data domain -- which the 
user knows and the streams implementation cannot know.



Because of that, i don't think we even have the choice of the semantics for 
Stream.toList(), it has to be the same as stream.collect(Collectors.toList()).


This doesn't remotely follow.  (And, if it did, I would likely not even 
support this RFE.)  The spec of Collectors::toList was crafted to 
disavow pretty much anything other than List-hood, in part in 
anticipation of this addition.




But it can not be immutable too, for the same reason.


Nope.  The spec of toList() very clearly says: no guarantees as to the 
type, mutability, serializability, etc etc of the returned list.  That 
doesn't mean that every method returning a List added to the JDK after 
Collectors::toList must similarly disavow all such guarantees.


(In fact, we would be entirely justified to change the behavior of 
Collectors::toList tomorrow, to match the proposed Stream::toList; the 
spec was crafted to preserve this option.  We probably won't, because 
there are too many programmers who refuse to read the specification, and 
have baked in the assumption that it returns an ArrayList, and this 
would likely just be picking a fight for little good reason, regardless 
of who is right.)


Please, can we stop having this same tired "I hate nulls, let's find a 
new way to punish people who like them" discussion for every single 
feature?

Re: RFR: 8180352: Add Stream.toList() method

[Brian Goetz]

This change introduces a new terminal operation on Stream. This looks like a
convenience method for Stream.collect(Collectors.toList()) or
Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
method directly on Stream enables it to do what can't easily by done by a
Collector. In particular, it allows the stream to deposit results directly into
a destination array (even in parallel) and have this array be wrapped in an
unmodifiable List without copying.

Hi Stuart,
I'm Okay with the idea of having a method toList() on Stream but really dislike 
the proposed semantics because tit is neither stream.collect(toList()) nor 
stream.collect(toUnmodifiableList()) but something in between.

It's true that a Stream support nulls, we want people to be able map() with a 
method that returns null and then filter out the nulls (even if using flatMap 
for this case is usually a better idea),
but it doesn't mean that all methods of the Stream interface has to support 
nulls, the original idea was more to allow nulls to flow in the stream because 
at some point they will be removed before being stored in a collection.


Uhm ... no and no.

It does mean that all methods of the stream interface have to support 
nulls.  Streams are null tolerant.  Because ...


The original idea was not "the nulls can be removed later."  The 
original idea was "Streams are plumbing, they pass values through 
pipelines, to user-specified lambdas, into arrays, etc, and the stream 
plumbing should not have an opinion on the values that are flowing 
through."   And this was the right choice.


There is no value in making users remember which stream methods are 
null-hostile and which are null-tolerant; this is just more accidental 
complexity.  And the root cause of that accidental complexity is the 
misguided belief that we can (and should) contain the consequences of 
nullity by making ad-hoc restrictions about nullity.   That doesn't 
work, and all it does is add more complexity and more ways for X to not 
interact with Y.


I understand the hatred for nulls, but it's a fantasy to think we can 
contain them with ad-hoc restrictions.   And the aggregate cost in 
complexity of all the ad-hoc decisions is pretty significant. Stuart 
made the right choice here.

Re: 8251989: Unified Hex formatting and parsing utility

[Brian Goetz]

Minor nit:

    Appendable foo(Appendable bar)

should be

     A foo(A bar)

That way, if you say

    hex.formatHex(aStringBuilder, bytes)
   .foo()

where foo() is a method on StringBuilder but not on Appendable, it will 
still work as expected.


On 9/28/2020 11:32 AM, Roger Riggs wrote:
After some reflection on the comments, suggestions, and other advice 
offered

the Hex format API has undergone a significant rewrite.

The improvements include:

 - Merged format and parse classes into a single class HexFormat that 
holds parameters and has methods for all conversions
 - Simplified static factories and added builder methods to create 
HexFormat copies with modified parameters.
 - Consistent naming of methods for conversion of byte arrays to 
formatted strings and back: formatHex and parseHex
 - Consistent naming of methods for conversion of primitive types: 
toHexDigits... and fromHexDigits...
 - Prefix and suffixes now apply to each formatted value, not the 
string as a whole
 - Using java.util.Appendable as a target for buffered conversions so 
output to Writers and PrintStreams
   like System.out are supported in addition to StringBuilder. 
(IOExceptions are converted to unchecked exceptions)


The HexFormat javadoc is at:
http://cr.openjdk.java.net/~rriggs/8251989-hex-formatter/java.base/java/util/HexFormat.html 



Thanks to all that commented and made suggestions on core-libs-dev and 
privately.


Please review and comment.  When the API has settled, I'll turn it 
into a PR and CSR.


Thanks, Roger

Re: RFR: JDK-8242451 : ensure semantics of non-capturing lambdas are preserved independent of execution mode

[Brian Goetz]

Should this patch be a workaround to existing releases rather than 
the main line?   As Brian mentions, lambda proxy class may become 
inline class in valhalla repo (Roger has a patch already).   The 
earlier fixing those programs the better.


Indeed if we know this is landing in this cycle in the main repo 
there's no point with my fix. Could you point me at the issue number 
or mail thread where this patch is being discussed?


Progress continues on Valhalla, and this is clearly where things are 
going, but integration is not imminent.


But, the main point here is: if the goal is to protect broken programs 
that make incorrect identity assumptions, said protection is inherently 
short-term; when Valhalla lands, all these programs will be broken, and 
there will be no interest in punishing the other 99.999% of code just to 
continue to coddle them.  So we should be clear that this is at most a 
short-term reprieve.

Re: RFR(M): 8249963: Make the zlib implementation selectively configurable at startup

[Brian Goetz]

So from your (and Alan's and Lance's) point of view does it still make 
sense to create a JEP for this enhancement and have a broader 
discussion about its usefulness on the "discuss" list? Or is your 
rejection definitive, no matter what the outcome of such a discussion 
will be?


I think it's perfectly fine to start a discussion.  I wouldn't start 
with a JEP draft, though, as that will have many of the same problems 
that the RFR did.  You'll want to start with what you see as the 
problem, why it is a problem, why it needs to be solved in the JDK, etc, 
and then proceed to evaluating solutions, as I outlined in my mail, and 
build consensus at each step.


Given the discussions we've had already, it seems highly unlikely that a 
JEP that enshrined _this exact solution_ would gain consensus.  But, it 
is entirely possible that an open-minded discussion of the problem and 
the many possible solutions, might lead to a solution that you could 
build some consensus around.  (Or it might not; that's OK too.)





I realize it sucks when you've done a lot of work and someone says
"but we don't want/need that"; this is why you socialize the idea
first.  Alan said in his first mail "I don't think the JDK should
be in the business of ..." -- that's a clear "this is not a
problem that needs to be solved in the JDK."  That's why we start
at the top of the list.


That's simply not true. I've developed this change to solve a real 
problem and while doing so I went through all the points you've listed 
above. I'm quite happy with it because we're using it productively 
with good results. I've just proposed it here because I thought it 
might be useful for others as well but if that's not the case it's 
perfectly fine for me.




I will answer this one as it speaks to a common mis-assumption about 
OpenJDK.  You obviously had some variant of the problem+solutions 
discussion internally on your team, and came to a solution that worked 
in your situation.  That's all good!  But, you should be prepared to 
start back at the beginning if you want to change "everybody's Java"; 
the set of considerations for a feature in a given deployment or even 
distribution are not the same as for the upstream, so the discussion 
might go differently.  (In particular, the answer to "is this a problem 
that needs to be solved in " may well vary with 
.)

Re: RFR(M): 8249963: Make the zlib implementation selectively configurable at startup

[Brian Goetz]

On 7/24/2020 7:48 AM, Volker Simonis wrote:

I think it's reasonable to ask if the JDK
really needs to support this complexity.

I can't see much complexity here.


Then I think we should start there.   Until you can see the complexity 
here that is obvious to Alan, Lance, myself, and others, then there is 
no point discussing the specific technical issue.


Every configuration knob or flag creates complexity, because it creates 
a transient environmental dependence -- yet another thing that can cause 
behavior to change even when running the same JDK on the same underlying 
platform.


I think you are looking at _your patch_ and saying "it's not that 
complex", but that's not the kind of complexity we're talking about.  
We're talking about the number of potential axes that can interact with 
each other to cause surprising or hard-to-debug behavior.  Can you 
honestly not imagine something going wrong with having Inflater use one 
zlib, Deflator another, and crc32 a third?



Until now I'm only hearing mostly high-level ,theoretical arguments
against the proposal rather than technical objections.


That's like when your mother tells you not to run with scissors, 
complaining that her concern is only theoretical, because not only do we 
not know anyone who has injured themselves running with scissors, but 
even if we did, they were not running with THIS pair of scissors."  But 
of course, your mother was right, and Alan (and all the others that 
reponded) are right too.  One of the most important roles of JDK 
stewards is pushing back on unnecessary complexity.  This is what Alan 
is doing.



The change itself is quite small


Small changes can still introduce complexity.  (It's one line of code to 
enforce that methods in Java only have an odd number of checked 
exceptions on tuesdays, but that would surely be new complexity.)



and it doesn't change any default
behaviour at all so I didn't think a JEP will be required.


Which means either it will not be tested, or we have to double the 
number of test modes.



I don't think we have to test all (or even various) zlib
which means customers using this will be running on an effectively 
untested configuration.  Does that seem wise?



Stepping back, we're in the classic trap where you've skipped over all 
the important discussion, and as a result we've gotten the obvious 
outcome, which is that we're talking about the wrong thing. Steps you 
should have run before getting tied to your solution, at a minimum, include:


 - Develop a clear and shared understanding about what the problem is
 - Develop consensus that it is a problem
 - Develop consensus that it is a problem that needs to be solved in 
the JDK

 - Brainstorm possible solutions, with tradeoffs, pros, and cons
 - Identify the best solution, and build consensus around that
 - Implement
 - Test
 - Review
 - Iterate

But you skipped straight to "Implement", and are now surprised that 
you're getting pushback against what should have been steps 1 or 2. You 
are trying to drive the discussion to "what changes do I have to make to 
have this patch accepted", but the conversation we should be having is 
"should we solve this problem at all" and then "if so, is this the right 
solution."  And it seems you're not finding anyone who is very compelled 
by either the problem or the solution.


I realize it sucks when you've done a lot of work and someone says "but 
we don't want/need that"; this is why you socialize the idea first.  
Alan said in his first mail "I don't think the JDK should be in the 
business of ..." -- that's a clear "this is not a problem that needs to 
be solved in the JDK."  That's why we start at the top of the list.

Re: RFR[8238286]: 'Add new flatMap stream operation that is more amenable to pushing’

[Brian Goetz]

On 6/24/2020 7:32 PM, Remi Forax wrote:

I don't really like the name "mapMulti", because flatMap can also be called 
mapMulti.


Public service announcement: remember just how frustrating it is for 
Patrick to have put in all this work and get only comments like "I don't 
like the name."


If you think everything about it is fantastic, except maybe the name, 
then you can say so, and then maybe make a comment on the name -- or not.


If you have anything more substantial to say, please don't comment on 
bikeshed colors like naming before you've made your more substantial 
comments -- or before everyone else has as well. Because all that will 
happen is we'll have 100 replies-to-replies about the name, and poor 
Patrick will get no substantive comments.


(Personally, I think the name is fine.  It is reminiscent of LinQ's 
"SelectMany", which is their flatMap.)

Re: RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

[Brian Goetz]

Consider a ClassValue for this, if there is not an obvious bit to be 
repurposed somewhere.


On 6/23/2020 12:30 PM, Chris Hegarty wrote:

I think it is worth considering caching the record-ness state of a j.l.Class, 
as I’m sure it will be widely used in third-party serialization libraries, as 
well as by Java Serialization.

Re: RFR: JDK-8242451 : ensure semantics of non-capturing lambdas are preserved independent of execution mode

[Brian Goetz]

On 6/23/2020 2:08 PM, Gilles Duboscq wrote:

In 8232806, a system property was introduce to disable eager initialization of 
the classes generated by the InnerClassLambdaMetafactory 
(`jdk.internal.lambda.disableEagerInitialization`).

However, when `disableEagerInitialization` is true, even for non-capturing 
lambdas, the capturing lambda path that uses a method handle to the constructor 
is taken.
This helps prevent eager initialization but also has the side effect of always 
returning a fresh instance of the lambda for every invocation instead of a 
singleton.
While this is allowed by the specs, this might change the behaviour of some 
(incorrect) programs that assume a singleton is used for non-capturing lambdas.


I need to reiterate that such programs are broken, and coddling such 
programs is likely to backfire in the long run.  When we switch to using 
inline classes for lambda proxies, for example, programs depending on 
the identity of lambda proxies likely will break, and tough luck on 
them.  The JLS was exceedingly clear from day 1 that the identity 
behavior of a lambda proxy is an implementation detail that should be 
_expected_ to change.


All that said, the change here seems fairly restrained, and I understand 
there are other motivations here.  I have a few concerns, which I think 
should be easily addressed:


 - I would prefer not to use Lookup.IMPL_LOOKUP.  We have been on a 
campaign to _reduce_ the use of IMPL_LOOKUP in code like this, both 
based on "principle of least privilege" as well as because we would like 
to move this code out of JLI into `java.lang.invoke` as soon as 
practical (this has been queued up behind some other changes).  Is there 
a translation strategy you can see that doesn't require IMPL_LOOKUP?  It 
seems reasonable to make the field accessible, since it is a cached 
instance that is freely dispensed to anyone who asks anyway.  The hidden 
class generation produces a Lookup for the class as part of the process; 
getting access to it might complicate the patch a little bit, but not 
doing so is creating fresh technical debt in the way of a refactor we 
have been trying to get to for a while.


 - I'm having a hard time reconciling this patch against the JDK tip, 
which incorporates the changes for Hidden Classes.  Was this patch done 
against an older version, or is this patch destined only for an update 
release?


 - Assuming we can resolve the above, I would like Mandy to review 
these as she has most recently been in this code (eliminating the 
dependence on `Unsafe::defineAnonymousClass` with the newly supported 
"hidden classes.")

Re: Type-parameterized complement to Object.equals(Object)

[Brian Goetz]

Without commenting on the merits of the proposal, let me point out that asking 
for a sponsor at this point is skipping many important steps.  

I would recommend you focus first on *building consensus* that (a) there is a 
problem, (b) you have identified the correct problem, (c) it is a problem that 
needs to be solved, (d) it is problem that needs to be solved in the JDK, (e) 
that you have evaluated the possible solutions and analyzed their strengths and 
weaknesses, and (f) that the correct solution has been identified.  (There are 
more steps, but you get the idea.)  

> On Apr 17, 2020, at 7:44 AM, dmytro sheyko  
> wrote:
> 
> I need a sponsor for following JDK change.

Re: Sponsor Request: 8241100: Make Boolean, Character, Byte, and Short implement Constable

[Brian Goetz]

Overall, the approach seems sound, and I like that you are introducing only one 
new bootstrap that is usable for a range of things.  A few comments and 
questions.  

1.  Not sure the explicit source type carries its weight, but whether it does 
or not is probably informed by whatever we do for (3) below.  

2.  Naming: “convert” seems kind of general; the name should suggest some sort 
of asType conversion.  asType is pretty rich, and will get richer in the 
future, so aligning with that is probably a good move.

3.  The spec on convert() needs to be fleshed out, as to exactly which set of 
conversions are supported.  “Exactly the same set as asType” is OK; “similar to 
asType” is kind of vague.  Similarly, the spec on parameters and exceptions 
could be fleshed out a bit too.  

I suspect you’ll want to iterate on (3) a few times before we have a spec that 
is solid enough that it says exactly what it does and doesn’t do.  




> On Mar 18, 2020, at 10:08 AM, Jorn Vernee  wrote:
> 
> Hi,
> 
> Can someone please sponsor this patch that makes Boolean, Character, Byte, 
> and Short implement Constable?
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8241100
> Webrev: http://cr.openjdk.java.net/~jvernee/8241100/webrev.00/
> 
> Having the other box types implement Constable makes them easier to use with 
> APIs that accept any Constable. Though I'm mostly interesting in boolean, for 
> which I'm currently falling back to "true" & "false" strings, but the 
> downside is that this also requires parsing the string again and having to 
> deal with random other strings.
> 
> This patch also adds the ConstantBootstraps::convert method that is used to 
> facilitate the conversion from int to (short|char|byte). This currently takes 
> a source type explicitly. In practice, it seems that Object can always be 
> used as a source type for the same behavior, but explicitly specifying source 
> and destination types seems more robust to me in case this behavior ever 
> changes, or we want to expand on the supported kinds of conversion. (for 
> instance it is currently not possible to convert from an int to a Long 
> directly, or from Short to Integer, but maybe those cases could be supported 
> in the future as well).
> 
> Testing: tier1-3 & downstream testing for my particular use case
> 
> Thanks,
> Jorn
> 

Re: Looking for Sponsor: JDK-8229959 Convert proxy class to use constant dynamic

[Brian Goetz]

OK, interesting.  Your interpretation seems right; that getting the MH 
is a big part of the expense of the LDC approach.  Caching simulates 
using Constant_MethodHandle_info, so that's a fair comparision.  It's 
not clear whether there's a performance difference getting the MH via 
lookup (as your approach does) vs CP resolution; measuring this is 
harder because once the CP is resolved, it's already cached.  But it 
does point at this approach being a tradeoff, where we're using a more 
expensive means to get our hands on the Method, in exchange for 
hopefully resolving fewer Methods overall (or pushing the cost to a less 
critical path.)


You can replace Class.forName("java.lang.Object") with Object.class 
(which turns into an LDC) to factor out the cost of that particular 
reflection.


It's unfortunate that InvocationHandler exposes the Method directly to 
the Proxy client, since a Method is a heavier object than we necessarily 
want, but we have limited ability to refactor here.


Another direction here would be as follows: generate code like the 
following:


    class Proxy$1 {
    private static final Class c1 = Object.class;

    private Method equals$bootstrap(Lookup, String, Class) {
    return c1.getMethod("equals", new Class[] { c1 });
    }

    public boolean equals(Object o) {
   return ... __LDC__[equals$bootstrap...] ...
    }
    }

In other words, use condy to delay evaluation, but don't go through the 
MH machinery.  There's a subtle proof we would have to do to prove this 
safe; we want reflective lookups to be done using the right access 
control context.  This is why I factored out the Class resolution and 
left that in the ; since the methods of a proxy are public (can 
only proxy interfaces), the interesting access control is done on the 
class lookup.  This would result in similar costs for looking up the 
Method, but delaying it to first use (still paying the one-by-one condy 
dispatch vs doing them all as a group in .)


On 11/23/2019 5:54 PM, Johannes Kuhn wrote:

On 11/23/2019 10:40 PM, Brian Goetz wrote:




Finally, we can benchmark the current approach against the LDC 
approach on a per-Method basis.  The LDC approach may well be doing 
more work per Method, so it's a tradeoff to determine whether 
deferring that work is a win.


By this last bit, I mean JMH'ing:

    Method m1() {
   return Class.forName("java.lang.Object").getMethod("equals", 
new Class[] { Class.forName("java.lang.Object") });

    }

vs

    Method m2() {
    return bootstrap(... constant bootstrap args for above ...)
    }


Thanks for the pointers, I did run one round on my machine - I don't 
have a dedicated one - so, maybe treat it with a fine grain of salt.
The test code and results can be found here: 
https://gist.github.com/DasBrain/501fd5ac1e2ade2e28347ec666299473


Benchmark  Mode  Cnt Score Error  Units
ProxyFindMethod.getMethod thrpt   25  1505594.515 ± 
42238.663  ops/s
ProxyFindMethod.lookupMethod  thrpt   25   760530.602 ± 
25074.003  ops/s
ProxyFindMethod.lookupMethodCachedMH  thrpt   25  4327814.284 ± 
103456.616  ops/s


I wrote three tests, the first one (getMethod) uses the 
Class.forName().getMethod(), the second uses lookup.findVirtual to get 
the MethodHandle every iteration, while the third one caches it in a 
static final field.


My interpretation: If the method has to be looked up (and the VM has 
to do that at least once), then the old getMethod call is faster.
I also suspect the Class.forName call to be optimized - on some old 
java versions, iirc pre-1.5, Foo.class was compiled into 
Class.forName("Foo"), so the JIT might do something there.

So, now I have to learn how to read assembly. Good.

Re: Looking for Sponsor: JDK-8229959 Convert proxy class to use constant dynamic

[Brian Goetz]

Finally, we can benchmark the current approach against the LDC 
approach on a per-Method basis.  The LDC approach may well be doing 
more work per Method, so it's a tradeoff to determine whether 
deferring that work is a win.


By this last bit, I mean JMH'ing:

    Method m1() {
   return Class.forName("java.lang.Object").getMethod("equals", new 
Class[] { Class.forName("java.lang.Object") });

    }

vs

    Method m2() {
    return bootstrap(... constant bootstrap args for above ...)
    }

Re: Looking for Sponsor: JDK-8229959 Convert proxy class to use constant dynamic

[Brian Goetz]

I just finished the first prototype, which can be found at 
https://gist.github.com/DasBrain/7766bedfbc8b76ba6a0ee66a54ba97ed - it 
contains a patch, the javap output of a generated proxy class with and 
without that patch, and the code I used to dump the proxy class. "make 
run-test-tier1" passes all JDK tests with the patch. I hope this will 
help the discussion a bit.


Good start.

https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-August/061923.html 



But there were never a real discussion about the benefits yet. It's 
good that we have one now.


As I am a novice JMH user, I don't trust myself to write good 
benchmarks that actually measure what I want to measure, not confirm 
what I would like it to be


Right, I think that's the next step.  You need to design an experiment 
to measure the cost of the  here.  I suspect the best way is to 
lift that work to an instance method (one that just overwrites the 
fields), and benchmark that, for various numbers of reflective calls.  
Then we can separately measure the cost of spinning the bytecode for a 
proxy, and of loading the class from a cached byte[].  This will give us 
a sense of the relative costs of the various pieces, to validate that 
reducing the  cost is relevant.


Finally, we can benchmark the current approach against the LDC approach 
on a per-Method basis.  The LDC approach may well be doing more work per 
Method, so it's a tradeoff to determine whether deferring that work is a 
win.

Re: Looking for Sponsor: JDK-8229959 Convert proxy class to use constant dynamic

[Brian Goetz]

Thanks for the examples.   

A few comments:

 - A reasonable place to consider putting the bootstrap is in Proxy itself.  I 
am not sure that ConstantBootstraps is the right place (or anywhere in JLI for 
that matter) unless Method is retrofitted to implement Constable, which is 
problematic given that Methods are not constants.  
 - In the department of “future things that might be  applied here” (Remi 
started it), this is a perfect application for lazy statics.  But we don’t have 
those yet.
 - The main benefit here is that it makes proxy creation faster.  However, 
given the amount of work that goes on at proxy creation (reflection, byte code 
spinning, class loading), its not clear how much of the cost is initializing 
the proxy class.  Some benchmarking to establish the cost benefit here would be 
good.  (If we are spinning the proxy at jlink time, then the benefit is 
probably greater.)

Overall, the translation approach seems sound but I would suggest that we 
clarify the benefits a little bit before embarking.  


> On Nov 22, 2019, at 4:44 PM, Johannes Kuhn  wrote:
> 
> Last mail got converted from HTML into plaintext, which dropped some 
> significant whit espace. So, again, doing it by hand this time.
> 
> On 22.11.2019 20:52, Brian Goetz wrote:
> > For those that are not intimately familiar with the mechanics of proxy 
> > class generation, it would be great to flesh this out with a code example, 
> > that shows the proposed old and new translation. This would make it easier 
> > for folks to evaluate the benefits and costs of the approach, and possibly 
> > suggest improvements.
> >
> > On 11/21/2019 10:23 PM, Johannes Kuhn wrote:
> >>
> >> * What benefits would such a change have?
> >> First, it allows us to drop the static final fields and the class 
> >> initializer for the generated proxy classes.
> >> Second, it allows the code (both generated and generator) to be more clear 
> >> by moving the essential parts together.
> >> Third, it might bring some performance improvement, because the 
> >> j.l.r.Method lookup is delayed to it's first use - which in some cases 
> >> might never happen.
> >
> Thanks for the suggestion. A proxy can be used to create an implementation of 
> one or more interfaces at run time.
> I just let Java 13 dump a generated Proxy and decompiled it using JD. This is 
> the result:
> 
> package com.sun.proxy;
> // imports ...;
> public final class $Proxy0 extends Proxy implements Runnable {
>   private static Method m1;
>   private static Method m3;
>   private static Method m2;
>   private static Method m0;
> 
>   public $Proxy0(InvocationHandler paramInvocationHandler) { 
> super(paramInvocationHandler); }
> 
>   public final boolean equals(Object paramObject) {
> try {
>   return ((Boolean)this.h.invoke(this, m1, new Object[] { paramObject 
> })).booleanValue();
> } catch (Error|RuntimeException error) {
>   throw error;
> } catch (Throwable throwable) {
>   throw new UndeclaredThrowableException(throwable);
> }
>   }
> 
>   public final void run() {
> try {
>   this.h.invoke(this, m3, null);
>   return;
> } catch (Error|RuntimeException error) {
>   throw error;
> } catch (Throwable throwable) {
>   throw new UndeclaredThrowableException(throwable);
> }
>   }
> 
>   public final String toString() {
> try {
>   return (String)this.h.invoke(this, m2, null);
> } catch (Error|RuntimeException error) {
>   throw error;
> } catch (Throwable throwable) {
>   throw new UndeclaredThrowableException(throwable);
> }
>   }
> 
>   public final int hashCode() {
> try {
>   return ((Integer)this.h.invoke(this, m0, null)).intValue();
> } catch (Error|RuntimeException error) {
>   throw error;
> } catch (Throwable throwable) {
>   throw new UndeclaredThrowableException(throwable);
> }
>   }
> 
>   static  {
> try {
>   m1 = Class.forName("java.lang.Object").getMethod("equals", new Class[] 
> { Class.forName("java.lang.Object") });
>   m3 = Class.forName("java.lang.Runnable").getMethod("run", new Class[0]);
>   m2 = Class.forName("java.lang.Object").getMethod("toString", new 
> Class[0]);
>   m0 = Class.forName("java.lang.Object").getMethod("hashCode", new 
> Class[0]);
>   return;
> } catch (NoSuchMethodException noSuchMethodException) {
>   throw new NoSuchMethodError(noSuchMethodException.getMessage());
> } catch (ClassNotFoundException classNotFoundException) {
>   throw n

Re: Looking for Sponsor: JDK-8229959 Convert proxy class to use constant dynamic

[Brian Goetz]

For those that are not intimately familiar with the mechanics of proxy 
class generation, it would be great to flesh this out with a code 
example, that shows the proposed old and new translation. This would 
make it easier for folks to evaluate the benefits and costs of the 
approach, and possibly suggest improvements.


On 11/21/2019 10:23 PM, Johannes Kuhn wrote:


* What benefits would such a change have?
First, it allows us to drop the static final fields and the class 
initializer for the generated proxy classes.
Second, it allows the code (both generated and generator) to be more 
clear by moving the essential parts together.
Third, it might bring some performance improvement, because the 
j.l.r.Method lookup is delayed to it's first use - which in some cases 
might never happen.

Re: RFR: JDK-8232806: The LambdaMetaFactory eagerly initializes generated lambdas

[Brian Goetz]

The patch seems suitably minimal to me to meet the goals and not cause 
difficulty for future evolution of LMF (for which we have some plans, 
once some other VM work finishes up.)   Essentially, the 
disable-eager-initialization case disables the caching optimization for 
non-capturing lambdas, and we then just revert to the main 
lambda-capture path, which makes this fairly low risk.




On 10/28/2019 2:10 PM, Vojin Jovanovic wrote:

Hi all,

This email proposes a change to the LambdaMetaFactory that allows to disable 
eager initialization (with Unsafe) of generated lambdas. This is needed by the 
GraalVM Native Image, as initialization of lambdas during AOT compilation 
breaks our heap snapshotting (via initialization of interfaces with default 
methods that are super-types of a lambda). We have started from the original 
proposal that stores lambda instances in the static fields

   
https://github.com/graalvm/labs-openjdk-11/commit/00b9ecd85dedd0411837eee33635dd83e8b7def8

and initializes them with Unsafe as an optimization


https://github.com/graalvm/labs-openjdk-11/commit/273e8590a7b57c0c10d9213ca9e0ba581e2817b8

After the discussion with Brian Goetz, we have trimmed down to the following 
change:

   
https://bugs.openjdk.java.net/secure/attachment/85247/lambda-disable-initialization.diff

The evolution of this change can be found at the issue:

  https://bugs.openjdk.java.net/browse/JDK-8232806
  
Cheers,

- Vojin

Re: [PATCH] Some improvements for java.lang.Class

[Brian Goetz]

I agree that getting rid of the append(concatenation) is a good move 
regardless; that’s just wasteful movement.  The rest of the patch seems 
harmless.  

As to applying the refactor more broadly, there’s a risk of recreating the 
“append(concatenation)” problem, where the concatenation is hidden inside the 
call to repeat().  A loop over append() is likely to be more performant than 
append(s.repeat()), even those the latter is more abstract and harder to get 
wrong.

If you’re willing to take the auto-refactor result and hand-edit it to 
eliminate the cases where we’d create new append(concat) situations, I’d be 
supportive of that as well.  

> On Mar 26, 2019, at 4:15 PM, Сергей Цыпанов  wrote:
> 
> Hello Peter,
> 
> I was unaware of mentioned detail (thank you a lot for pointing that out, it 
> made me read the whole JEP280 again) so I've rebenchmarked my changes 
> compiled with -XDstringConcat=inline.
> 
> 1) as of Class::getTypeName for Object[] it turned out, that String::repeat 
> performs slightly better:
> 
> Benchmark Mode  Cnt 
> Score Error   Units
> 
> getTypeName_patched   avgt   25
> 36,676 ±   3,559   ns/op
> getTypeName   avgt   25
> 39,083 ±   1,737   ns/op
> 
> getTypeName_patched:·gc.alloc.rate.norm   avgt   25   
> 152,000 ±   0,001B/op
> getTypeName:·gc.alloc.rate.norm   avgt   25   
> 152,000 ±   0,001B/op
> 
> 
> But for Object[][] situation is the opposite:
> 
>Mode  Cnt 
> Score Error   Units
> 
> getTypeName_patched avgt   50
> 47,045 ±   0,455   ns/op
> getTypeName avgt   50
> 40,536 ±   0,196   ns/op
> 
> getTypeName_patched:·gc.alloc.rate.norm avgt   50   
> 176,000 ±   0,001B/op
> getTypeName:·gc.alloc.rate.norm avgt   50   
> 152,000 ±   0,001B/op
> 
> 
> 2) as of Class::methodToString patched version clearly wins:
> 
> methodToString_1arg avgt   50   
> 238,476 ±   1,641   ns/op
> methodToString_1arg_patched avgt   50   
> 197,900 ±   5,824   ns/op
> 
> methodToString_1arg:·gc.alloc.rate.norm avgt   50  
> 1209,600 ±   6,401B/op
> methodToString_1arg_patched:·gc.alloc.rate.norm avgt   50   
> 936,000 ±   0,001B/op
> 
> methodToString_noArgs   avgt   50   
> 224,054 ±   1,840   ns/op
> methodToString_noArgs_patched   avgt   50
> 78,195 ±   0,418   ns/op
> 
> methodToString_noArgs:·gc.alloc.rate.norm   avgt   50  
> 1131,200 ±   7,839B/op
> methodToString_noArgs_patched:·gc.alloc.rate.norm   avgt   50   
> 408,000 ±   0,001B/op
> 
> 
> As Brian suggested I've separated the changes. The patch attached contains 
> only the changes for Class::methodToString. They are obviously helpful as 
> they rid concatenation as argument of StringBuilder::append call (obvious 
> antipattern to me) and skip Stream creation for empty array.
> 
> String::repeat obviously requires more investigation, it might turn out we 
> don't need it in java.base in majority of use cases or in all of them.
> 
> Regards,
> Sergei Tsypanov
> 
> 
> 
> 21.03.2019, 00:56, "Peter Levart" :
>> Hi Sergei,
>> 
>> I don't know if you are aware that the new invokedynamic based
>> translation of string concatenation expressions introduced in JDK 9
>> (http://openjdk.java.net/jeps/280) only applies to code outside
>> java.base module. java.base module is still compiled with old-style
>> StringBuilder based translation of string concatenation expressions
>> because of bootstraping issues.
>> 
>> If your benchmarks bellow are compiled with option
>> -XDstringConcat=inline to disable JEP280 translation (as java.base
>> module is), then it's OK. Else you are not benchmarking the right
>> translation of the code as you intend to change the code in java.base
>> module.
>> 
>> Regards, Peter
>> 
>> On 3/20/19 7:35 PM, Сергей Цыпанов wrote:
>>>  I had a brief conversation with Brian Goetz which has left off the mailing 
>>> list for some reason. Here's the text:
>>>  ---
>>>  Brian:
>>> 
>>>  These enhancements seem reasonable; these are exactly the cases that 
>>&

Re: RFR: 8221723: Avoid storing zero to String.hash

[Brian Goetz]

There's another reason to avoid these writes, besides CDS optimizations: 
do-nothing writes generate useless card mark activity.


On 4/1/2019 7:57 AM, Claes Redestad wrote:

Hi,

when a String has a calculated hash code value of 0, we recalculate and
store a 0 to the String.hash field every time (except for the empty
String, which is special cased). To make String objects more amenable to
storage in shared read-only memory, e.g., CDS archives, we should avoid
this redundant store.

Bug:    https://bugs.openjdk.java.net/browse/JDK-8221723
Webrev: http://cr.openjdk.java.net/~redestad/8221723/

Testing: tier1-3, no regression on existing and new StringHashCode
micros

/Claes

Re: RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation

[Brian Goetz]

+1 from me.



http://cr.openjdk.java.net/~dlsmith/8174222/webrev.00/




AbstractValidatingLMF
-

I see you renamed most of the fields and params.  Most of these are 
improvements, but it may be worth being more explicit in ambiguous 
cases.  For example, there are two method names, so perhaps instead of 
`methodName` it should be `interfaceMethodName` to make it clear this is 
the SAM method and not the implementation method.


I presume the SecurityException is something that was always thrown, and 
we are just documenting it now.  Should that be wrapped in a LCE instead?


(Similar for ICLMF)

LMF
---

Accidentally lost the {@code ...} surrounding MethodHandle in class 
description.

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

[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

[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

[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

[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

[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: JDK 12's String::transform vs F#'s |> operator

[Brian Goetz]

On 3/8/2019 3:47 PM, Lukas Eder wrote:

Hello,

I've recently learned about JDK 12's new String::transform method:
https://bugs.openjdk.java.net/browse/JDK-8203703

Obviously, this is useful. And obviously, this would be far more useful as
a general pattern than just something for String. E.g. why not also for any
Number subtype. For Boolean. For java.time types. Etc.


Yes, that's the plan -- to add this method more widely.

Yes, we could go bigger, and add the |> operator, but that's not in the 
current plan.

Re: Proposal to enhance Stream.collect

[Brian Goetz]

So, unfortunately I think this particular API evolution idiom is kind of 
questionable.  There is a new method, sizedSupplier, with a default that 
delegates to the unsized supplier.  But, that leaves implementors in a position 
where they don’t really know which one to implement, and the use of 
characteristics to hint at which one to call is kind of convoluted.  

You’re focused on the benefits — that in some cases, collection can be faster.  
That’s good.  But one of the big costs here is that an interfaces that was 
simple is now considerably less so.  That’s a cost; not convinced the two are 
in balance.

> On Mar 4, 2019, at 12:35 AM, August Nagro  wrote:
> 
> Hi everyone,
> 
> My implementation is at 
> https://github.com/AugustNagro/presize-collectors-bench 
>  and I have a webrev 
> here: http://august.nagro.us/presized-collectors/webrev/ 
> .
> 
> The changes that I made were:
> Add `default IntFunction sizedSupplier()` to Collector
> Add 2 new Collector.of helper methods taking a sized supplier
> Update the applicable collectors in Collectors to provide a sizedSupplier
> Have ReferencePipeline & ReduceOps call sizedSupplier when appropriate
> Some notes/questions I have:
> I considered adding Collector.Characteristics.PRESIZEABLE, but figured it was 
> not needed given that sizedSupplier should always delegate to supplier.
> Collector.sizedSupplier could be a LongFunction instead of IntFunction (since 
> Sink.begin provides long sizes), but every Collection type I've looked at 
> uses int for initialCapacity, so I think IntFunction makes more sense.
> StringJoiner should be updated to take an initalCapacity (should I commit 
> this change in the webrev, or leave for another enhancement?)
> What tests should I add for this enhancement? 
> There are some benchmarks in the github repo. I was initially surprised when 
> I saw that my bare-bones parallel stream did not have a throughput 
> improvement like the serial one, but after doing some debugging I think it is 
> from Collector.combiner dominating the runtime.
> 
> Regards,
> 
> August
> 
> On Thu, Feb 28, 2019 at 12:56 AM Tagir Valeev  > wrote:
> Hello! 
> 
> I wouldn't use presized HashSet, because you never know how many duplicates 
> are expected. What if the input stream has a million of elements, but only 
> two of them are distinct? Do you really want to allocate a hash table for 
> million elements in advance?
> 
> For toMap() without custom supplier and merger preallocation sounds 
> reasonable as in this case duplicating key is an exceptional case, so we may 
> expect a specific number of elements.
> 
> чт, 28 февр. 2019 г., 11:38 August Nagro augustna...@gmail.com 
> :
> Tagir,
> 
> Great to see the validating benchmarks.
> 
> > I think it's the best solution given the fact that very few collectors may 
> > benefit from the exact known size, and this benefit usually disappears when 
> > collectors are composed (e.g. using groupingBy: downstream toList() would 
> > not win anything if it provides a sizedSupplier).
> 
> I would like to see your benchmarks for that statement too. After all, 
> Hashmap, HashSet, etc all have presizing constructors, aren't those there for 
> a reason?
> 
> So I am still convinced that presizing is important for other collector 
> types, including custom collectors. Although I'm open to having my mind 
> changed.  
> 
>  I'm happy to contribute an implementation as well, I was hoping that this 
> this (smallish) patch would help me learn the OpenJdk process and make future 
> contributions easier!
> 
> - August
> 
> On Wed, Feb 27, 2019 at 1:06 AM Tagir Valeev  > wrote:
> Hello!
> 
> > A less intrusive API direction might be a version of Collector whose
> > supplier function took a size-estimate argument; this might even help in
> > parallel since it allows for intermediate results to start with a better
> > initial size guess.  (And this could be implemented as a default that
> > delegates to the existing supplier.)  Still, not really sure this
> > carries its weight.
> 
> It's interesting that such optimization is possible without API
> change, using the "hidden knowledge".
> While public API stays the same, the collect method implementation may
> check if custom
> non-public Collector implementation is supplied, and in this case may
> use the sized supplier.
> I think it's the best solution given the fact that very few collectors
> may benefit from the exact
> known size, and this benefit usually disappears when collectors are
> composed (e.g. using groupingBy:
> downstream toList() would not win anything if it provides a sizedSupplier).
> 
> I created a quick patch and launched a quick benchmark like this:
> return new SplittableRandom(0).ints(length, 0,
> 100).boxed().collect(Collectors.toList());
> For length = 

Re: Proposal to enhance Stream.collect

[Brian Goetz]

We did consider this problem when designing the Collector API; for 
example, it would have been nice if we could have a `toArray()` 
collector that had all the optimizations of `Stream::toArray`.


When we looked into it, we found a number of concerning details that 
caused us to turn back (many of which you've already identified), such 
as the difficulty of managing parallelism, the intrusion into the API, 
etc.  What we found is that all this additional complexity was basically 
in aid of only a few use cases -- such as collecting into a pre-sized 
ArrayList.  Where are the next hundred use cases for such a mechanism, 
that would justify this incremental API complexity?  We didn't see them, 
but maybe there are some.


A less intrusive API direction might be a version of Collector whose 
supplier function took a size-estimate argument; this might even help in 
parallel since it allows for intermediate results to start with a better 
initial size guess.  (And this could be implemented as a default that 
delegates to the existing supplier.)  Still, not really sure this 
carries its weight.



  The below code returns false, for example (is this
a bug?):

Stream.of(1,2,3).parallel().map(i ->
i+1).spliterator().hasCharacteristics(Spliterator.CONCURRENT)


Not a bug.   The `Stream::spliterator` method (along with `iterator`) is 
provided as an "escape hatch" for operations that need to get at the 
elements but which cannot be easily expressed using the Stream API.  
This method makes a good-faith attempt to propagate a reasonable set of 
characteristics (for a stream with no intermediate ops, it does delegate 
to the underlying source for its spliterator), but, given that `Stream` 
is not in fact a data structure, when there is nontrivial computation on 
the actual source, a relatively bare-bones spliterator is provided.


While this could probably be improved in specific cases, the return on 
effort (and risk) is likely to be low, because `Stream::spliterator` is 
already an infrequently used method, and it would only matter in a small 
fraction of those cases.  So you're in "corner case of a corner case" 
territory.



On 2/23/2019 5:27 PM, August Nagro wrote:

Calling Stream.collect(Collector) is a popular terminal stream operation.
But because the collect methods provide no detail of the stream's
characteristics, collectors are not as efficient as they could be.

For example, consider a non-parallel, sized stream that is to be collected
as a List. This is a very common case for streams with a Collection source.
Because of the stream characteristics, the Collector.supplier() could
initialize a list with initial size (since the merging function will never
be called), but the current implementation prevents this.

I should note that the characteristics important to collectors are those
defined by Spliterator, like: Spliterator::characteristics,
Spliterator::estimateSize, and Spliterator::getExactSizeIfKnown.

One way this enhancement could be implemented is by adding a method
Stream.collect(Function collectorBuilder).
ReadOnlySpliterator would implement the spliterator methods mentioned
above, and Spliterator would be made to implement this interface.

For example, here is a gist with what Collectors.toList could look like:
https://gist.github.com/AugustNagro/e66a0ddf7d47b4f11fec8760281bb538

ReadOnlySpliterator may need to be replaced with some stream specific
abstraction, however, since Stream.spliterator() does not return with the
correct characteristics. The below code returns false, for example (is this
a bug?):

Stream.of(1,2,3).parallel().map(i ->
i+1).spliterator().hasCharacteristics(Spliterator.CONCURRENT)

Looking forward to your thoughts,

- August Nagro

Re: RFE: add missing methods to Optional, OptionalInt, OptionalLong and OptionalDouble

[Brian Goetz]

This is a very nice patch, complete with spec and tests, and evidence of 
OCA.


But, before we talk about patches and code, let's step back and talk 
about stewardship.  With API proposals, we like to start with problems, 
rather than solutions: what problems we are concerned with, and are 
these are problems that need to be solved in the JDK?  Assuming we like 
the answers to the former, we can then move on to whether the proposed 
solution is the right one, and then to details like spec and code review.


We are currently trying to avoid adding anything to 
OptionalInt/Long/Double, because our current plan is to replace these 
with a specialized Optional once Valhalla delivers.  Any methods 
that differ from the base type (Optional) creates potential roadblocks 
to such a migration, so we'd prefer to leaves these classes be.


To the main question, though, whether Optional should have specialized 
map and flatMap methods ... I'm skeptical.  The goal was never to create 
parity between Optional and Stream, and my gut reaction is that the 
return on additional API surface area here is negative.  But, we can 
surely have a discussion on this.




On 2/16/2019 7:52 AM, Rob Spoor wrote:
I noticed that, while Stream and its primitive equivalents have 
multiple map and flapMap methods, Optional and its primitive 
equivalents were missing those. Since these can still be very useful, 
I wrote a patch to add the following methods:


* Optional: mapToInt, mapToLong, mapToDouble, flatMapToInt, 
flatMapToLong, flatMapToDouble

* OptionalInt: map, mapToObj, mapToLong, mapToDouble, flatMap
* OptionalLong: map, mapToObj, mapToInt, mapToDouble, flatMap
* OptionalDouble: map, mapToObj, mapToInt, mapToLong, flatMap

I did not add any more flatMap methods to OptionalInt, OptionalLong 
and OptionalDouble because these also don't exist on IntStream, 
LongStream and DoubleStream.



In addition, I also added the missing or method to OptionalInt, 
OptionalLong and OptionalDouble based on that of Optional.



My OCA has been processed on 2019-01-22.


Rob

Re: Collectors.converting

[Brian Goetz]

Arguably, it could have been exposed as a default method on Collector 
(Collector::andThen) -- but we avoided this approach out of concern that 
a generic methods in receiver position can interact with type inference 
in ways that confuse people.


On 1/30/2019 3:22 AM, Peter Levart wrote:



On 1/29/19 9:52 PM, Brian Goetz wrote:

How is this different from Collectors.collectingAndThen?


Hi Brian,

It is exactly the same!

I'm sorry, I haven discovered that method when I needed it. Perhaps I 
was looking for another name?


Regards, Peter



On 1/29/2019 3:30 PM, Peter Levart wrote:

Hi,

I wonder if there's any interest in adding a convenience factory 
method for a Collector to the standard repertoire which would look 
like the following:


    /**
 * Convert given {@link Collector} so that it applies an 
additional finisher function that
 * converts the result of given collector. The characteristics 
of returned collector is

 * the same as that of the given inner collector but without any
 * {@link 
java.util.stream.Collector.Characteristics#IDENTITY_FINISH}.

 *
 * @param collector   the inner collector to delegate 
collection to

 * @param resultConverter the additional result finisher function
 * @param  the type of input stream elements
 * @param  the type of intermediate aggregation
 * @param  the type of result of inner collector
 * @param     the type of final result
 * @return the converted collector
 */
    public static  Collector converting(
    Collector collector,
    Function resultConverter
    ) {
    return new Collector() {
    @Override
    public Supplier supplier() { return 
collector.supplier(); }


    @Override
    public BiConsumer accumulator() { return 
collector.accumulator(); }


    @Override
    public BinaryOperator combiner() { return 
collector.combiner(); }


    @Override
    public Function finisher() { return 
resultConverter.compose(collector.finisher()); }


    @Override
    public Set characteristics() {
    EnumSet characteristics = 
EnumSet.noneOf(Characteristics.class);

characteristics.addAll(collector.characteristics());
characteristics.remove(Characteristics.IDENTITY_FINISH);
    return Collections.unmodifiableSet(characteristics);
    }
    };
    }


The rationale is as follows... Composability of collectors allows 
doing things like:


    interface Measurement {
    long value();
    String type();
    }

    Stream measurements = 

    Map statsByType =
    measurements
    .collect(
    groupingBy(
    Measurement::type,
    summarizingLong(Measurement::value)
    )
    );

...but say I wanted the final result to be a map of avarage values 
by type and the values to be BigDecimal objects calculated with 
scale of 19. I can create an intermediate map like above and then 
transform it to new map, like this:


    static BigDecimal toBDAverage(LongSummaryStatistics stats) {
    return BigDecimal.valueOf(stats.getSum())
 .divide(
BigDecimal.valueOf(stats.getCount()),
 20, RoundingMode.HALF_EVEN);
    }

    Map averageByType =
    statsByType
    .entrySet()
    .stream()
    .collect(toMap(Map.Entry::getKey, e -> 
toBDAverage(e.getValue(;


...this is ugly as it requires intermediate result to be 
materialized as a HashMap. Wouldn't it be possible to collect the 
original stream to final result in one go?


With the above Collectors.converting factory method, it would:

    Map averageByType =
    measurements
    .collect(
    groupingBy(
    Measurement::type,
    converting(
summarizingLong(Measurement::value),
    stats -> toBDAverage(stats)
    )
    )
    );

We already have Collectors.mapping(Function, Collector) method that 
constructs Collector that maps elements to be collected by inner 
collector. Collectors.converting(Collectors, Function) would be a 
twin brother that constructs Collector that converts the collection 
result of the inner collector. Both are useful in compositions like 
above, but we only have the 1st...


What do you think?

Regards, Peter

Re: Collectors.converting

[Brian Goetz]

How is this different from Collectors.collectingAndThen?

On 1/29/2019 3:30 PM, Peter Levart wrote:

Hi,

I wonder if there's any interest in adding a convenience factory 
method for a Collector to the standard repertoire which would look 
like the following:


    /**
 * Convert given {@link Collector} so that it applies an 
additional finisher function that
 * converts the result of given collector. The characteristics of 
returned collector is

 * the same as that of the given inner collector but without any
 * {@link 
java.util.stream.Collector.Characteristics#IDENTITY_FINISH}.

 *
 * @param collector   the inner collector to delegate 
collection to

 * @param resultConverter the additional result finisher function
 * @param  the type of input stream elements
 * @param  the type of intermediate aggregation
 * @param  the type of result of inner collector
 * @param     the type of final result
 * @return the converted collector
 */
    public static  Collector converting(
    Collector collector,
    Function resultConverter
    ) {
    return new Collector() {
    @Override
    public Supplier supplier() { return 
collector.supplier(); }


    @Override
    public BiConsumer accumulator() { return 
collector.accumulator(); }


    @Override
    public BinaryOperator combiner() { return 
collector.combiner(); }


    @Override
    public Function finisher() { return 
resultConverter.compose(collector.finisher()); }


    @Override
    public Set characteristics() {
    EnumSet characteristics = 
EnumSet.noneOf(Characteristics.class);

characteristics.addAll(collector.characteristics());
characteristics.remove(Characteristics.IDENTITY_FINISH);
    return Collections.unmodifiableSet(characteristics);
    }
    };
    }


The rationale is as follows... Composability of collectors allows 
doing things like:


    interface Measurement {
    long value();
    String type();
    }

    Stream measurements = 

    Map statsByType =
    measurements
    .collect(
    groupingBy(
    Measurement::type,
    summarizingLong(Measurement::value)
    )
    );

...but say I wanted the final result to be a map of avarage values by 
type and the values to be BigDecimal objects calculated with scale of 
19. I can create an intermediate map like above and then transform it 
to new map, like this:


    static BigDecimal toBDAverage(LongSummaryStatistics stats) {
    return BigDecimal.valueOf(stats.getSum())
 .divide(
BigDecimal.valueOf(stats.getCount()),
 20, RoundingMode.HALF_EVEN);
    }

    Map averageByType =
    statsByType
    .entrySet()
    .stream()
    .collect(toMap(Map.Entry::getKey, e -> 
toBDAverage(e.getValue(;


...this is ugly as it requires intermediate result to be materialized 
as a HashMap. Wouldn't it be possible to collect the original stream 
to final result in one go?


With the above Collectors.converting factory method, it would:

    Map averageByType =
    measurements
    .collect(
    groupingBy(
    Measurement::type,
    converting(
    summarizingLong(Measurement::value),
    stats -> toBDAverage(stats)
    )
    )
    );

We already have Collectors.mapping(Function, Collector) method that 
constructs Collector that maps elements to be collected by inner 
collector. Collectors.converting(Collectors, Function) would be a twin 
brother that constructs Collector that converts the collection result 
of the inner collector. Both are useful in compositions like above, 
but we only have the 1st...


What do you think?

Regards, Peter

Re: RFR: 8207851 JEP Draft: Support ByteBuffer mapped over non-volatile memory

[Brian Goetz]

> Hi Alan/Brian,

My apologies for having missed this the first time around; messages to lists 
get swept into folders, and staying on top of many lists is not an exact 
science.  


> I believe I have addressed all outstanding comments on the JEP per se,
> including those made by Alan. Is it now possible for one of you to
> endorse the JEP so it can be submitted?

You don’t actually need my endorsement to Submit a JEP to be a Candidate; all 
you need is evidence that it’s been socialized on the right list (check) and a 
reviewer (check).  So you’re not blocked on submitting at all.  

That said, Candidate is actually supposed to be an early stage in the 
lifecycle; making something a candidate is essentially a statement that the 
feature is worth considering.  It is not any sort of commitment that the 
feature meets the high bar for inclusion, or is done — just that it is 
something that is worth continuing to work on in OpenJDK.  

Clearly, you’ve done a lot more than the minimum to meet the Candidate bar.  
Where I see this JEP at is you’ve iterated on the implementation and the design 
for a few rounds — and where we’re at now is we’re facing the stewardship 
questions of exactly what the right way to position this feature in the JDK is. 
 Based on the list discussion so far, it seems like we’ve not yet reached 
consensus on this, so these discussions can continue, whether the JEP is a 
Candidate or not.  

I concur with Alan and Stuart’s assessment that this is very much a “stop gap” 
measure.  (That’s not a value judgment, either the design of MBB, or your 
work.)  It is clear that MBBs are reaching the end of their design lifetime, 
and the set of mismatches to the problems people want to solve are growing, and 
the amount of investment needed to bring MBBs up to the task would be 
significant.  It seems likely a much better long-term solution is Panama, but I 
agree that Panama is not yet ready to replace MBB, and so some stop-gap is 
needed to prolong the usefulness of MBBs for long enough for a better solution 
to emerge.  However, Alan’s concern — which I share — is that by expanding the 
MBB API now, we open ourselves up to increased maintenance costs in the 
too-near future.  Before we settle on an API, we’re going to need to come to 
consensus on the right tradeoffs here, and I’m not convinced we’ve found this 
yet.  So we’ll keep talking.  

> Is there any other impediment to submitting the JEP and proceeding to
> code review?

There do not seem to be impediments to submitting the JEP, but I would call out 
the assumption that the next stop is code review, and then integration.  That 
seems to want to skip over the far more important “should we / how should we” 
discussions, which are still in progress.  

Cheers,
-Brian

Re: RFR: JDK-8215510: j.l.c.ClassDesc is accepting descriptors not allowed by the spec

[Brian Goetz]

+1 from me.

On 1/7/2019 1:17 PM, Vicente Romero wrote:
I have updated the webrev after additional feedback from the TCK 
tester please check last version at [1]


Thanks,
Vicente

[1] http://cr.openjdk.java.net/~vromero/8215510/webrev.01


On 1/3/19 12:21 PM, Vicente Romero wrote:
Please review the fix for bug [1] at [2]. Basically the constants API 
introduced as part of JEP-334 [3] were accepting descriptors and 
class names not allowed by the spec. This implementation fixes 
several issues found by TCK tests on JEP-334,


Thanks,
Vicente

[1] https://bugs.openjdk.java.net/browse/JDK-8215510
[2] http://cr.openjdk.java.net/~vromero/8215510/webrev.00/
[3] https://bugs.openjdk.java.net/browse/JDK-8203252

Re: Feature suggestion: Provide efficient way for nullable value lookup in Map

[Brian Goetz]

Usually when we add defaults to Collection interfaces, we provide 
efficient implementations for the several dozen primary JDK 
implementations.


On 1/8/2019 11:16 AM, 
some-java-user-99206970363698485...@vodafonemail.de wrote:

Would this method then be useful enough if the default implementation is that 
inefficient (in case
you only want to get a nullable value)?


Brian Goetz  hat am 8. Januar 2019 um 16:57 geschrieben:


Here's a default implementation that returns the actual key:

     default Optional> getEntry(K key) {
     for (Map.Entry e : entrySet) {
     if (Objects.equals(key, e.getKey())
     return Optional.of(e);
     }
     return Optional.empty();
     }

On 1/8/2019 10:50 AM,
some-java-user-99206970363698485...@vodafonemail.de wrote:

Yes it is now possible to implement the methods `getKey(Object key)` and 
`getEntry(Object key)`
requested by JDK-6552529 as default methods, however both would have to include 
that
"The default implementation returns the provided key. Overriding 
implementations may return the
actual key used by the map."
In this case it is questionable how useful the methods actually are, e.g.
Two key objects k1 and k2 with `k1.equals(k2)`
Two objects m1 and m2 of different map implementations both containing k1.
Then the following is possible:
m1.getKey(k2) == k2 // true
m2.getKey(k2) == k2 // false


Brian Goetz  hat am 7. Januar 2019 um 00:54 geschrieben:


FYI, the comment about compatibility was obsoleted by the addition of
default methods in Java 8.

Re: Feature suggestion: Provide efficient way for nullable value lookup in Map

[Brian Goetz]

Here's a default implementation that returns the actual key:

   default Optional> getEntry(K key) {
   for (Map.Entry e : entrySet) {
   if (Objects.equals(key, e.getKey())
   return Optional.of(e);
   }
   return Optional.empty();
   }

On 1/8/2019 10:50 AM, 
some-java-user-99206970363698485...@vodafonemail.de wrote:

Yes it is now possible to implement the methods `getKey(Object key)` and 
`getEntry(Object key)`
requested by JDK-6552529 as default methods, however both would have to include 
that
"The default implementation returns the provided key. Overriding 
implementations may return the
actual key used by the map."
In this case it is questionable how useful the methods actually are, e.g.
   Two key objects k1 and k2 with `k1.equals(k2)`
   Two objects m1 and m2 of different map implementations both containing k1.
   Then the following is possible:
   m1.getKey(k2) == k2 // true
   m2.getKey(k2) == k2 // false


Brian Goetz  hat am 7. Januar 2019 um 00:54 geschrieben:


FYI, the comment about compatibility was obsoleted by the addition of
default methods in Java 8.

Re: Feature suggestion: Add static equals methods to Float and Double

[Brian Goetz]

Followers of Project Valhalla will see that this issue comes up when 
defining equality on value types.  The relation you are looking for is 
being called "substitutible"; it asks whether there is any way to 
distinguish between two values.  For primitives other than float/double, 
this coincides with `==`, and similarly for references.


An `isSubstitutible()` API point will likely emerge from Valhalla.

On 1/6/2019 2:01 PM, some-java-user-99206970363698485...@vodafonemail.de 
wrote:

My main goal was to provide a way where NaN is equal to NaN and I went with the 
behavior of the
respective wrapper classes, Float and Double, which consider -0.0 and +0.0 as 
not equal.
The static method could be called "exactlyEquals" if that is better.

It might also depend on the usecase whether you want -0.0 being equal to +0.0. 
Maybe an additional
method would be required for the case you are describing, e.g. "valueEquals" 
(though that can be
ambiguous).


Hans Boehm  hat am 6. Januar 2019 um 19:40 geschrieben:
  


What's the motivation for making it easier to spell a comparison method that 
tells me that -1.0*0.0 is not equal to 1.0*0.0? I would have thought that 
making this somewhat hard to write is a feature?
  
I agree that there are cases in which this method is useful. But they all seem esoteric enough to me that it's not unreasonable for users to have enough expertise to write the method. Or at least the method should have a name that makes the unusual behavior explicit.

Re: Feature suggestion: Provide efficient way for nullable value lookup in Map

[Brian Goetz]

FYI, the comment about compatibility was obsoleted by the addition of 
default methods in Java 8.


On 1/4/2019 6:34 PM, some-java-user-99206970363698485...@vodafonemail.de 
wrote:

The methods currently provided by the Map interface 
(https://docs.oracle.com/javase/8/docs/api/java/util/Map.html) do not provide 
an efficient way to look up nullable values. This problem would be solved if 
JDK-6552529 was implemented, but that will likely not be happening since the 
interface cannot be changed without breaking compatibility.

Ways to currently look up nullable values are:

* Combined ussage of `get()` and `containsKey()` -> inefficient
* Usage of `getOrDefault()` with private no-value constant, e.g.:
private static final Object NO_VALUE = new Object();
// ...
V value = map.getOrDefault(key, (V) NO_VALUE);
if (value == NO_VALUE) {
  // No value
}
else {
  // Value exists
}

Both solutions are not really that great, therefore it would be good to provide an 
easier and (if overridden) more efficient method: default OptionalNullable 
getOptional(Object key) {
 V value = get(key);

 if (value != null || containsKey(key)) {

 return OptionalNullable.of(value);
 }
 else {
 return OptionalNullable.empty();
 }
}

Where `OptionalNullable` is a new class similar to `Optional` except that null 
is considered a value.

Maybe a `void ifExists(K key, Consumer consumer)`, which makes the consumer 
consume the value if it exists (= nullable), would be good as well to avoid creation 
of `OptionalNullable` objects, though that method might (at least for performance 
reasons) become obsolute when `OptionalNullable` becomes a value type.

Re: enhanced for loop with multiple iteration variables

[Brian Goetz]

For Map, you can do:

    for (Map.Entry e : map.entrySet()) { ... }

and you're already there.



On 12/19/2018 9:54 AM, Alan Snyder wrote:

Has any consideration been given to supporting iterators that provide more than 
one iteration variable in the enhanced for loop?

Obvious uses would be maps (keys and values) and lists (indexes and values).

I have in mind keeping the syntactic sugar approach by using one or more 
extensions of the Iterator/Iterable interfaces, such as, for example:

interface Iterator2 extends Iterator {
   E2 get2();
}

with the extra methods providing the values for the extra variables (associated 
with the previous call to next).

Extending interfaces is not required, but it makes the trailing variables 
optional, which might be useful. For example, the same iterator could provide 
values or values and keys.

The fact that this approach only works for a fixed set of numbers of variables 
does not bother me unduly.

   Alan

Re: RFR JDK-8215300: additional changes to constants API

[Brian Goetz]

+1 from me.

Appears that it covers all the issues that were raised in the CSR and by JCK 
since integration.

> On Dec 13, 2018, at 10:14 AM, Vicente Romero  
> wrote:
> 
> Hi all,
> 
> I have provided another iteration to the webrev at [1]. This one includes 
> additional changes to make sure that no array descriptor with more than 255 
> dimensions is created.
> 
> Thanks,
> Vicente
> 
> http://cr.openjdk.java.net/~vromero/8215300/webrev.01/
> 
> 
> On 12/12/18 12:02 PM, Vicente Romero wrote:
>> Hi,
>> 
>> Please review some final changes to the constants API. The changes should 
>> make the API clearer and more precise and were recommended in the CSR [3]
>> 
>> Thanks,
>> Vicente
>> 
>> [1] (jira issue) https://bugs.openjdk.java.net/browse/JDK-8215300
>> [2] (webrev) http://cr.openjdk.java.net/~vromero/8215300/webrev.00/
>> [3] https://bugs.openjdk.java.net/browse/JDK-8202031
> 

Re: Add convenience collect methods to the Stream interface

[Brian Goetz]

You say it jokingly, but we've explored this.  (The exact way you phrase 
it (the supertype specified to throw if called more than once) means 
that Iterable can't extend IterableOnce, because then Iterable does not 
conform to the superclass contract, but there are other ways to stack 
it.)  It's not completely unworkable, but as you've observed, it's not 
so clean.


In the JSR-335 EG, we also explored (and rejected, because it felt like 
the library tail wagging the language dog) the route of adding language 
support for more kinds of things on the RHS of the for-each loop.


(Zooming out: the main reason this is irritating is the limitations of 
what you can do in a method like .forEach() vs the foreach loop -- 
exception transparency, nonlocal return, up-scope local mutation.  
Though it is unlikely that we'll have great solutions for these all that 
soon, so its reasonable to consider library-based solutions in the 
meantime.)


On 12/11/2018 7:55 AM, Rachel Greenham wrote:
Although I do understand the reasoning (non-repeatability of streams), 
Stream not implementing Iterable is a minor but frequent annoyance for 
this precise reason. Using forEach() instead isn't always an option.


Maybe Iterable can have a superinterface IterableOnce with the methods 
moved up to there, and iterator() specified to throw 
IllegalStateException if called twice, and which enhanced-for 
recognises, then Stream can implement that? (Ducks and runs...)


Or enhanced-for can also take Supplier, so we can do 'for 
(Thing e : stream::iterator)' instead of 'for (Thing e : 
(Iterable)stream::iterator)'


(runs faster...)