date:20210430

> jpackage should specify architecture for produced PKG files via 
> hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable 
> on x64 without specifying hostArchitectures which is not correct and if 
> install on arm Mac it will request Rosetta 2. With proposed fix by setting 
> hostArchitectures="x86_x64" if installer contains x64 binaries, it will be 
> installable on x64 Mac and will require Rosetta 2 on arm Mac. 
> hostArchitectures will be set to arm64 if installer contain aarch64 binaries 
> and will gave error when run on x64 Mac and will be installable on arm Mac 
> without triggering installation of Rosetta 2.

Alexander Matveev has updated the pull request incrementally with one 
additional commit since the last revision:

  8266179: [macos] jpackage should specify architecture for produced pkg files 
[v2]

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3807/files
  - new: https://git.openjdk.java.net/jdk/pull/3807/files/1937e9e1..11d9a2cf

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3807=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3807=00-01

  Stats: 96 lines in 2 files changed: 94 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3807.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3807/head:pull/3807

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

Re: RFR: 8266179: [macos] jpackage should specify architecture for produced pkg files

On Fri, 30 Apr 2021 04:22:37 GMT, Alexander Matveev  
wrote:

> jpackage should specify architecture for produced PKG files via 
> hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable 
> on x64 without specifying hostArchitectures which is not correct and if 
> install on arm Mac it will request Rosetta 2. With proposed fix by setting 
> hostArchitectures="x86_x64" if installer contains x64 binaries, it will be 
> installable on x64 Mac and will require Rosetta 2 on arm Mac. 
> hostArchitectures will be set to arm64 if installer contain aarch64 binaries 
> and will gave error when run on x64 Mac and will be installable on arm Mac 
> without triggering installation of Rosetta 2.

Added HostArchPkgTest test to verify hostArchitectures attribute.

-

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

Re: New convenience methods on Stream

2021-04-30 Thread Tagir Valeev

Hello!

1. toCollection looks too specific to be added to JDK. Essentially,
it's a shortcut for toCollection constructor and unlike toList, it
cannot add many optimizations there. So we basically save several
characters and nothing more. And toCollection collector is orders of
magnitude less used than toList. I think that it's even less used than
joining or groupingBy, so why not providing these shortcuts first?
It's always about where to draw a line.

2. to() sounds quite specific, as it expects that the target object is
ready to accept an array. But why? Probably its API more suitable to
accept a collection as an input? It's essentially 'toArrayAndThen'
(similar to 'collectingAndThen' collector). There's a suggestion [1]
to add transform() method to stream which is much more general, and
can be used like `stream.transform(s -> Arrays.asList(s.toArray()))`.
Note that collection libraries may provide ready methods that supply
the lambdas specific to this library, and this allows to encapsulate
the exact implementation detail, like whether we need an array or
something else as an intermediate step, like:

class MyCustomCollection {
...
  static  Function, MyCustomCollection> toMyCustomCollection() {
return s -> createMyCustomCollectionFromStream((T[])s.toArray());
// in the next version we may change to for-each version if we
find that it's more performant
// or s -> {var c = new MyCustomCollection();
s.forEach(c::add); return c;}
  }
}

And use `stream.transform(MyCustomCollection.toMyCustomCollection())`
without caring whether it's array or something else in-between.

Finally, adding a `(T[])` cast to the standard library sounds a bad
idea. Imagine if the custom collection has a fixed element type:

class MyStringCollection implements Collection {
  public MyStringCollection(String[] array) {...}
}

It looks like stream.to(MyStringCollection::new) should work but in
fact, it will throw a ClassCastException

3. into() sounds more interesting as it's indeed useful to dump the
stream into an existing collection. It's mostly useful if the
collection is non-empty, as you can append into single collection from
existing sources. Essentially, into(c) == forEach(c::add), but it's
also possible to add optimizations for specific collections (like `if
(isSizedStream() && c instanceof ArrayList al) {
al.ensureCapacity(...); }`), so it could be faster.

With best regards,
Tagir Valeev

[1] https://bugs.openjdk.java.net/browse/JDK-8140283

