Re: [15] RFR: 8239480: Support for CLDR version 37

[Joe Wang]

On 5/20/2020 7:06 PM, naoto.s...@oracle.com wrote:

Hi Joe, thanks again!


No problem.


The license file "unicode-license.txt" is actually included in the 
version 37 artifact:


https://github.com/unicode-org/cldr/blob/release-37/unicode-license.txt

The changeset uses the file as is, so I believe it is OK.


I meant the md files that include the license text with the additional 
"Terms of Use", starting from line 55. That looks like a copy of this: 
https://www.unicode.org/copyright.html, but headings and numbering were 
lost. Plus, without html format, some of the lines stretched very long 
. 
But again, the format of the md files might not be that important. Your 
call.


-Joe



Naoto

On 5/20/20 6:51 PM, Joe Wang wrote:

Looks good to me.

A minor comment: in the license files, Terms of Use lost its original 
format  (e.g. headings and 
numbering), lines are also super long. But I think we've seen them in 
the previous update, it's up to you whether you'd want to reformat 
them. Either way, no need to regenerate the huge webrev :-)


Best,
Joe

On 5/20/2020 2:31 PM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issue:

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

The proposed changeset, and the release note draft are located at:

https://cr.openjdk.java.net/~naoto/8239480/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8243581

This updates the CLDR locale data to version 37. Although the 
changeset is huge, this mainly consists of the locale data and 
licenses changes. Other than those, only one test case was updated 
(IncludeLocalesPluginTest.java) due to the locale data change.


Naoto

Re: [15] RFR: 8239480: Support for CLDR version 37

[naoto . sato]

Hi Joe, thanks again!

The license file "unicode-license.txt" is actually included in the 
version 37 artifact:


https://github.com/unicode-org/cldr/blob/release-37/unicode-license.txt

The changeset uses the file as is, so I believe it is OK.

Naoto

On 5/20/20 6:51 PM, Joe Wang wrote:

Looks good to me.

A minor comment: in the license files, Terms of Use lost its original 
format  (e.g. headings and 
numbering), lines are also super long. But I think we've seen them in 
the previous update, it's up to you whether you'd want to reformat them. 
Either way, no need to regenerate the huge webrev :-)


Best,
Joe

On 5/20/2020 2:31 PM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issue:

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

The proposed changeset, and the release note draft are located at:

https://cr.openjdk.java.net/~naoto/8239480/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8243581

This updates the CLDR locale data to version 37. Although the 
changeset is huge, this mainly consists of the locale data and 
licenses changes. Other than those, only one test case was updated 
(IncludeLocalesPluginTest.java) due to the locale data change.


Naoto

Re: [15] RFR: 8239480: Support for CLDR version 37

[Joe Wang]

Looks good to me.

A minor comment: in the license files, Terms of Use lost its original 
format  (e.g. headings and 
numbering), lines are also super long. But I think we've seen them in 
the previous update, it's up to you whether you'd want to reformat them. 
Either way, no need to regenerate the huge webrev :-)


Best,
Joe

On 5/20/2020 2:31 PM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issue:

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

The proposed changeset, and the release note draft are located at:

https://cr.openjdk.java.net/~naoto/8239480/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8243581

This updates the CLDR locale data to version 37. Although the 
changeset is huge, this mainly consists of the locale data and 
licenses changes. Other than those, only one test case was updated 
(IncludeLocalesPluginTest.java) due to the locale data change.


Naoto

Re: [15] RFR: 8245241: Incorrect locale provider preference is not logged

[Joe Wang]

Hi Naoto,

Thanks for the explanation. I agree on the needs for maintaining 
backward compatibility.  The patch looks good me then.


Best,
Joe

On 5/20/2020 4:49 PM, naoto.s...@oracle.com wrote:

Hi Joe, thanks for the review.

On 5/20/20 4:10 PM, Joe Wang wrote:

Hi Naoto,

I don't seem to see the DateFormat class or the getDateInstance 
methods specify how errors may be handled (or logged). Is that stated 
somewhere else?


Description of the system property "java.locale.providers" is in the 
class description of java.util.spi.LocaleServiceProvider class, in 
which the possible provider names can only be 
CLDR/COMPAT/SPI/HOST/JRE. So possible error with this system property 
is user's typo of the provider names. Although the behavior is not 
described, wrong names have been simply ignored at runtime. The 
problem here is that user cannot tell that he's done typo, and this 
fix is exactly to address it.


  In other cases, I see that you've changed it to throw 
ServiceConfigurationError, that looks to me may be better than a log 
as a configuration error  (or specifying wrong provider) sounds to me 
more severe than info.


Those exceptions will never happen in normal situation, since those 
locations are loading SPI/HostLocaleProviderAdapter class(es) that are 
the JDK classes (classes/methods are known to exist).
On the other hand, I kept exception handlers in LocaleProviderAdapter 
to generate Level.INFO log, as this is for the user's typo of the 
provider names (explained above). Again this should be ignored and 
replaced/continue with proper default behavior, and letting user know 
the typo. If this were throwing SCError, it would break compatibility.




Of the three HostLocaleProviderAdapterImpl, the one for unix is 
deleted, is there a specific reason?


Because it is simply not necessary (empty class). It was meant to be 
implemented later, but the host provider on Unixes has never been 
requested. Probably because OS date/time/number settings in Unixes are 
less important to Java clients.


HTH,
Naoto



Best,
Joe

On 5/20/2020 10:29 AM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issue:

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

The proposed changeset is located at:

https://cr.openjdk.java.net/~naoto/8245241/webrev.00/

Incorrect user-provided provider preference is supposed to be 
logged. However it is not so because it is using 
PlatformLogger(SurrogateLogger) which uses the default logging 
level. I changed it to use j.l.System's logger and bumped it to INFO 
level (it should notify the user by default). By taking this 
opportunity, I did some clean-up in locale provider adapter's 
logging related code as well.


Naoto

Re: [15] RFR: 8245241: Incorrect locale provider preference is not logged

[naoto . sato]

Hi Joe, thanks for the review.

On 5/20/20 4:10 PM, Joe Wang wrote:

Hi Naoto,

I don't seem to see the DateFormat class or the getDateInstance methods 
specify how errors may be handled (or logged). Is that stated somewhere 
else?


Description of the system property "java.locale.providers" is in the 
class description of java.util.spi.LocaleServiceProvider class, in which 
the possible provider names can only be CLDR/COMPAT/SPI/HOST/JRE. So 
possible error with this system property is user's typo of the provider 
names. Although the behavior is not described, wrong names have been 
simply ignored at runtime. The problem here is that user cannot tell 
that he's done typo, and this fix is exactly to address it.


  In other cases, I see that you've changed it to throw 
ServiceConfigurationError, that looks to me may be better than a log as 
a configuration error  (or specifying wrong provider) sounds to me more 
severe than info.


Those exceptions will never happen in normal situation, since those 
locations are loading SPI/HostLocaleProviderAdapter class(es) that are 
the JDK classes (classes/methods are known to exist).
On the other hand, I kept exception handlers in LocaleProviderAdapter to 
generate Level.INFO log, as this is for the user's typo of the provider 
names (explained above). Again this should be ignored and 
replaced/continue with proper default behavior, and letting user know 
the typo. If this were throwing SCError, it would break compatibility.




Of the three HostLocaleProviderAdapterImpl, the one for unix is deleted, 
is there a specific reason?


Because it is simply not necessary (empty class). It was meant to be 
implemented later, but the host provider on Unixes has never been 
requested. Probably because OS date/time/number settings in Unixes are 
less important to Java clients.


HTH,
Naoto



Best,
Joe

On 5/20/2020 10:29 AM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issue:

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

The proposed changeset is located at:

https://cr.openjdk.java.net/~naoto/8245241/webrev.00/

Incorrect user-provided provider preference is supposed to be logged. 
However it is not so because it is using 
PlatformLogger(SurrogateLogger) which uses the default logging level. 
I changed it to use j.l.System's logger and bumped it to INFO level 
(it should notify the user by default). By taking this opportunity, I 
did some clean-up in locale provider adapter's logging related code as 
well.


Naoto

Re: RFR: JDK-8225056 VM support for sealed classes

[David Holmes]

Hi Vicente,

On 21/05/2020 1:40 am, Vicente Romero wrote:

Hi David,


src/java.base/share/classes/java/lang/Class.java

There needs to be a CSR request for these changes.


yes there is one already: https://bugs.openjdk.java.net/browse/JDK-8244556


+  * Returns an array containing {@code ClassDesc} objects 
representing all the
+  * permitted subclasses of this {@linkplain Class} if it is 
sealed. Returns an empty array if this

+  * {@linkplain Class} is not sealed.

should add "or this class represents an array or primitive type" 
(using the standard wording for such cases).


well given that array and primitive classes are not sealed classes I 
think we are already covered by the method's spec.


Yes, now I've seen the JLS updates, this is more clear to me.

Thanks,
David
-



+  * @throws IllegalArgumentException if a class descriptor is not 
in the correct format


IllegalArgumentException is not an appropriate exception to use as 
this method takes no arguments. If the class descriptor is not valid 
and it comes from the VM then I think we have a problem with how the 
VM validates class descriptors. Any IAE from ClassDesc.of should be 
caught and converted to a more suitable exception type - preferably 
InternalError if the VM should always return valid strings.


we agree with you here, this will be fixed in the next review iteration.