On Thu, Apr 29, 2021 at 5:58 AM Donald Raab  wrote:
>
> I looked through a few libraries and found some methods where the option #2 
> proposal for Steam might be useful. If the JDK had constructors for 
> ArrayList, HashSet and other collection types that take arrays this method 
> would work there as well.
>
> > default > R to(Function function)
> > {
> >return function.apply((T[]) this.toArray());
> > }
>
>
> // JDK
> Set set = stream.to(Set::of);
> List list = stream.to(List::of);
> List arraysAsList = stream.to(Arrays::asList);
>
> // Guava
> ArrayList arrayList = stream.to(Lists::newArrayList);
> HashSet hashSet = stream.to(Sets::newHashSet);
> Multiset multiset = stream.to(ImmutableMultiset::copyOf);
> List guavaList = stream.to(ImmutableList::copyOf);
> Set guavaSet = stream.to(ImmutableSet::copyOf);
>
> // Apache Commons Collections
> FluentIterable fluentIterable = stream.to(FluentIterable::of);
>
> // Eclipse Collections
> MutableList adapter = stream.to(ArrayAdapter::adapt);
>
> MutableList mutableList = stream.to(Lists.mutable::with);
> MutableSet mutableSet = stream.to(Sets.mutable::with);
> MutableBag mutableBag = stream.to(Bags.mutable::with);
>
> // Eclipse Collections - ListIterable, SetIterable and Bag all extend 
> Iterable, not Collection
> ListIterable listIterable = stream.to(Lists.mutable::with);
> SetIterable setIterable = stream.to(Sets.mutable::with);
> Bag bag = stream.to(Bags.mutable::with);
>
> // Eclipse Collections - Immutable Collections do not extend Collection
> ImmutableList immutableList = stream.to(Lists.immutable::with);
> ImmutableSet immutableSet = stream.to(Sets.immutable::with);
> ImmutableBag immutableBag = stream.to(Bags.immutable::with);
>
> // Eclipse Collections - Stack does not extend Collection
> StackIterable stackIterable = stream.to(Stacks.mutable::with);
> MutableStack mutableStack = stream.to(Stacks.mutable::with);
> ImmutableStack immutableStack = stream.to(Stacks.immutable::with);
>
> // Eclipse Collections - Mutable Map and MutableBiMap are both Iterable so 
> they are valid returns
> MutableMap map =
> stream.to(array -> ArrayAdapter.adapt(array)
> .toMap(String::toLowerCase, String::toUpperCase));
>
> MutableBiMap biMap =
> stream.to(array -> ArrayAdapter.adapt(array)
> .toBiMap(String::toLowerCase, String::toUpperCase));
>
> Thanks,
> Don
>
> > On Apr 27, 2021, at 1:35 AM, Donald Raab  wrote:
> >
> > I realized after sending that option 2 can be made more

Re: RFR: 8265356: need code example for getting canonical constructor of a Record [v2]

On Sat, 24 Apr 2021 01:32:45 GMT, Tagir F. Valeev  wrote:

>> I decided to show a complete static method in the example, so it could be 
>> copied to user utility class as is. Not sure if it's reasonable to add 
>> `assert cls.isRecord();` there. Also I don't know whether there's a 
>> limitation on max characters in the sample code. Probable a line break in 
>> `static \nConstructor getCanonicalConstructor(Class 
>> cls)` is unnecessary.
>> 
>> ---
>> Aside from this PR, I've found a couple of things to clean up in 
>> `java.lang.Class`:
>> 1. There's erroneous JavaDoc link in `getSimpleName()` JavaDoc (introduced 
>> by @jddarcy in #3038). It should be `#isArray()` instead of `isArray()`.
>> 2. Methods Atomic::casAnnotationType and Atomic::casAnnotationData have 
>> unused type parameters ``.
>> 3. Probably too much but AnnotationData can be nicely converted to a record! 
>> Not sure, probably nobody wants to have `java.lang.Record` initialized too 
>> early or increasing the footprint of such a basic class in the metaspace, so 
>> I don't insist on this.
>> 
>> 
>> private record AnnotationData(
>> Map, Annotation> annotations,
>> Map, Annotation> declaredAnnotations,
>> // Value of classRedefinedCount when we created this AnnotationData 
>> instance
>> int redefinedCount) {
>> }
>> 
>> 
>> Please tell me if it's ok to fix 1 and 2 along with this PR.
>
> Tagir F. Valeev has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - Trailing space removed
>  - Add a reference from java.lang.Record to related Class methods
>  - Fix cosmetic issues

Overall looks fine modulo adding `@apiNote`. Since the changes are all either 
editorial/markup or informational, I don't think this needs a CSR. Well, the 
removal of an unused type variable strictly constitutes a signature change, but 
those methods aren't public APIs so I still think it's ok without a CSR.

src/java.base/share/classes/java/lang/Class.java line 2363:

> 2361:  * Conversely, if {@link #isRecord()} returns {@code true}, then 
> this method
> 2362:  * returns a non-null value.
> 2363:  *

I forgot to mention, this example should be within an `@apiNote`.

-

Marked as reviewed by smarks (Reviewer).

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

Re: Collection::getAny discussion

2021-04-30 Thread Donald Raab

To clarify, RichIterable is not a subclass of Collection. As we discovered in 
JDK 15, a problem exists when we add default methods to interfaces that might 
get “mixed” with other interfaces that already have those methods. There are a 
few potential issues with adding zero argument default methods to common 
interfaces. The two easiest to reason about that we have experience with are:

1. Signatures don’t match (e.g. getAny() returns Optional) - very bad - lots of 
pain caused - forces backwards incompatible change to library APIs - this 
happened when default sort() was added to List and returned void
2. Signatures match, but no concrete implementations when mixing competing 
default implementations - work for library developers - forces clients to 
upgrade to new version of library to use new version of JDK (e.g. EC 10.4 
upgrade to work with JDK 15 for CharSequence.isEmpty())

https://stuartmarks.wordpress.com/2020/09/22/incompatibilities-with-jdk-15-charsequence-isempty/

I think adding getAny() to Collection makes sense, that’s why we have defined 
the method on RichIterable. For Eclipse Collections, option #2 is the only 
option that works (getAny() returns E). It will cause us OSS library 
maintainers a bunch of work and a release upgrade. Knowing the direction and 
testing with early access versions (as we do) prior to our next release will 
help so we can start coding out any necessary concrete implementations wherever 
they don’t exist in the hierarchy.

The default getAny() implementation on RichIterable is different than just 
calling iterator.next() though. The getAny() method is currently defined as 
calling getFirst() which will return null in the case an iterable is empty. 

So this creates a new kind of potential issue we haven’t experienced where we 
could wind up with two getAny() methods with the same signature but with 
different specifications and results on empty. This will be potentially 
confusing for Eclipse Collections users as they could get different behavior 
from any JDK collections for getAny(), or libraries they use could get 
different behavior for Eclipse Collections types that extend JDK types. Do we 
change the Eclipse Collections specification to match the new JDK 
specification? Will this break anyone that currently uses it by causing new 
unexpected Runtime exceptions? I honestly don’t know.

For additional reference, there is also a getOnly() method on RichIterable 
which behaves differently than getAny(). 

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

There are also getFirst() and getLast() methods on RichIterable which were 
deprecated in 6.0 but will never be removed. They were added to OrderedIterable 
where they make more sense.

> On Apr 30, 2021, at 4:14 PM, Brian Goetz  wrote:
> 
> 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

Re: RFR: 8265128: [REDO] Optimize Vector API slice and unslice operations [v3]

2021-04-30 Thread Sandhya Viswanathan

On Fri, 30 Apr 2021 23:34:15 GMT, Paul Sandoz  wrote:

> > > @PaulSandoz would it be possible for you to run this through your testing?
> > 
> > 
> > Started, will report back when done.
> 
> Tier 1 to 3 tests all pass on build profiles linux-x64 linux-aarch64 
> macosx-x64 windows-x64

@PaulSandoz Thanks a lot!

-

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

Re: RFR: 8265128: [REDO] Optimize Vector API slice and unslice operations [v3]

On Fri, 30 Apr 2021 17:44:38 GMT, Paul Sandoz  wrote:

> > @PaulSandoz would it be possible for you to run this through your testing?
> 
> Started, will report back when done.

Tier 1 to 3 tests all pass on build profiles linux-x64 linux-aarch64 macosx-x64 
windows-x64

-

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

Re: ReversibleCollection proposal

2021-04-30 Thread Alan Snyder

It sounds like the items processing maintainer would be looking for 
OrderedCollection and might or might not find ReversibleCollection. :-)