+ public ClassDesc[] getPermittedSubclasses() {

As mentioned for jvm.cpp this Java code should do the isArray() and 
isPrimitive() check before calling the VM.


agreed.




Thanks,
Vicente

Re: [15] RFR: 8245241: Incorrect locale provider preference is not logged

[Joe Wang]

Hi Naoto,

I don't seem to see the DateFormat class or the getDateInstance methods 
specify how errors may be handled (or logged). Is that stated somewhere 
else?  In other cases, I see that you've changed it to throw 
ServiceConfigurationError, that looks to me may be better than a log as 
a configuration error  (or specifying wrong provider) sounds to me more 
severe than info.


Of the three HostLocaleProviderAdapterImpl, the one for unix is deleted, 
is there a specific reason?


Best,
Joe

On 5/20/2020 10:29 AM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issue:

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

The proposed changeset is located at:

https://cr.openjdk.java.net/~naoto/8245241/webrev.00/

Incorrect user-provided provider preference is supposed to be logged. 
However it is not so because it is using 
PlatformLogger(SurrogateLogger) which uses the default logging level. 
I changed it to use j.l.System's logger and bumped it to INFO level 
(it should notify the user by default). By taking this opportunity, I 
did some clean-up in locale provider adapter's logging related code as 
well.


Naoto

Re: Update Re: RFR JDK-8223347 Integration of Vector API (Incubator): Java API, implementation, and tests

[Ekaterina Pavlova]

On 5/20/20 10:29 AM, Paul Sandoz wrote:

Thanks, tier3 it is!


Great, now Vector API tests could be automatically run with default HS flags
as part of tier3 testing in mach5.



I am curious about how those tests can be co-opted for HS tiers. Can you share 
details on the panama-dev thread?


I would suggest just to add open/test/jdk/jdk/incubator/vector tests into 
corresponding HS compiler testing tiers
(mach5 tasks) which do run tests with -Xcomp and other compiler flags.
I can take care of this once Vector API is integrated.


-katya


Paul.

diff -r a606409980d6 test/jdk/TEST.groups
--- a/test/jdk/TEST.groups  Fri May 15 17:23:27 2020 -0700
+++ b/test/jdk/TEST.groups  Wed May 20 10:28:34 2020 -0700
@@ -72,6 +72,7 @@
  
  tier3 = \

  :build \
+:jdk_vector
  :jdk_rmi \
  :jdk_beans \
  :jdk_imageio \
@@ -338,6 +339,9 @@
  jdk_foreign = \
  java/foreign
  
+jdk_vector = \

+jdk/incubator/vector
+
  #
  
  #




On May 19, 2020, at 5:06 PM, Ekaterina Pavlova  
wrote:

As I wrote to openjdk alias tier3 seems to be more reasonable tier for 
incubating feature tests.

Once the tests will be integrated we will also need to add these tests into 
hs-comp tiers to be tested with
additional compiler flags (like -Xcomp).

-katya


On 5/19/20 3:25 PM, Paul Sandoz wrote:

I just realized that the vector tests have not been included in any JDK test 
category e.g. tier1.
Ordinarily I would expect the tests to be added to tier1 since the tests 
exercise intrinsics. However, those intrinsics are only enabled with the Vector 
API module so we could place in another tier to potentially reduce the cost of 
testing.
Advice very much appreciate on which tier the tests should belong to.
Paul.

On May 18, 2020, at 12:13 PM, Paul Sandoz  wrote:

HI,

Here’s an update of the API and implementation webrevs based on (mostly) API 
feedback. We have removed the performance tests to make the review easier 
(those will be dealt with separately to integration as a follow on task).

I think over the past year and recently via the CSR the API has had significant 
review. Reviews focusing on the Java implementation and tests would be greatly 
appreciated.

It’s worth reiterating that the implementation and tests are quite formulaic, 
there is a lot of code that is generated from templates, so it's possible to 
focus on the template + subset (e.g. byte, long, float, and sizes of say 256, 
max).

Notable changes from the prior webrev are:

- Removal of the fromValues methods to construct a vector from a varargs array. 
Feedback indicated this was a misleading way to obtain a vector.

- Unification of byte array and ByteBuffer load/store method signatures and 
unification of the implementations with more tests (including out-of-bounds 
tests for all the kinds of loads/stores).

Paul.

--

Latest javadoc
http://cr.openjdk.java.net/~psandoz/panama/vector-api-review/docs-2020-05-15-88a83f7238d8/api/jdk.incubator.vector/jdk/incubator/vector/package-summary.html
 


Latest specdiff
http://cr.openjdk.java.net/~psandoz/panama/vector-api-review/specdiff-jdk-2020-05-15-88a83f7238d8/overview-summary.html
 


Incremental specdiff
http://cr.openjdk.java.net/~psandoz/panama/vector-api-review/specdiff-2020-05-11-38dd763d023e-2020-05-15-88a83f7238d8/overview-summary.html
 



Latest implementation webrev
http://cr.openjdk.java.net/~psandoz/panama/JDK-8223347-vector-api-integration-java/jdk_src_webrev-2020-05-16-default-2020-05-15-88a83f7238d8/webrev/
 


Incremental Implementation webrev
http://cr.openjdk.java.net/~psandoz/panama/JDK-8223347-vector-api-integration-java/jdk_src_webrev-2020-05-11-38dd763d023e-2020-05-15-88a83f7238d8/webrev/
 


Latest test webrev
http://cr.openjdk.java.net/~psandoz/panama/JDK-8223347-vector-api-integration-java/jdk_test_webrev-2020-05-16-default-2020-05-15-88a83f7238d8/webrev/
 


Incremental test webrev
http://cr.openjdk.java.net/~psandoz/panama/JDK-8223347-vector-api-integration-java/jdk_test_webrev-2020-05-11-38dd763d023e-2020-05-15-88a83f7238d8/webrev/
 

[15] RFR: 8239480: Support for CLDR version 37

[naoto . sato]

Hello,

Please review the fix to the following issue:

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

The proposed changeset, and the release note draft are located at:

https://cr.openjdk.java.net/~naoto/8239480/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8243581

This updates the CLDR locale data to version 37. Although the changeset 
is huge, this mainly consists of the locale data and licenses changes. 
Other than those, only one test case was updated 
(IncludeLocalesPluginTest.java) due to the locale data change.


Naoto

Re: BigInteger.squareToLen() can be wrong (when called directly)

[Jeff Hain]

Hi.

>I filed https://bugs.openjdk.java.net/browse/JDK-8245038 to track this.

Can't seem to comment the bug so I post here.

>>The core issue is in implMulAdd(), where 'offset' is set to
>>"out.length-offset - 1"
>>instead of
>>"zlen - offset - 1"

There is another place that needs a fix for pure Java squareToLen()
to always work properly, it's in addOne(), where 'offset' is set to
"a.length-1-mlen-offset"
instead of
"zlen-1-mlen-offset"

I found a new test cases that allows to check both corrections:
x = [1, -2]
len = 2 (i.e. zlen = 4)
z.length = 5
z = [0, 1, -2, 0, 0] (no correction)
z = [0, 1, -6, 4, 0] (only correction 1)
z = [0, 3, -4, 0, 0] (only correction 2)
z = [0, 3, -8, 4, 0] (expected value)

I don't think there is a third place to correct:
this kind of thing is only done in these two places,
and I now checked this code through millions of random
magnitudes/values.

-Jeff

Re: RFR [15] 6394757: rev2: AbstractSet.removeAll semantics are surprisingly dependent on relative sizes

[Jason Mehrens]

Biasing towards ordering is covered in 
https://bugs.openjdk.java.net/browse/JDK-8181146 (read the markmail thread link 
in the ticket) and some of those changes worked there way into
http://hg.openjdk.java.net/jdk/jdk14/file/6c954123ee8d/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java#l1708
 and 
http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/main/java/util/concurrent/ConcurrentSkipListMap.java?r1=1.176&r2=1.177

I think there is some merit in even biasing toward ordering for non-sorted 
sets/maps to avoid expensive contains calls.  Any change we apply to equals we 
can usually apply a similar idea to containsAll.

I'm not sure about computing the hash code during equals.  It is not ideal but 
seems to solve the case you raised. I doubt it passes the compatibility bar for 
behavior or performance.

Jason


From: Alan Snyder 
Sent: Wednesday, May 20, 2020 1:53 PM
To: Jason Mehrens
Cc: core-libs-dev
Subject: Re: RFR [15] 6394757: rev2: AbstractSet.removeAll semantics are 
surprisingly dependent on relative sizes

Do you believe that Set.equals() should behave this way on SortedSets?

On May 20, 2020, at 11:23 AM, Jason Mehrens 
mailto:jason_mehr...@hotmail.com>> wrote:

public class SetEquals {

   public static void main(String[] args) {
   Comparator cc = String.CASE_INSENSITIVE_ORDER;
   Set s1 = new TreeSet<>(cc);
   Set s2 = new TreeSet<>(cc);
   s1.add("hello");
   s2.add("Hello");
   System.out.println(equals(s1, s2));
   System.out.println(equals(s2, s1));
   System.out.println(s1.hashCode() == s2.hashCode());
   }

   private static boolean equals(Set s1, Set s2) {
   if (s2 == null) {
   return false;
   }

   if (s1 == s2) {
   return true;
   }

   int s1h = 0;
   int s2h = 0;
   Iterator e1 = s1.iterator();
   Iterator e2 = s2.iterator();

   if (s1 instanceof SortedSet && s2 instanceof SortedSet
   && isSameOrder((SortedSet) s1, (SortedSet) s2)) {
   //TODO: raw type warnings.
   Comparator cmp = ((SortedSet) s1).comparator();
   if (cmp == null) {
   cmp = Comparator.naturalOrder();
   }
   while (e1.hasNext() && e2.hasNext()) {
   Object o1 = e1.next();
   Object o2 = e2.next();

   try {
   if (cmp.compare(o1, o2) != 0
   && (!s1.contains(o2) || !s2.contains(o1))) {
   return false;
   }
   } catch (ClassCastException cce) {
   return false;
   }

   s1h += Objects.hashCode(o1);
   s2h += Objects.hashCode(o2);

   //Iteration order should be the same.
   if (s1h != s2h) {
   return false;
   }
   }
   } else {
   while (e1.hasNext() && e2.hasNext()) {
   Object o1 = e1.next();
   Object o2 = e2.next();

   try {
   if (!Objects.equals(o1, o2)
   && (!s1.contains(o2) || !s2.contains(o1))) {
   return false;
   }
   } catch (ClassCastException cce) {
   return false;
   }

   s1h += Objects.hashCode(o1);
   s2h += Objects.hashCode(o2);
   }
   }
   return s1h == s2h && !e1.hasNext() && !e2.hasNext();
   }

   private static boolean isSameOrder(SortedSet s1, SortedSet s2) {
   //null matches natural ordering or unwrap to find a singleton.
   //Avoid calling equals on comparator.
   return Collections.reverseOrder(s1.comparator())
   == Collections.reverseOrder(s2.comparator());
   }
}

Re: RFR: JDK-8225056 VM support for sealed classes

[Harold Seigel]

Hi Remi,

Thank you for reviewing the ASM changes.

Harold

On 5/19/2020 4:41 AM, Remi Forax wrote:


- Mail original -

De: "David Holmes" 
À: "Harold David Seigel" , "hotspot-runtime-dev" 
,
"amber-dev" , "core-libs-dev" 
, "serviceability-dev"

Envoyé: Mardi 19 Mai 2020 07:26:38
Objet: Re: RFR: JDK-8225056 VM support for sealed classes
Hi Harold,

Adding serviceability-dev for the serviceability related changes.

Nit: "VM support for sealed classes"

This RFR covers the VM, core-libs, serviceability and even some
langtools tests. AFAICS only the javac changes are not included here so
when and where will they be reviewed and under what bug id? Ideally
there will be a single JBS issue for "Implementation of JEP 360: Sealed
types". It's okay to break up the RFRs across different areas, but it
should be done under one bug id.

Overall this looks good. I've looked at all files and mainly have some
style nits in various places. But there are some more significant items
below.

On 14/05/2020 7:09 am, Harold Seigel wrote:

Hi,

Please review this patch for JVM and Core-libs support for the JEP 360
Sealed Classes preview feature.  Code changes include the following:

   * Processing of the new PermittedSubclasses attribute to enforce that
     a class or interface, whose super is a sealed class or interface,
     must be listed in the super's PermittedSubclasses attribute.
   * Disallow redefinition of a sealed class or interface if its
     redefinition would change its PermittedSubclasses attribute.
   * Support API's to determine if a class or interface is sealed and, if
     it's sealed, return a list of its permitted subclasses.
   * asm support for the PermittedSubclasses attribute

I assume Remi is providing the upstream support in ASM? :) But also see
below ...


Currently the version of ASM used JDK already supports sealed classes but with 
the attribute named PermittedSubtypes instead of PermittedSubclasses.
This patch renames the attribute which is the right thing to do.
Then when JDK 15 will be released, we will release ASM 9 with full support of 
PermittedSubclasses, with the right method names, docs, etc, that will be then 
integrated in JDK 16.

So with my ASM hat, the changes to the 3 ASM classes are good.

Rémi

Re: Optimize (Linked-)HashMap lookup when backing array is null

[Claes Redestad]

I don't have a good intuition either, but this optimization feels like a
natural companion to the optimization to not allocate backing arrays for
empty maps in the first place - which seems to be a rather common
occurrence[1]. And since we'll not call hashCode() when the map is
empty, the gain might be more significant for classes that have a non-
caching hashCode function.

/Claes

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

On 2020-05-20 21:26, Martin Buchholz wrote:

No strong opinion either way, but ...
Generally we try to avoid special case code just for performance.
The value depends on how rare lookups on empty maps are; I don't have
a good intuition on that.

On Wed, May 20, 2020 at 12:11 PM Claes Redestad
 wrote:


Hi,

seems acceptable to me, too.

I'd be happy to sponsor this, but just starting a long weekend here so
might not have time before start of next week.

/Claes

On 2020-05-19 22:21, Roger Riggs wrote:

Hi,

Right, its only in the case of removing the node because the new value
was null.
Besides being infrequent, that should not be a problem.

Thanks, Roger


On 5/19/20 3:56 PM, Christoph Dreis wrote:

Hi Roger,


How does the performance degrade when the computation of the hash
is non-trivial. For example, with an array as a key or an object with
fields that are objects?
The original code made a point of computing the hash only once.

HashMap.get() and HashMap.containsKey() etc. would still calculate it
only once.
Only computeIfPresent would degrade. Did you mean that?

Cheers,
Christoph

On 5/19/20 3:21 PM, Claes Redestad wrote:

Thanks for producing the simpler variant and getting some comparative
runs done.

On 2020-05-19 19:50, Christoph Dreis wrote:

I would probably go for the first version although it is a bit more
complicated and has the computeIfPresent caveat, because it doesn't
slow down the common Map.get() case when the map is actually filled.

It is a bit of step up in complexity, but getting the win without
adding
a branch to Map.get() _is_ a nice property.


Let me know what you think.

If core libs maintainers are fine with the added complexity of your
original patch, I think it looks ok.

/Claes

Re: Optimize (Linked-)HashMap lookup when backing array is null

[Martin Buchholz]

No strong opinion either way, but ...
Generally we try to avoid special case code just for performance.
The value depends on how rare lookups on empty maps are; I don't have
a good intuition on that.

On Wed, May 20, 2020 at 12:11 PM Claes Redestad
 wrote:
>
> Hi,
>
> seems acceptable to me, too.
>
> I'd be happy to sponsor this, but just starting a long weekend here so
> might not have time before start of next week.
>
> /Claes
>
> On 2020-05-19 22:21, Roger Riggs wrote:
> > Hi,
> >
> > Right, its only in the case of removing the node because the new value
> > was null.
> > Besides being infrequent, that should not be a problem.
> >
> > Thanks, Roger
> >
> >
> > On 5/19/20 3:56 PM, Christoph Dreis wrote:
> >> Hi Roger,
> >>
> >>> How does the performance degrade when the computation of the hash
> >>> is non-trivial. For example, with an array as a key or an object with
> >>> fields that are objects?
> >>> The original code made a point of computing the hash only once.
> >> HashMap.get() and HashMap.containsKey() etc. would still calculate it
> >> only once.
> >> Only computeIfPresent would degrade. Did you mean that?
> >>
> >> Cheers,
> >> Christoph
> >>
> >> On 5/19/20 3:21 PM, Claes Redestad wrote:
>  Thanks for producing the simpler variant and getting some comparative
>  runs done.
> 
>  On 2020-05-19 19:50, Christoph Dreis wrote:
> > I would probably go for the first version although it is a bit more
> > complicated and has the computeIfPresent caveat, because it doesn't
> > slow down the common Map.get() case when the map is actually filled.
>  It is a bit of step up in complexity, but getting the win without
>  adding
>  a branch to Map.get() _is_ a nice property.
> 
> > Let me know what you think.
>  If core libs maintainers are fine with the added complexity of your
>  original patch, I think it looks ok.
> 
>  /Claes
> >>
> >>
> >