I suspect you would agree that OrderedCollection by itself is too weak to 
justify being a type. It’s basically Iterable with the extra bit that the 
iteration order is not an implementation artifact.

In this example, using the type system to detect a bug like the old bug seems 
like overkill. Even if the parameter were Ordered, it still might not be the 
*right* order. The maintainer of the client code needs to understand that.

Suppose the items processor wants to require that the parameter collection not 
contain duplicates. Would you suggest adding a type for that? Clearly Set would 
be just as unnecessarily restrictive as List was for ordering. Absurdity 
approaches…

The issue of Reversible remains, above and beyond Ordered. Have you considered 
the alternative of a collection providing a Reversed view of itself, in the 
same sense that unmodifiable collections are views of an underlying collection?

  Alan

> On Apr 30, 2021, at 3:42 PM, Stuart Marks  wrote:
> 
> Consider the case of a large application or other system, one that's large 
> enough to have lots of internal APIs, but that is built as a single unit, so 
> release-to-release compatibility isn't an issue. Suppose there is some method
> 
>processItemsInOrder(List items)
> 
> that has to process items in the order in which they occur, because 
> processing of later items might depend the processing of earlier ones. The 
> maintainer of this API chose to accept a List as a parameter, because it's a 
> common interface and it's clearly an ordered collection.
> 
> Now consider a client that gets items from different places, keeping them in 
> order, but removing duplicates. It might do something like this:
> 
>var items = new LinkedHashSet();
>items.addAll(getItemsFromSomeplace());
>items.addAll(getItemsFromAnotherPlace());
>items.addAll(getItemsFromSomeplaceElse());
>processItemsInOrder(new ArrayList<>(items));
> 
> It turns out the copying of the items into an ArrayList is a performance 
> bottleneck, so the maintainer of the client code asks the maintainer of the 
> items processing code to change the API to accept Collection instead.
> 
> The items processing maintainer demurs, recalling that the API *did* accept 
> Collection in the past, and a bug where somebody accidentally passed a 
> HashSet resulted in a customer escalation because of item processing 
> irregularities. In the aftermath of that escalation, the API was changed to 
> List. The client maintainer reluctantly pursues alternatives for generating a 
> deduplicated List.
> 
> But wait! Those Java guys added a ReversibleCollection interface in Java N. 
> It has the desired property of being ordered, and conveniently it's a 
> supertype of both List and LinkedHashSet. The items processing maintainer 
> adjusts the API to consume ReversibleCollection, and the client maintainer 
> removes the temporary ArrayList, and everybody is happy.
> 
> s'marks
>

Re: ReversibleCollection proposal






You did not really answer to the real question, why should i use 
ReversibleCollection instead of a Collection as parameter.
You said that it's a better type because you can not send a HashSet, but as i 
said, sending a HashSet of 0 or 1 element is perfectly valid.
For me, we are not far from, it's typechecking for the sake of typechecking.


I thought I did answer it, but it might have been in response to a different message 
on a different part of the thread. But I'll make the case in a more explicit fashion 
here.


First, a couple background points related to this:

 - ReversibleCollection is not intended to, and indeed cannot represent all ordered 
collections. Queue is ordered and is not a ReversibleCollection, and there are 
likely other collections out there that are ordered but that implement Collection 
directly. Also, they might or might not be reversible.


 - I expect that few, if any Java SE APIs will be adjusted to use 
ReversibleCollection as a parameter type. Any APIs that use Collection but require 
an ordered type cannot be changed because of compatibility reasons.