Re: Optimize (Linked-)HashMap lookup when backing array is null

[Claes Redestad]

Hi,

seems acceptable to me, too.

I'd be happy to sponsor this, but just starting a long weekend here so
might not have time before start of next week.

/Claes

On 2020-05-19 22:21, Roger Riggs wrote:

Hi,

Right, its only in the case of removing the node because the new value 
was null.

Besides being infrequent, that should not be a problem.

Thanks, Roger


On 5/19/20 3:56 PM, Christoph Dreis wrote:

Hi Roger,


How does the performance degrade when the computation of the hash
is non-trivial. For example, with an array as a key or an object with
fields that are objects?
The original code made a point of computing the hash only once.
HashMap.get() and HashMap.containsKey() etc. would still calculate it 
only once.

Only computeIfPresent would degrade. Did you mean that?

Cheers,
Christoph

On 5/19/20 3:21 PM, Claes Redestad wrote:

Thanks for producing the simpler variant and getting some comparative
runs done.

On 2020-05-19 19:50, Christoph Dreis wrote:

I would probably go for the first version although it is a bit more
complicated and has the computeIfPresent caveat, because it doesn't
slow down the common Map.get() case when the map is actually filled.
It is a bit of step up in complexity, but getting the win without 
adding

a branch to Map.get() _is_ a nice property.


Let me know what you think.

If core libs maintainers are fine with the added complexity of your
original patch, I think it looks ok.

/Claes

Re: RFR [15] 6394757: rev2: AbstractSet.removeAll semantics are surprisingly dependent on relative sizes

[Alan Snyder]

Do you believe that Set.equals() should behave this way on SortedSets?

> On May 20, 2020, at 11:23 AM, Jason Mehrens  wrote:
> 
> public class SetEquals {
> 
>public static void main(String[] args) {
>Comparator cc = String.CASE_INSENSITIVE_ORDER;
>Set s1 = new TreeSet<>(cc);
>Set s2 = new TreeSet<>(cc);
>s1.add("hello");
>s2.add("Hello");
>System.out.println(equals(s1, s2));
>System.out.println(equals(s2, s1));
>System.out.println(s1.hashCode() == s2.hashCode());
>}
> 
>private static boolean equals(Set s1, Set s2) {
>if (s2 == null) {
>return false;
>}
> 
>if (s1 == s2) {
>return true;
>}
> 
>int s1h = 0;
>int s2h = 0;
>Iterator e1 = s1.iterator();
>Iterator e2 = s2.iterator();
> 
>if (s1 instanceof SortedSet && s2 instanceof SortedSet
>&& isSameOrder((SortedSet) s1, (SortedSet) s2)) {
>//TODO: raw type warnings.
>Comparator cmp = ((SortedSet) s1).comparator();
>if (cmp == null) {
>cmp = Comparator.naturalOrder();
>}
>while (e1.hasNext() && e2.hasNext()) {
>Object o1 = e1.next();
>Object o2 = e2.next();
> 
>try {
>if (cmp.compare(o1, o2) != 0
>&& (!s1.contains(o2) || !s2.contains(o1))) {
>return false;
>}
>} catch (ClassCastException cce) {
>return false;
>}
> 
>s1h += Objects.hashCode(o1);
>s2h += Objects.hashCode(o2);
> 
>//Iteration order should be the same.
>if (s1h != s2h) {
>return false;
>}
>}
>} else {
>while (e1.hasNext() && e2.hasNext()) {
>Object o1 = e1.next();
>Object o2 = e2.next();
> 
>try {
>if (!Objects.equals(o1, o2)
>&& (!s1.contains(o2) || !s2.contains(o1))) {
>return false;
>}
>} catch (ClassCastException cce) {
>return false;
>}
> 
>s1h += Objects.hashCode(o1);
>s2h += Objects.hashCode(o2);
>}
>}
>return s1h == s2h && !e1.hasNext() && !e2.hasNext();
>}
> 
>private static boolean isSameOrder(SortedSet s1, SortedSet s2) {
>//null matches natural ordering or unwrap to find a singleton.
>//Avoid calling equals on comparator.
>return Collections.reverseOrder(s1.comparator())
>== Collections.reverseOrder(s2.comparator());
>}
> }

Re: RFR [15] 6394757: rev2: AbstractSet.removeAll semantics are surprisingly dependent on relative sizes

[Jason Mehrens]

Alan,

This contains that and that contains this at least avoids calling size which 
could be expensive or ephemeral at the AbstractSet level.  Using the iterators 
you can at least do a final check to see there are no more elements to walk.   
Biasing towards ordering could pay off for things like naturally ordered 
SortedSet to SortedSet, LinkedHashSet, or CopyOnWriteArraySet.

Computing the hash during equals is about the best I can come up with:  

public class SetEquals {

public static void main(String[] args) {
Comparator cc = String.CASE_INSENSITIVE_ORDER;
Set s1 = new TreeSet<>(cc);
Set s2 = new TreeSet<>(cc);
s1.add("hello");
s2.add("Hello");
System.out.println(equals(s1, s2));
System.out.println(equals(s2, s1));
System.out.println(s1.hashCode() == s2.hashCode());
}

private static boolean equals(Set s1, Set s2) {
if (s2 == null) {
return false;
}

if (s1 == s2) {
return true;
}

int s1h = 0;
int s2h = 0;
Iterator e1 = s1.iterator();
Iterator e2 = s2.iterator();

if (s1 instanceof SortedSet && s2 instanceof SortedSet
&& isSameOrder((SortedSet) s1, (SortedSet) s2)) {
//TODO: raw type warnings.
Comparator cmp = ((SortedSet) s1).comparator();
if (cmp == null) {
cmp = Comparator.naturalOrder();
}
while (e1.hasNext() && e2.hasNext()) {
Object o1 = e1.next();
Object o2 = e2.next();

try {
if (cmp.compare(o1, o2) != 0
&& (!s1.contains(o2) || !s2.contains(o1))) {
return false;
}
} catch (ClassCastException cce) {
return false;
}

s1h += Objects.hashCode(o1);
s2h += Objects.hashCode(o2);

//Iteration order should be the same.
if (s1h != s2h) {
return false;
}
}
} else {
while (e1.hasNext() && e2.hasNext()) {
Object o1 = e1.next();
Object o2 = e2.next();

try {
if (!Objects.equals(o1, o2)
&& (!s1.contains(o2) || !s2.contains(o1))) {
return false;
}
} catch (ClassCastException cce) {
return false;
}

s1h += Objects.hashCode(o1);
s2h += Objects.hashCode(o2);
}
}
return s1h == s2h && !e1.hasNext() && !e2.hasNext();
}

private static boolean isSameOrder(SortedSet s1, SortedSet s2) {
//null matches natural ordering or unwrap to find a singleton.
//Avoid calling equals on comparator.
return Collections.reverseOrder(s1.comparator())
== Collections.reverseOrder(s2.comparator());
}
}

Jason

From: Alan Snyder 
Sent: Thursday, May 14, 2020 8:39 AM
To: Jason Mehrens
Cc: core-libs-dev
Subject: Re: RFR [15] 6394757: rev2: AbstractSet.removeAll semantics are 
surprisingly dependent on relative sizes

> HashSet/TreeSet could do what ConcurrentHashMap/ConcurrentSkipListSet do by 
> using the "this contains that and that contains this" logic.

Yes, at the cost of yet another performance regression.

But how about this problem:

Comparator cc = String.CASE_INSENSITIVE_ORDER;
Set s1 = new TreeSet<>(cc);
Set s2 = new TreeSet<>(cc);
s1.add("hello");
s2.add("Hello");
s1.equals(s2) -> true
s2.equals(s1) -> true
s1.hashCode() == s2.hashCode() -> false

Re: RFR 8243491: Implementation of Foreign-Memory Access API (Second Incubator)

[Chris Hegarty]

> On 20 May 2020, at 15:01, Maurizio Cimadamore 
>  wrote:
> 
> Another very small iteration which fixes a gap in the javadoc specification 
> for MemorySegment::toArray (thanks Chris!)
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/8243491_v5/webrev
> 
> Delta webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/8243491_v5/webrev_delta/

Still LGTM.

-Chris.

[15] RFR: 8245241: Incorrect locale provider preference is not logged

[naoto . sato]

Hello,

Please review the fix to the following issue:

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

The proposed changeset is located at:

https://cr.openjdk.java.net/~naoto/8245241/webrev.00/

Incorrect user-provided provider preference is supposed to be logged. 
However it is not so because it is using PlatformLogger(SurrogateLogger) 
which uses the default logging level. I changed it to use j.l.System's 
logger and bumped it to INFO level (it should notify the user by 
default). By taking this opportunity, I did some clean-up in locale 
provider adapter's logging related code as well.


Naoto

Re: Update Re: RFR JDK-8223347 Integration of Vector API (Incubator): Java API, implementation, and tests

[Paul Sandoz]

Thanks, tier3 it is!

I am curious about how those tests can be co-opted for HS tiers. Can you share 
details on the panama-dev thread?

Paul.

diff -r a606409980d6 test/jdk/TEST.groups
--- a/test/jdk/TEST.groups  Fri May 15 17:23:27 2020 -0700
+++ b/test/jdk/TEST.groups  Wed May 20 10:28:34 2020 -0700
@@ -72,6 +72,7 @@
 
 tier3 = \
 :build \
+:jdk_vector
 :jdk_rmi \
 :jdk_beans \
 :jdk_imageio \
@@ -338,6 +339,9 @@
 jdk_foreign = \
 java/foreign
 
+jdk_vector = \
+jdk/incubator/vector
+
 #
 
 #


> On May 19, 2020, at 5:06 PM, Ekaterina Pavlova  
> wrote:
> 
> As I wrote to openjdk alias tier3 seems to be more reasonable tier for 
> incubating feature tests.
> 
> Once the tests will be integrated we will also need to add these tests into 
> hs-comp tiers to be tested with
> additional compiler flags (like -Xcomp).
> 
> -katya
> 
> 
> On 5/19/20 3:25 PM, Paul Sandoz wrote:
>> I just realized that the vector tests have not been included in any JDK test 
>> category e.g. tier1.
>> Ordinarily I would expect the tests to be added to tier1 since the tests 
>> exercise intrinsics. However, those intrinsics are only enabled with the 
>> Vector API module so we could place in another tier to potentially reduce 
>> the cost of testing.
>> Advice very much appreciate on which tier the tests should belong to.
>> Paul.
>>> On May 18, 2020, at 12:13 PM, Paul Sandoz  wrote:
>>> 
>>> HI,
>>> 
>>> Here’s an update of the API and implementation webrevs based on (mostly) 
>>> API feedback. We have removed the performance tests to make the review 
>>> easier (those will be dealt with separately to integration as a follow on 
>>> task).
>>> 
>>> I think over the past year and recently via the CSR the API has had 
>>> significant review. Reviews focusing on the Java implementation and tests 
>>> would be greatly appreciated.
>>> 
>>> It’s worth reiterating that the implementation and tests are quite 
>>> formulaic, there is a lot of code that is generated from templates, so it's 
>>> possible to focus on the template + subset (e.g. byte, long, float, and 
>>> sizes of say 256, max).
>>> 
>>> Notable changes from the prior webrev are:
>>> 
>>> - Removal of the fromValues methods to construct a vector from a varargs 
>>> array. Feedback indicated this was a misleading way to obtain a vector.
>>> 
>>> - Unification of byte array and ByteBuffer load/store method signatures and 
>>> unification of the implementations with more tests (including out-of-bounds 
>>> tests for all the kinds of loads/stores).
>>> 
>>> Paul.
>>> 
>>> --
>>> 
>>> Latest javadoc
>>> http://cr.openjdk.java.net/~psandoz/panama/vector-api-review/docs-2020-05-15-88a83f7238d8/api/jdk.incubator.vector/jdk/incubator/vector/package-summary.html
>>>  
>>> 
>>> 
>>> Latest specdiff
>>> http://cr.openjdk.java.net/~psandoz/panama/vector-api-review/specdiff-jdk-2020-05-15-88a83f7238d8/overview-summary.html
>>>  
>>> 
>>> 
>>> Incremental specdiff
>>> http://cr.openjdk.java.net/~psandoz/panama/vector-api-review/specdiff-2020-05-11-38dd763d023e-2020-05-15-88a83f7238d8/overview-summary.html
>>>  
>>> 
>>> 
>>> 
>>> Latest implementation webrev
>>> http://cr.openjdk.java.net/~psandoz/panama/JDK-8223347-vector-api-integration-java/jdk_src_webrev-2020-05-16-default-2020-05-15-88a83f7238d8/webrev/
>>>  
>>> 
>>> 
>>> Incremental Implementation webrev
>>> http://cr.openjdk.java.net/~psandoz/panama/JDK-8223347-vector-api-integration-java/jdk_src_webrev-2020-05-11-38dd763d023e-2020-05-15-88a83f7238d8/webrev/
>>>  
>>> 
>>> 
>>> Latest test webrev
>>> http://cr.openjdk.java.net/~psandoz/panama/JDK-8223347-vector-api-integration-java/jdk_test_webrev-2020-05-16-default-2020-05-15-88a83f7238d8/webrev/
>>>  
>>> 
>>> 
>>> Incremental test webrev
>>> http://cr.openjdk.java.net/~psandoz/panama/JDK-8223347-vector-api-integration-java/jdk_test_webrev-2020-05-11-38dd763d023e-2020-05-15-88a83f7238d8/webrev/
>>>  
>>> 