Despite both of these points, I believe ReversibleCollection can be successful, and 
it can be useful in APIs as a parameter type. (The proposal itself uses 
ReversibleCollection and ReversibleSet as method return types.)


Consider the case of a large application or other system, one that's large enough to 
have lots of internal APIs, but that is built as a single unit, so 
release-to-release compatibility isn't an issue. Suppose there is some method


processItemsInOrder(List items)

that has to process items in the order in which they occur, because processing of 
later items might depend the processing of earlier ones. The maintainer of this API 
chose to accept a List as a parameter, because it's a common interface and it's 
clearly an ordered collection.


Now consider a client that gets items from different places, keeping them in order, 
but removing duplicates. It might do something like this:


var items = new LinkedHashSet();
items.addAll(getItemsFromSomeplace());
items.addAll(getItemsFromAnotherPlace());
items.addAll(getItemsFromSomeplaceElse());
processItemsInOrder(new ArrayList<>(items));

It turns out the copying of the items into an ArrayList is a performance bottleneck, 
so the maintainer of the client code asks the maintainer of the items processing 
code to change the API to accept Collection instead.


The items processing maintainer demurs, recalling that the API *did* accept 
Collection in the past, and a bug where somebody accidentally passed a HashSet 
resulted in a customer escalation because of item processing irregularities. In the 
aftermath of that escalation, the API was changed to List. The client maintainer 
reluctantly pursues alternatives for generating a deduplicated List.


But wait! Those Java guys added a ReversibleCollection interface in Java N. It has 
the desired property of being ordered, and conveniently it's a supertype of both 
List and LinkedHashSet. The items processing maintainer adjusts the API to consume 
ReversibleCollection, and the client maintainer removes the temporary ArrayList, and 
everybody is happy.


s'marks

Re: RFR: 8265989: System property for the native character encoding name [v3]

2021-04-30 Thread Naoto Sato