Re: RFR: JDK-8225056 VM support for sealed classes

[Mandy Chung]

Hi Vicente,

On 5/20/20 8:40 AM, Vicente Romero wrote:

Hi David,


src/java.base/share/classes/java/lang/Class.java

There needs to be a CSR request for these changes.


yes there is one already: 
https://bugs.openjdk.java.net/browse/JDK-8244556


Adding to David's comment w.r.t. @throws IAE:

The Class::getXXX APIs returns `Class` or `Class[]` if the result is 
about type(s).  This new `getPermittedSubclasses` returns 
`ClassDesc[]`.   I wonder if this should be renamed to 
`permittedSubclasses` to follow the convention of the new APIs added to 
support descriptors e.g. `describeConstable`


Nit: {@linkplain Class} should be {@code Class}

Mandy

Re: RFR 8243491: Implementation of Foreign-Memory Access API (Second Incubator)

[Paul Sandoz]

+1
Paul.

> On May 20, 2020, at 7:01 AM, Maurizio Cimadamore 
>  wrote:
> 
> Another very small iteration which fixes a gap in the javadoc specification 
> for MemorySegment::toArray (thanks Chris!)
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/8243491_v5/webrev
> 
> Delta webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/8243491_v5/webrev_delta/
> 
> 
> Cheers
> Maurizio

Re: RFR: JDK-8225056 VM support for sealed classes

[Vicente Romero]

Hi David,


src/java.base/share/classes/java/lang/Class.java

There needs to be a CSR request for these changes.


yes there is one already: https://bugs.openjdk.java.net/browse/JDK-8244556


+  * Returns an array containing {@code ClassDesc} objects 
representing all the
+  * permitted subclasses of this {@linkplain Class} if it is 
sealed. Returns an empty array if this

+  * {@linkplain Class} is not sealed.

should add "or this class represents an array or primitive type" 
(using the standard wording for such cases).


well given that array and primitive classes are not sealed classes I 
think we are already covered by the method's spec.




+  * @throws IllegalArgumentException if a class descriptor is not 
in the correct format


IllegalArgumentException is not an appropriate exception to use as 
this method takes no arguments. If the class descriptor is not valid 
and it comes from the VM then I think we have a problem with how the 
VM validates class descriptors. Any IAE from ClassDesc.of should be 
caught and converted to a more suitable exception type - preferably 
InternalError if the VM should always return valid strings.


we agree with you here, this will be fixed in the next review iteration.



+ public ClassDesc[] getPermittedSubclasses() {

As mentioned for jvm.cpp this Java code should do the isArray() and 
isPrimitive() check before calling the VM.


agreed.




Thanks,
Vicente

Re: RFR: 8215401: Add isEmpty default method to CharSequence

[Claes Redestad]

On 2020-05-20 16:10, Alan Bateman wrote:



On 19/05/2020 20:35, Claes Redestad wrote:

Hi Jason,

I guess overriding and marking the CharBuffer method final for
consistency wouldn't hurt. Except I probably need to pass this through
another CSR review.

Also added test cases for char[] and String-based CharBuffers:

http://cr.openjdk.java.net/~redestad/8215401/open.01/


Yes, CharBuffer sometimes needs attention default methods are added to 
CharSequence. In this case it shouldn't need anything but overriding the 
javadoc is okay (just missing @since 15).


Ok - I'll also update and resubmit the CSR with the edits and additions
that came up during review here.

For the test then you could expand it to test the views of heap and 
direct buffers (asCharBuffer).


How about this:

http://cr.openjdk.java.net/~redestad/8215401/open.02/

/Claes

Re: RFR (CSR) JDK-8245399 Remove addition preview adornment from String::formatted

[Jim Laskey]

Disregard. I have two bugs against String.java and overlapped the email.

> On May 20, 2020, at 8:54 AM, Jim Laskey  wrote:
> 
> All call for aid response to a comment on the CSR from Joe.
> 
>> I suggesting having some explicit statement about the line count in addition 
>> to the number of line terminators.
> 
> 
> That is the crux of the skirting around. Line count can mean two things 
> depending on whether lines are interpreted as a) character sequences 
> separated by line terminators (\r, \n) or b) character sequences terminated 
> by line terminators. If we state one thing a) then a developer will state, no 
> it's b). It all depends on what the developer does next.
> 
> Generally, core libraries (String::split, String::lines, String::stripIndent) 
> interpret lines using a) with the option of having a final line terminator 
> which then kind of looks like b). String::lines tries to make this clear, 
> "stream of lines extracted from this string, separated by line terminators.", 
> but you can still have the optional final line terminator (as does 
> String::split("\\R")).
> 
> Files::lines and BufferedReader::lines side-step the issue all together 
> (assumptions left to the user.) Any wordsmything assistance would be 
> appreciated,
> 
> -- Jim
> 
> 
> 
> 
>> On May 19, 2020, at 5:00 PM, Jim Laskey  wrote:
>> 
>> Please review this change to remove the preview heading from the javadoc of 
>> String::formatted. This also updates the @since to 15.
>> 
>> Thank you.
>> 
>> Cheers,
>> 
>> -- Jim
>> 
>> 
>> csr: https://bugs.openjdk.java.net/browse/JDK-8245399 
>> 
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8245398 
>> 
>> 
>> 
> 

Re: RFR: 8215401: Add isEmpty default method to CharSequence

[Alan Bateman]

On 19/05/2020 20:35, Claes Redestad wrote:

Hi Jason,

I guess overriding and marking the CharBuffer method final for
consistency wouldn't hurt. Except I probably need to pass this through
another CSR review.

Also added test cases for char[] and String-based CharBuffers:

http://cr.openjdk.java.net/~redestad/8215401/open.01/


Yes, CharBuffer sometimes needs attention default methods are added to 
CharSequence. In this case it shouldn't need anything but overriding the 
javadoc is okay (just missing @since 15).


For the test then you could expand it to test the views of heap and 
direct buffers (asCharBuffer).


-Alan

Re: RFR 8243491: Implementation of Foreign-Memory Access API (Second Incubator)

[Maurizio Cimadamore]

Another very small iteration which fixes a gap in the javadoc 
specification for MemorySegment::toArray (thanks Chris!)


Webrev:

http://cr.openjdk.java.net/~mcimadamore/8243491_v5/webrev

Delta webrev:

http://cr.openjdk.java.net/~mcimadamore/8243491_v5/webrev_delta/


Cheers
Maurizio

On 13/05/2020 13:12, Maurizio Cimadamore wrote:
Another iteration which addresses the latest CSR comments (the CSR has 
now been approved):


* make MemorySegment::withAccessModes/hasAccessMode throw 
IllegalArgumentException in cases where the provided mask is invalid 
(this required a test tweak)
* sprinkled a couple of references to the JLS in the package javadoc, 
as per CSR suggestions
* Fixed the ParallelSum::findAny_bulk benchmarks, which were 
(erroneously) not testing all the elements in the segment


Webrev:

http://cr.openjdk.java.net/~mcimadamore/8243491_v4/webrev

Delta webrev:

http://cr.openjdk.java.net/~mcimadamore/8243491_v4/webrev_delta/


Cheers
Maurizio

On 01/05/2020 12:06, Maurizio Cimadamore wrote:

Latest iteration - the follow issues were addressed:

* fix a bug with alignment of native segments triggering spurious 
failures (contributed by Jorn)

* fix javadoc and tests for access modes (contributed by Chris)
* added benchmarks for Stream::findAny using segment spliterator 
(contributed by Peter)

* addressed CSR comments

Webrev:

http://cr.openjdk.java.net/~mcimadamore/8243491_v3/webrev

Delta webrev:

http://cr.openjdk.java.net/~mcimadamore/8243491_v3/webrev_delta/

Javadoc:

http://cr.openjdk.java.net/~mcimadamore/8243491_v3/javadoc

Specdiff:

http://cr.openjdk.java.net/~mcimadamore/8243491_v3/specdiff/overview-summary.html 




Cheers
Maurizio

On 27/04/2020 13:13, Maurizio Cimadamore wrote:

Another iteration, which addresses the following issues:

* wrong copyright headers in certain tests
* issue with TestNative when running in debug mode caused by 
mismatched malloc/os::free (contributed by Jorn)

* clarify javadoc of MemoryHandles::withStride
* improved implementation of MemoryAccessVarHandleGenerator to use 
hidden classes rather than Unsafe.dAC (contributed by Mandy)



Webrev:

http://cr.openjdk.java.net/~mcimadamore/8243491_v2/webrev

Delta webrev:

http://cr.openjdk.java.net/~mcimadamore/8243491_v2/webrev_delta/

Javadoc:

http://cr.openjdk.java.net/~mcimadamore/8243491_v2/javadoc

Specdiff:

http://cr.openjdk.java.net/~mcimadamore/8243491_v2/specdiff/overview-summary.html 




Cheers
Maurizio


On 23/04/2020 21:33, Maurizio Cimadamore wrote:

Hi,
time has come for another round of foreign memory access API 
incubation (see JEP 383 [3]). This iteration aims at polishing some 
of the rough edges of the API, and adds some of the functionalities 
that developers have been asking for during this first round of 
incubation. The revised API tightens the thread-confinement 
constraints (by removing the MemorySegment::acquire method) and 
instead provides more targeted support for parallel computation via 
a segment spliterator. The API also adds a way to create a custom 
native segment; this is, essentially, an unsafe API point, very 
similar in spirit to the JNI NewDirectByteBuffer functionality [1]. 
By using this bit of API,  power-users will be able to add support, 
via MemorySegment, to *their own memory sources* (e.g. think of a 
custom allocator written in C/C++). For now, this API point is 
called off as "restricted" and a special read-only JDK property 
will have to be set on the command line for calls to this method to 
succeed. We are aware there's no precedent for something like this 
in the Java SE API - but if Project Panama is to remain true about 
its ultimate goal of replacing bits of JNI code with (low level) 
Java code, stuff like this has to be *possible*. We anticipate 
that, at some point, this property will become a true launcher 
flag, and that the foreign restricted machinery will be integrated 
more neatly into the module system.


A list of the API, implementation and test changes is provided 
below. If you have any questions, or need more detailed 
explanations, I (and the rest of the Panama team) will be happy to 
point at existing discussions, and/or to provide the feedback 
required.


Thanks
Maurizio

Webrev:

http://cr.openjdk.java.net/~mcimadamore/8243491_v1/webrev

Javadoc:

http://cr.openjdk.java.net/~mcimadamore/8243491_v1/javadoc

Specdiff:

http://cr.openjdk.java.net/~mcimadamore/8243491_v1/specdiff/overview-summary.html 



CSR:

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



API changes
===

* MemorySegment
  - drop support for acquire() method - in its place now you can 
obtain a spliterator from a segment, which supports divide-and-conquer
  - revamped support for views - e.g. isReadOnly - now segments 
have access modes
  - added API to do serial confinement hand-off 
(MemorySegment::withOwnerThread)
  - added unsafe factory to construct a native segment out of an 
existing address; this API is "restricted" and only available if 
th

Re: RFR (CSR) JDK-8245399 Remove addition preview adornment from String::formatted

[Jim Laskey]

All call for aid response to a comment on the CSR from Joe.

> I suggesting having some explicit statement about the line count in addition 
> to the number of line terminators.


That is the crux of the skirting around. Line count can mean two things 
depending on whether lines are interpreted as a) character sequences separated 
by line terminators (\r, \n) or b) character sequences terminated by line 
terminators. If we state one thing a) then a developer will state, no it's b). 
It all depends on what the developer does next.

Generally, core libraries (String::split, String::lines, String::stripIndent) 
interpret lines using a) with the option of having a final line terminator 
which then kind of looks like b). String::lines tries to make this clear, 
"stream of lines extracted from this string, separated by line terminators.", 
but you can still have the optional final line terminator (as does 
String::split("\\R")).

Files::lines and BufferedReader::lines side-step the issue all together 
(assumptions left to the user.) Any wordsmything assistance would be 
appreciated,

-- Jim




> On May 19, 2020, at 5:00 PM, Jim Laskey  wrote:
> 
> Please review this change to remove the preview heading from the javadoc of 
> String::formatted. This also updates the @since to 15.
> 
> Thank you.
> 
> Cheers,
> 
> -- Jim
> 
> 
> csr: https://bugs.openjdk.java.net/browse/JDK-8245399 
> 
> jbs: https://bugs.openjdk.java.net/browse/JDK-8245398 
> 
> 
>