> After some internal discussion, we thought it was good to expose the native 
> environment's default character encoding, which Charset.defaultCharset() is 
> currently based on. This way applications will have a better migration path 
> after the [JEP 400](https://openjdk.java.net/jeps/400) is implemented, in 
> which Charset.defaultCharset() will return UTF-8, but the value of this new 
> system property will remain intact. A 
> [CSR](https://bugs.openjdk.java.net/browse/JDK-8266075) has been filed with 
> more detailed information.

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Fixed a typo.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3777/files
  - new: https://git.openjdk.java.net/jdk/pull/3777/files/e76ff410..01b2ffc3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3777=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3777=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3777.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3777/head:pull/3777

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

Re: RFR: 8265989: System property for the native character encoding name [v2]

2021-04-30 Thread Naoto Sato

On Fri, 30 Apr 2021 21:09:36 GMT, Roger Riggs  wrote:

> To support the statement that changing the property has no effect.
> Please add it to the jdk.internal.util.StaticProperties cached values and an 
> internal access method.

Thanks. Added.

-

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

Re: RFR: 8265989: System property for the native character encoding name [v2]

2021-04-30 Thread Naoto Sato

> After some internal discussion, we thought it was good to expose the native 
> environment's default character encoding, which Charset.defaultCharset() is 
> currently based on. This way applications will have a better migration path 
> after the [JEP 400](https://openjdk.java.net/jeps/400) is implemented, in 
> which Charset.defaultCharset() will return UTF-8, but the value of this new 
> system property will remain intact. A 
> [CSR](https://bugs.openjdk.java.net/browse/JDK-8266075) has been filed with 
> more detailed information.

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Added internal getter for native.encoding

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3777/files
  - new: https://git.openjdk.java.net/jdk/pull/3777/files/fa5246c8..e76ff410

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3777=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3777=00-01

  Stats: 16 lines in 1 file changed: 15 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3777.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3777/head:pull/3777

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

Re: RFR: JDK-8266129: tools/jpackage/windows/WinInstallerIconTest.java hangs with fastdebug

On Fri, 30 Apr 2021 20:19:25 GMT, Andy Herrick  wrote:

> JDK-8266129: tools/jpackage/windows/WinInstallerIconTest.java hangs with 
> fastdebug
> 
> just disabling this test when vm.debug

Marked as reviewed by almatvee (Committer).

-

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

Re: RFR: JDK-8266227: Fix help text for --mac-signing-keychain

On Fri, 30 Apr 2021 15:25:13 GMT, Andy Herrick  wrote:

> JDK-8266227: Fix help text for --mac-signing-keychain

Marked as reviewed by almatvee (Committer).

-

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

Re: Collection::getAny discussion

2021-04-30 Thread Stephen Colebourne

On Fri, 30 Apr 2021 at 19:50, Stuart Marks  wrote:
> 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.)

I think any get() or getAny() method on Collection is semantically
equivalent to iterator.next(). I'm not sure there is another viable
option.

>   * 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 should be added to the JDK. They are useful.

Stephen

Re: RFR: 8265989: System property for the native character encoding name

2021-04-30 Thread Roger Riggs

On Wed, 28 Apr 2021 22:24:31 GMT, Naoto Sato  wrote:

> After some internal discussion, we thought it was good to expose the native 
> environment's default character encoding, which Charset.defaultCharset() is 
> currently based on. This way applications will have a better migration path 
> after the [JEP 400](https://openjdk.java.net/jeps/400) is implemented, in 
> which Charset.defaultCharset() will return UTF-8, but the value of this new 
> system property will remain intact. A 
> [CSR](https://bugs.openjdk.java.net/browse/JDK-8266075) has been filed with 
> more detailed information.

To support the statement that changing the property has no effect.
Please add it to the jdk.internal.util.StaticProperties cached values and an 
internal access method.

Otherwise looks good.

-

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

Re: RFR: JDK-8266129: tools/jpackage/windows/WinInstallerIconTest.java hangs with fastdebug

2021-04-30 Thread Alexey Semenyuk

On Fri, 30 Apr 2021 20:19:25 GMT, Andy Herrick  wrote:

> JDK-8266129: tools/jpackage/windows/WinInstallerIconTest.java hangs with 
> fastdebug
> 
> just disabling this test when vm.debug

Marked as reviewed by asemenyuk (Reviewer).

-

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

Re: 8252827: Caching Integer.toString just like Integer.valueOf

2021-04-30 Thread Maurizio Cimadamore


Ciao Raffaello,
another general consideration: caching sometimes makes certain 
optimizations harder to pull off.


Few years ago I've seen this talk at Fosdem:

https://archive.fosdem.org/2020/schedule/event/reducing_gc_times/

See minute 5:53 - where it shows that cached Integer values (as per 
Integer::valueOf) inhibit scalarization, and using `new Integer(...)` 
leads to superior performances.


It's a pattern I have observed several time when working with the 
Foreign Memory API, where most of the abstractions are immutable: 
attempting to cache things (e.g. MemoryAddress instances) often comes up 
with the same GC overhead (because these instances are escape analyzed 
away) and ends up with worse performance overall (loss of scalarization).


Of course this is not _always_ the case - but I wanted to add this 
particular angle to the discussion.


Cheers
Maurizio

On 16/04/2021 17:48, Raffaello Giulietti wrote:

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827

RFR: JDK-8266129: tools/jpackage/windows/WinInstallerIconTest.java hangs with fastdebug

2021-04-30 Thread Andy Herrick

JDK-8266129: tools/jpackage/windows/WinInstallerIconTest.java hangs with 
fastdebug

just disabling this test when vm.debug

-

Commit messages:
 - JDK-8266129: tools/jpackage/windows/WinInstallerIconTest.java hangs with 
fastdebug

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

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

Re: Collection::getAny discussion

2021-04-30 Thread 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: Collection::getAny discussion

2021-04-30 Thread Donald Raab

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 proposal is similar to what 
> onlyElement would look like if it were on Collection.
> 
> Let's summarize the variations so far:
> 
> * preconditions: exactly one element, at-most-one, at-least-one
> * behavior if preconditions not met: return null, return empty Optional, throw
>   exception
> * remove element or just peek
> * match a particular element, or return an arbitrary element
> 
> That's a lot of variations!
> 
> Before we talk about specific APIs, though, I wanted to talk about use cases. 
> Which of these variations are more useful or less useful? Which are likely to 
> appear in code? Henri gave a fairly specific example with a

Integrated: 8266155: Convert java.base to use Stream.toList()

2021-04-30 Thread Ian Graves

On Tue, 27 Apr 2021 21:34:02 GMT, Ian Graves  wrote:

> 8266155: Convert java.base to use Stream.toList()

This pull request has now been integrated.

Changeset: dd05158b
Author:Ian Graves 
Committer: Pavel Rappo 
URL:   
https://git.openjdk.java.net/jdk/commit/dd05158b24e8b399052a170ea9fe9ee6f65c0432
Stats: 25 lines in 8 files changed: 0 ins; 11 del; 14 mod

8266155: Convert java.base to use Stream.toList()

Reviewed-by: bpb, naoto, iris, chegar

-

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

Integrated: 8260560: convert jdeps and jdeprscan tools to use Stream.toList()

2021-04-30 Thread Ian Graves

On Tue, 27 Apr 2021 00:20:49 GMT, Ian Graves  wrote:

> 8260560: convert jdeps and jdeprscan tools to use Stream.toList()

This pull request has now been integrated.

Changeset: c36c63a0
Author:Ian Graves 
Committer: Pavel Rappo 
URL:   
https://git.openjdk.java.net/jdk/commit/c36c63a008fa5e8b00dfc36c887cd9497fb91ab5
Stats: 9 lines in 4 files changed: 0 ins; 0 del; 9 mod

8260560: convert jdeps and jdeprscan tools to use Stream.toList()

Reviewed-by: alanb, mchung, iris

-

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

Re: [External] : Re: ReversibleCollection proposal


OK, I think we can wrap up this portion of the thread.

As the proposal stands, it has add{First,Last} returning void instead of some useful 
value. For SortedSet and for LinkedHashMap's views, these throw UOE. Can we do better?


Collection has add(), Deque has add{First,Last} and offer{First,Last}, and 
BlockingDeque has put{First,Last}. I'm loathe to add new insertion methods that 
differ from these, either in signatures or semantics.


The BlockingDeque putX methods have blocking behavior, which only makes sense for a 
concurrent collection. Still, because these exist, we mustn't add putX methods 
elsewhere that have different semantics.


After having thought about this for a couple days, I think it's a really bad idea to 
reuse offerX methods. These would allow a collection to refuse the addition of an 
element and merely return a boolean indicating that. I can easily see people writing 
offerX code that doesn't check the return value, and if things shift around in their 
program, elements can be silently dropped. This would set a new precedent, as the 
present behavior is that collections can reject adding an element only by throwing 
an exception. Allowing refusal by returning 'false' would be a bad precedent.


So I think we're stuck with void-returning add{First,Last} methods.

Regarding throwing UOE, I think it's useful to distinguish between the LinkedHashMap 
views and SortedSet. Today, it's already the case that Map's view collections don't 
support addition, so the ReversibleX views of LinkedHashMap are similar in this regard.


SortedSet is different, as it's a top-level collection interface instead of a view 
collection. It's unusual for it not to support the addX operations. We explored some 
alternatives, such as throwing exceptions if preconditions aren't met. These seem 
fairly rare, and the alternative behaviors don't seem all that useful. In any case 
nothing emerged that was clearly better than simple UOE-throwing behavior.


OK, I think it's been useful to analyze various alternatives -- in particular I 
hadn't seriously considered the offerX methods -- but I'll leave the proposal as it 
stands regarding add{First,Last} methods.


s'marks



On 4/28/21 6:54 AM, Peter Levart wrote:


On 4/28/21 7:19 AM, Stuart Marks wrote:



On 4/27/21 9:17 AM, Anthony Vanelverdinghe wrote:

On Tuesday, April 27, 2021 11:25 CEST, Peter Levart  
wrote:

Now just some of my thoughts about the proposal:

- SortedSet.addFirst/.addLast - I think an operation that would be used
in situations like: "I know this element should always be greater than
any existing element in the set and I will push it to the end which
should also verify my assumption" is a perfectly valid operation. So
those methods throwing IllegalStateException in case the invariant can't
be kept seem perfectly fine to me.


This was raised before and addressed by Stuart in [0]:
"An alternative as you suggest might be that SortedSet::addFirst/addLast could 
throw
something like IllegalStateException if the element is wrongly positioned.
(Deque::addFirst/addLast will throw ISE if the addition would exceed a capacity
restriction.) This seems error-prone, though, and it's easier to understand and
specify that these methods simply throw UOE unconditionally. If there's a good 
use
case for the alternative I'd be interested in hearing it though."


Yes, to be clear, it was Stephen Coleborne who raised this previously [1] and it's 
my response that's quoted above.


Some further thoughts on this.

This is an example where, depending on the current state of the collection, the 
method might throw or it might succeed. This is useful in concurrent collections 
(such as the capacity-restricted Deque above), where the caller cannot check 
preconditions beforehand, because they might be out of date by the time the 
operation is attempted. In such cases the caller might not want to block, but 
instead it might catch the exception and report an error to its caller (or drop 
the request). Thus, calling the exception-throwing method is appropriate.


Something like SortedSet::addLast seems different, though. The state is the 
*values* of the elements already in the collection. This is something that can 
easily be checked, and probably should be checked beforehand:


    if (sortedSet.isEmpty() || sortedSet.last().compareTo(e) <= 0)
    sortedSet.add(e);
    else
    // e wouldn't be the last element, do something different



I was thinking more of a case where the else branch would actually throw 
IllegalStateException and do nothing else - a kind of add with precondition check. A 
precondition in the sense of Objects.requireNonNull(). I can't currently think of a 
real usecase now, so this kind of operation is probably very rare. Probably not 
useful since if you're adding to SortedSet, the order of elements added should not 
matter because SortedSet will sort them. If you just want to check the order of 
elements added and you are not willing

Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v6]

2021-04-30 Thread Igor Veresov

On Fri, 30 Apr 2021 02:19:30 GMT, Yi Yang  wrote:

>> The JDK codebase re-created many variants of checkIndex(`grep -I -r 
>> 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which 
>> annotated with @IntrinsicCandidate and it only has a corresponding C1 
>> intrinsic version.
>> 
>> In fact, there is an utility method 
>> `jdk.internal.util.Preconditions.checkIndex`(wrapped by 
>> java.lang.Objects.checkIndex) that behaves the same as these variants of 
>> checkIndex, we can replace these re-created variants of checkIndex by 
>> Objects.checkIndex, it would significantly reduce duplicated code and enjoys 
>> performance improvement because Preconditions.checkIndex is 
>> @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot.
>> 
>> But, the problem is currently HotSpot only implements the C2 version of 
>> Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we 
>> can firstly implement its C1 counterpart. There are also a few kinds of 
>> stuff we can do later:
>> 
>> 1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK 
>> codebase.
>> 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag
>> 
>> Testing: cds, compiler and jdk
>
> Yi Yang has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   better check1-4

Looks like now the test fails in the pre-submit tests?

-

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

Collection::getAny discussion


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 proposal is similar to what onlyElement would look like if 
it were on Collection.


Let's summarize the variations so far:

 * preconditions: exactly one element, at-most-one, at-least-one
 * behavior if preconditions not met: return null, return empty Optional, throw
   exception
 * remove element or just peek
 * match a particular element, or return an arbitrary element

That's a lot of variations!

Before we talk about specific APIs, though, I wanted to talk about use cases. Which 
of these variations are more useful or less useful? Which are likely to appear in 
code? Henri gave a fairly specific example with a reasonable "success" case 
(preconditions met) but it's not clear what should happen if the preconditions 
aren't met. Clearly an API would have to choose. What would the use site look like? 
In particular, does the use site always have to check for null, or catch an 
exception, or something?


Answers to these questions will help determine what APIs, if any, we might want 
to add.

***

There's another thing looming in the distance, which is pattern matching. You might 
have seen one of Brian Goetz's talks on pattern matching futures. You can get a 
glimpse of some upcoming pattern matching features in JEP 405.


    http://openjdk.java.net/jeps/405

In particular, this JEP extends pattern matching with an /extraction/ step, where, 
if there is a match, record or array components can be extracted and bound to local 
variables. This is a step closer to /deconstruction patterns/, where arbitrary 
classes and interfaces (not just records or arrays) can participate in pattern 
matching. (See discussion of this at the end of the JEP.)

Re: 8252827: Caching Integer.toString just like Integer.valueOf

(Catching up on old email threads.)

I don't have much to add to the technical discussion here. Yes, caching the Integer 
instances seems obsolescent, and it seems unlikely that caching String conversions 
will be helpful. I've gone ahead and closed out the bug. [1]

On triaging bugs... we do triage bugs pretty effectively, I think. However, 
enhancement requests (like this one) tend to languish. People tend to notice ones 
that are obviously bad ideas or obviously good ideas and act on them quickly. 
However, there are a lot of items that are neither obviously good nor bad, so they 
just sit there. Sometimes they just sit there even after there has been some email 
discussion on them. :-)

Sometimes I think we take these requests a bit too seriously. It looks to me like 
the submitter put about five minutes of thought into the request, and we've 
collectively probably spent 10x that dealing with it. I probably put too much effort 
into this bug myself; instead of researching the history, I could have simply closed 
it with a comment saying, "Closed, per discussion in " which 
would have been reasonable.

Anyway, it's closed.

s'marks

[1] https://bugs.openjdk.java.net/browse/JDK-8252827

On 4/17/21 5:19 AM, Raffaello Giulietti wrote:

Hi,

in view of Integer becoming a primitive class [1], the IntegerCache is probably 
going to disappear.

For a small, fixed range like the one you are proposing [-1, 16], there's no real 
need for a separate cache class. You could have a switch in the implementation of 
toString(), with the string literals then being part of the constant pool of the 
class. Not free beer, but supported by the VM since day 0.

It's only when the range is open that you'd need a cache similar to 
IntegerCache.

My 2 cents as well :-)
Raffaello

[1] https://openjdk.java.net/jeps/402

On 2021-04-17 11:18, Laurent Bourgès wrote:

Hi,

I read the JBS bug and I interpret it as:
- IntegerCache provides Integer instances for [-128, 127] by default
- Having Integer.toString(int) could behave the same or at least cache most 
probable values like [-1 to 16] or using the IntegerCache range.

It looks trivial and potentially could benefits to jdk itself to reduce memory 
garbage : is Integer.toString(int) widely used in the jdk codebase ?

Finally it can be alternatively implemented in application's code.

My 2 cents,
Laurent

Le sam. 17 avr. 2021 à 11:06, Raffaello Giulietti > a écrit :

    On 2021-04-17 07:07, David Holmes wrote:
 > On 17/04/2021 4:54 am, Raffaello Giulietti wrote:
 >> I guess the reporter meant to limit the cache range similarly to
    the
 >> one used for valueOf().
 >>
 >> I have no clue about the benefit/cost ratio for the proposed String
 >> cache. It really depends on usage, workload, etc. One can easily
 >> imagine both extreme scenarios but it's hard to tell how the
    average
 >> one would look.
 >>
 >> My post is only about either solving the issue by implementing the
 >> cache, which I can contribute to; or closing it because of lack of
 >> real-world need or interest.
 >
 > Caching for the sake of caching is not an objective in itself.
    Unless
 > the caching can be shown to solve a real problem, and the
    strategy for
 > managing the cache is well-defined, then I would just close the
 > enhancement request. (Historically whether an issue we don't have
    any
 > firm plans to address is just left open "forever" or closed, depends
 > very much on who does the bug triaging in that area. :) )
 >
 > Cheers,
 > David
 >

    Indeed, the impression is that many of the issues are probably open
    because it's unclear whether they should be addressed with some
    implementation or spec effort or whether they should be closed.
    Triaging
    is certainly a harder job than it appears at first sight ;-)

    It would be useful to have a kind of "suspended" or "limbo" resolution
    state on the JBS for issues like this one, so people searching for more
    compelling open ones would not encounter them.

    Personally, I would close this issue without a "fix"; or "suspend" it.

    Greetings
    Raffaello

 >>
 >> Greetings
 >> Raffaello
 >>
 >>
 >> On 2021-04-16 20:36, Roger Riggs wrote:
 >>> Hi,
 >>>
 >>> Is there any way to quantify the savings?
 >>> And what technique can be applied to limit the size of the cache.
 >>> The size of the small integer cache is somewhat arbitrary.
 >>>
 >>> Regards, Roger
 >>>
 >>> On 4/16/21 12:48 PM, Raffaello Giulietti wrote:
  Hello,

  does the enhancement proposed in [1] make sense, both today
    and when
  wrappers will be migrated to primitive classes?
  If so, it should be rather simple to add it and I could
    prepare a PR.
  If not, the issue might perhaps be closed.

Re: RFR: 8266179: [macos] jpackage should specify architecture for produced pkg files

2021-04-30 Thread Alexey Semenyuk

On Fri, 30 Apr 2021 04:22:37 GMT, Alexander Matveev  
wrote:

> jpackage should specify architecture for produced PKG files via 
> hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable 
> on x64 without specifying hostArchitectures which is not correct and if 
> install on arm Mac it will request Rosetta 2. With proposed fix by setting 
> hostArchitectures="x86_x64" if installer contains x64 binaries, it will be 
> installable on x64 Mac and will require Rosetta 2 on arm Mac. 
> hostArchitectures will be set to arm64 if installer contain aarch64 binaries 
> and will gave error when run on x64 Mac and will be installable on arm Mac 
> without triggering installation of Rosetta 2.

Marked as reviewed by asemenyuk (Reviewer).

-

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

Re: RFR: 8265128: [REDO] Optimize Vector API slice and unslice operations [v3]

On Fri, 30 Apr 2021 02:31:07 GMT, Paul Sandoz  wrote:

>> Sandhya Viswanathan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Review comments: blendmask etc
>
> Marked as reviewed by psandoz (Reviewer).

> @PaulSandoz would it be possible for you to run this through your testing?

Started, will report back when done.

-

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

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v5]

2021-04-30 Thread Vicente Romero

> Please review this PR that intents to make sealed classes a final feature in 
> Java. This PR contains compiler and VM changes. In line with similar PRs, 
> which has made preview features final, this one is mostly removing preview 
> related comments from APIs plus options in test cases. Please also review the 
> related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090)
> 
> Thanks
> 
> note: this PR is related to 
> [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated 
> after it.

Vicente Romero 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:

 - Merge branch 'master' into JDK-8260517
 - Merge branch 'master' into JDK-8260517
 - updating comment after review feedback
 - removing javax.lang.model changes
 - Merge branch 'master' into JDK-8260517
 - Merge branch 'master' into JDK-8260517
 - fixing failing regression tests
 - JVM related changes
 - 8260517: Compiler implementation for Sealed Classes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3526/files
  - new: https://git.openjdk.java.net/jdk/pull/3526/files/43e9cb5f..2744f615

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3526=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3526=03-04

  Stats: 506473 lines in 3844 files changed: 20558 ins; 483521 del; 2394 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3526.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3526/head:pull/3526

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

Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v6]

On Fri, 30 Apr 2021 02:19:30 GMT, Yi Yang  wrote:

>> The JDK codebase re-created many variants of checkIndex(`grep -I -r 
>> 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which 
>> annotated with @IntrinsicCandidate and it only has a corresponding C1 
>> intrinsic version.
>> 
>> In fact, there is an utility method 
>> `jdk.internal.util.Preconditions.checkIndex`(wrapped by 
>> java.lang.Objects.checkIndex) that behaves the same as these variants of 
>> checkIndex, we can replace these re-created variants of checkIndex by 
>> Objects.checkIndex, it would significantly reduce duplicated code and enjoys 
>> performance improvement because Preconditions.checkIndex is 
>> @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot.
>> 
>> But, the problem is currently HotSpot only implements the C2 version of 
>> Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we 
>> can firstly implement its C1 counterpart. There are also a few kinds of 
>> stuff we can do later:
>> 
>> 1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK 
>> codebase.
>> 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag
>> 
>> Testing: cds, compiler and jdk
>
> Yi Yang has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   better check1-4

It was my hope this would eventually happen when we added `Objects.checkIndex` 
and the underlying intrinsic. Very good!

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v3]

2021-04-30 Thread Mandy Chung

On Fri, 30 Apr 2021 12:24:38 GMT, Maurizio Cimadamore  
wrote:

> I've added `@CS` in the interface methods too. I've also added a stronger 
> test which creates method handles in one module (which doesn't have native 
> access) and then calls them from another module (which does NOT have native 
> access enabled), and make sure that all call fails as expected (e.g. that 
> caller context is that of the lookup module).

Thanks.
 
> Surprisingly, CheckCSMs does NOT fail if both interface and implementation 
> methods are `@CS` annotated - but if only interface is annotated, the test 
> fails. This seems odd - since I can't find any logic in the test which check 
> for overriding. Is the test accidentally passing?

I create JDK-8266383 and I will look into that.

-

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

Re: RFR: 8265128: [REDO] Optimize Vector API slice and unslice operations [v3]

2021-04-30 Thread Sandhya Viswanathan

On Fri, 30 Apr 2021 02:31:07 GMT, Paul Sandoz  wrote:

>> Sandhya Viswanathan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Review comments: blendmask etc
>
> Marked as reviewed by psandoz (Reviewer).

@PaulSandoz would it be possible for you to run this through your testing?

-

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

Re: RFR: 8266013: Unexpected replacement character handling on stateful CharsetEncoder [v2]

2021-04-30 Thread Ichiroh Takiguchi

> When an invalid character is converted by getBytes() method, the character is 
> converted to replacement byte data.
> Shift code (SO/SI) may not be added into right place by EBCDIC Mix charset.
> EBCDIC Mix charset encoder is stateful encoder.
> Shift code should be added by switching character set.
> On x-IBM1364, "\u3000\uD800" should be converted to "\x0E\x40\x40\x0F\x6F", 
> but "\x0E\x40\x40\x6F\x0F"
> SI is not in right place.
> 
> Also ISO2022 related charsets use escape sequence to switch character set.
> But same kind of issue is there.

Ichiroh Takiguchi has updated the pull request incrementally with one 
additional commit since the last revision:

  8266013: Unexpected replacement character handling on stateful CharsetEncoder

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3719/files
  - new: https://git.openjdk.java.net/jdk/pull/3719/files/d6a0a41b..33107e67

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3719=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3719=00-01

  Stats: 59 lines in 2 files changed: 40 ins; 5 del; 14 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3719.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3719/head:pull/3719

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

Re: RFR: 8266054: VectorAPI rotate operation optimization [v2]