Re: RFR(XS): 8226286 Remove unused method java.lang.Integer::formatUnsignedInt

[Tagir Valeev]

Hello, Brian!

Thank you for review. Here's updated version:
http://cr.openjdk.java.net/~tvaleev/webrev/8226286/r2/

I left dangling javadoc in-place in the same way as it's done in
> java.lang.Long.
>
>
> I don’t know whether this was intentional per some policy or an oversight.
>

Seems it was an oversight. Now I properly attached javadoc to both methods,
replaced "character buffer" with "byte buffer", added "(LATIN1 version)",
"(UTF16 version)" and did the same changes in java.lang.Long. I also made
formatUnsignedInt and formatUnsignedLong0 private (they were
package-private),
because they are not called from other classes and UTF16 methods are
already
declared as private.


> Also I removed redundant reassignment of parameter like s =
> Objects.requireNonNull(s).
>
> By the way I noticed that remaining methods formatUnsignedInt
> and formatUnsignedIntUTF16 always accept offset = 0. Probably it would be
> good to inline offset parameter?
>
>
> Yes at it is always zero.
>

Also done. In Long class offsets are used for UUID generation, so this
change was
applied to Integer only.

I updated the issue description to reflect all the changes made. Probably I
should
add some tags to the issue to indicate that it's a cleanup only?

With best regards,
Tagir Valeev.

The final optimized version of Dual-Pivot Quicksort (ver.19.1)

[Vladimir Yaroslavskiy]

Hi team,

Please review the final optimized version of Dual-Pivot Quicksort (ver.19.1),
see http://cr.openjdk.java.net/~alanb/8226297/0/webrev
(link to Jira task https://bugs.openjdk.java.net/browse/JDK-8226297)

The new version was published here more than one year ago (on Jan 19, 2018).
Since that time Dual-Pivot Quicksort has been significantly improved
and this work was done in collaboration with Doug Lea and Laurent Bourges.

You can find summary of all changes below.

DualPivotQuicksort.java
--
* The 1-st and 5-th candidates are taken as pivots
  instead of 2-nd and 4-th
* Pivots are chosen with another step
* Pivot candidates are sorted by combination of
  5-element network sorting and insertion sort
* New backwards partitioning was introduced
* Partitioning is related to the golden ratio
* Quicksort tuning parameters were updated
* Thresholds for byte / char / short sorting were updated
* Additional steps for float / double were fully rewritten
* Parallel sorting is based on combination of merge sort
  and Dual-Pivot Quicksort
* Pair insertion sort was optimized and became a part
  of mixed insertion sort
* Mixed insertion sort was introduced: combination
  of simple insertion sort, pin insertion sort and
  pair insertion sort
* Simple insertion sort is invoked on tiny array
* Pin insertion sort is started on small array
* Pair insertion sort is continued on remain part
* Merging of runs is invoked on each iteration of Quicksort
* Merging of runs was fully rewritten
* Optimal partitioning of merging is used
* Not more than one copy of part to buffer during merging
* Merging parameters were updated
* Initial capacity of runs is based on size
* Max number of runs is constant
* Optimized version of heap sort was introduced
* Heap sort is used as a guard against quadratic time
  (int / long / float / double)
* Parallel merging of runs was also provided
* Parallel sorting, heap sort and merging of runs
  are not used in byte / char / short sorting
* Counting sort for byte / char / short were optimized
* Counting sort is used as a guard against quadratic time
  (byte / char / short)
* Code style and javadoc improvements

Sorting.java
--
* New test cases were added
* Test cases are invoked for both: sequential and
  parallel sorting
* Check for Object sorting was added
* New tests were added against heap sort
* Added test case against bug in parallel merge
  sort on float / double data

ParallelSorting.java
--
* This class was removed, because Sorting class
  covers all cases

SortingHelper.java 
--
* The helper class provides access package-private
  methods of DualPivotQuicksort class during testing

Arrays.java
--
* Calls of Dual-Pivot Quicksort were updated
* Parallel sorting of primitives was switched to parallel
  Dual-Pivot Quicksort
* Javadoc was updated, double spaces were removed
* Format changes

ArraysParallelSortHelpers.java
--
* Implementation of parallel sorting
  for primitives was removed
* Javadoc was updated

Thank you,
Vladimir

Re: [14]RFR: 8220229: Timezone pattern "OOOO" does not result in the full "GMT+00:00" substring

[Lance Andersen]

Hi Naoto,

It reads OK.  I will add myself as a CSR reviewer also

> On Jun 18, 2019, at 3:37 PM, naoto.s...@oracle.com wrote:
> 
> Hello,
> 
> Please review the fix to the following issue:
> 
> https://bugs.openjdk.java.net/browse/JDK-8220229
> 
> The proposed changeset and CSR are located at:
> 
> https://cr.openjdk.java.net/~naoto/8220229/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8226308
> 
> This is to simply clarify the cases for zero offset.
> 
> Naoto

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

[14]RFR: 8220229: Timezone pattern "OOOO" does not result in the full "GMT+00:00" substring

[naoto . sato]

Hello,

Please review the fix to the following issue:

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

The proposed changeset and CSR are located at:

https://cr.openjdk.java.net/~naoto/8220229/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8226308

This is to simply clarify the cases for zero offset.

Naoto

Re: RFR [14] 8226303: Examine the HttpRequest.BodyPublishers for exception handling

[Daniel Fuchs]

Hi Pavel,

Mostly looks good to me.

One remark about the test:

 107 CompletableFuture future() {
 108 return f.copy();
 109 }

I had to read the javadoc to convince myself that this was OK ;-)

If the existing tests all pass reliably you have my review.

best regards,

-- daniel

On 18/06/2019 14:22, Pavel Rappo wrote:

Hello,

Please review the following change:

  http://cr.openjdk.java.net/~prappo/8226303/webrev.00

This change increases robustness of convenience BodyPublishers by going the
extra mile and ensuring unlikely thrown exceptions are relayed downstream.

Thanks,
-Pavel

Re: RFR (T): 8226203: MappedByteBuffer.force method may have no effect on implementation specific map modes

[Alan Bateman]

On 18/06/2019 17:15, Andrew Dinn wrote:

Could I please have a review for the following trivial change to the
javadoc for the two MappedByteBuffer.force methods:

   JIRA:   https://bugs.openjdk.java.net/browse/JDK-8226203
   webrev: http://cr.openjdk.java.net/~adinn/8226203/webrev.00/

It allows for the possibility of failures when extended map modes are
used to create the MappedByteBuffer.

The case documented here will not actually arise in JDK13 since nothing
in the JDK creates, let alone exposes extended map modes. However, it
would be better if the javadoc reflected that possibility. Is it still
possible to push this? If that is a problem then I am happy for it to go
into jdk14.

This looks good. Will you create a CSR for this? I think it can be fixed 
in jdk/jdk13 as it follows JDK-8221397 and JDK-8221696 (and there is no 
risk as it's javadoc only).


-Alan

Re: RFR [14] 8226303: Examine the HttpRequest.BodyPublishers for exception handling

[Chris Hegarty]

Pavel,

> On 18 Jun 2019, at 17:56, Pavel Rappo  wrote:
> 
> Hi Chris,
> 
> Thanks for adding net-dev. I should've posted there in the first place, but
> something went wrong. As for your comments:
> 
> 1) I will add this before pushing the change:
> 
>* @summary Verifies that some of the standard BodyPublishers relay 
> exception
>*  rather than throw it
>* @bug 8226303
> 
> Is that okay?

Yes.

> 
> 2) Done.
> 
> Thanks.

-Chris.

>> On 18 Jun 2019, at 16:07, Chris Hegarty  wrote:
>> 
>> [ adding net-dev ]
>> 
>> Pavel,
>> 
>>> On 18 Jun 2019, at 14:22, Pavel Rappo  wrote:
>>> 
>>> Hello,
>>> 
>>> Please review the following change:
>>> 
>>> http://cr.openjdk.java.net/~prappo/8226303/webrev.00
>> 
>> This looks good. Just a few minor comments:
>> 
>> 1) Please add the bug no. and a brief summary, to the test tags.
>> 
>> 2) The creation and deletion of a file is necessary to exercise test
>>  scenario, but this could prove to be problematic, at least
>>  intermittently on Windows.
>> 
>> 64 Files.delete(file);
>> 65 if (Files.exists(file)) { // just an assumption of the test
>> 66 throw new RuntimeException("File hasn't been deleted");
>> 67 }
>> 
>> Can I suggest that jdk.test.lib.util.FileUtils::deleteFileWithRetry be
>> used instead of Files::delete, as it is less likely to fail.
>> 
>> -Chris.
> 

Re: RFR [14] 8226303: Examine the HttpRequest.BodyPublishers for exception handling

[Pavel Rappo]

Hi Chris,

Thanks for adding net-dev. I should've posted there in the first place, but
something went wrong. As for your comments:

1) I will add this before pushing the change:

* @summary Verifies that some of the standard BodyPublishers relay exception
*  rather than throw it
* @bug 8226303

Is that okay?

2) Done.

Thanks.

> On 18 Jun 2019, at 16:07, Chris Hegarty  wrote:
> 
> [ adding net-dev ]
> 
> Pavel,
> 
>> On 18 Jun 2019, at 14:22, Pavel Rappo  wrote:
>> 
>> Hello,
>> 
>> Please review the following change:
>> 
>> http://cr.openjdk.java.net/~prappo/8226303/webrev.00
> 
> This looks good. Just a few minor comments:
> 
> 1) Please add the bug no. and a brief summary, to the test tags.
> 
> 2) The creation and deletion of a file is necessary to exercise test
>   scenario, but this could prove to be problematic, at least
>   intermittently on Windows.
> 
>  64 Files.delete(file);
>  65 if (Files.exists(file)) { // just an assumption of the test
>  66 throw new RuntimeException("File hasn't been deleted");
>  67 }
> 
>  Can I suggest that jdk.test.lib.util.FileUtils::deleteFileWithRetry be
>  used instead of Files::delete, as it is less likely to fail.
> 
> -Chris.

RFR (T): 8226203: MappedByteBuffer.force method may have no effect on implementation specific map modes

[Andrew Dinn]

Could I please have a review for the following trivial change to the
javadoc for the two MappedByteBuffer.force methods:

  JIRA:   https://bugs.openjdk.java.net/browse/JDK-8226203
  webrev: http://cr.openjdk.java.net/~adinn/8226203/webrev.00/

It allows for the possibility of failures when extended map modes are
used to create the MappedByteBuffer.

The case documented here will not actually arise in JDK13 since nothing
in the JDK creates, let alone exposes extended map modes. However, it
would be better if the javadoc reflected that possibility. Is it still
possible to push this? If that is a problem then I am happy for it to go
into jdk14.

regards,


Andrew Dinn
---

Re: RFR(XS): 8226286 Remove unused method java.lang.Integer::formatUnsignedInt

[Brian Burkhalter]

Hello,

> On Jun 17, 2019, at 9:55 PM, Tagir Valeev  wrote:
> 
> Please review:
> https://bugs.openjdk.java.net/browse/JDK-8226286 
> 
> http://cr.openjdk.java.net/~tvaleev/webrev/8226286/r1/ 
> 
> 
> I left dangling javadoc in-place in the same way as it's done in
> java.lang.Long.

I don’t know whether this was intentional per some policy or an oversight.

> Also I removed redundant reassignment of parameter like s =
> Objects.requireNonNull(s).
> 
> By the way I noticed that remaining methods formatUnsignedInt
> and formatUnsignedIntUTF16 always accept offset = 0. Probably it would be
> good to inline offset parameter?

Yes at it is always zero.

Thanks,

Brian

Re: RFR [14] 8226303: Examine the HttpRequest.BodyPublishers for exception handling

[Chris Hegarty]

[ adding net-dev ]

Pavel,

> On 18 Jun 2019, at 14:22, Pavel Rappo  wrote:
> 
> Hello,
> 
> Please review the following change:
> 
> http://cr.openjdk.java.net/~prappo/8226303/webrev.00

This looks good. Just a few minor comments:

1) Please add the bug no. and a brief summary, to the test tags.

2) The creation and deletion of a file is necessary to exercise test
   scenario, but this could prove to be problematic, at least
   intermittently on Windows.

  64 Files.delete(file);
  65 if (Files.exists(file)) { // just an assumption of the test
  66 throw new RuntimeException("File hasn't been deleted");
  67 }

  Can I suggest that jdk.test.lib.util.FileUtils::deleteFileWithRetry be
  used instead of Files::delete, as it is less likely to fail.

-Chris.

Re: EnumSet.class serialization broken - twice

[Peter Levart]

On 6/18/19 4:00 PM, Martin Buchholz wrote:

Java Historian says:
I was a reviewer for Effective Java 3rd Edition and EnumSet is the 
canonical example of the Serialization Proxy pattern,
so I tried to make sure the pattern was implemented as perfectly as 
possible.

8192935: Fix EnumSet's SerializationProxy javadoc
All of us who try to make java serialization work right have a mental 
model of the many things that might go wrong.
Serialization of Class objects has never been part of my own mental 
model - I've only ever considered instances.


Perhaps the necessity for Class objects representing Serializable 
classes to agree in sertialVersionUID is a bug in Java serialization 
implementation? There's no such requirement for Class objects 
representing non-Serializable classes and I don't see why this 
requirement is there for Serializable classes. Could this requirement 
simply be relaxed with no ill consequences?


Regards, Peter




On Tue, Jun 18, 2019 at 5:32 AM Peter Levart > wrote:


Hi,

I recently stumbled on an exception thrown when deserializing stream
produced on JDK 8 and read with JDK 11. I narrowed the problem
down to
serialization/deserialization of a public EnumSet.class object. There
were several changes made to EnumSet in the Mercurial history of jdk
repo, but I think the following two broke the serialization:

http://hg.openjdk.java.net/jdk/jdk/rev/d0e8542ef650
http://hg.openjdk.java.net/jdk/jdk/rev/a7e13065a7a0

It is interesting to note that before those two changes were made,
there
was a chance to fix the problem reported by newly added serial lint
warnings. Unfortunately they were just silenced:

http://hg.openjdk.java.net/jdk/jdk/rev/501d8479f798

+@SuppressWarnings("serial") // No serialVersionUID due to usage of
+    // serial proxy pattern

It is true that serialization of instances of Serializable classes is
not broken by changes to them when they implement serial proxy
pattern
(i.e. writeReplace() method) even if they don't itself declare a
private
static final long serialVersionUID field, but this is not true of
Class
objects representing those Serializable classes. It is even more
controversial that serialization of Class objects representing
non-Serializable classes is never broken (which is understandable as
they don't have a habit of declaring serialVersionUID fields).

Both of the above braking changes were made post JDK 8 release, so
deserialization of JDK 8 (and older) streams is affected in all
JDK 9 +
releases or vice versa.

So, what shall be done. I suggest adding serialVersionUID field to
EnumSet vith a value that corresponds to JDK 8 serialization
format and
later backport this change to JDK 11.

What do you think?


Regards, Peter


PS: ImmutableCollections nested classes also implement serial proxy
pattern and don't declare serialVersionUID fields, but they are not
public, so it is less chance that Class objects representing them
could
be used in serial streams, although it is not impossible. For example:

objectOutputStream.writeObject(Set.of().getClass());

Re: EnumSet.class serialization broken - twice

[Martin Buchholz]

Java Historian says:
I was a reviewer for Effective Java 3rd Edition and EnumSet is the
canonical example of the Serialization Proxy pattern,
so I tried to make sure the pattern was implemented as perfectly as
possible.
8192935: Fix EnumSet's SerializationProxy javadoc
All of us who try to make java serialization work right have a mental model
of the many things that might go wrong.
Serialization of Class objects has never been part of my own mental model -
I've only ever considered instances.


On Tue, Jun 18, 2019 at 5:32 AM Peter Levart  wrote:

> Hi,
>
> I recently stumbled on an exception thrown when deserializing stream
> produced on JDK 8 and read with JDK 11. I narrowed the problem down to
> serialization/deserialization of a public EnumSet.class object. There
> were several changes made to EnumSet in the Mercurial history of jdk
> repo, but I think the following two broke the serialization:
>
> http://hg.openjdk.java.net/jdk/jdk/rev/d0e8542ef650
> http://hg.openjdk.java.net/jdk/jdk/rev/a7e13065a7a0
>
> It is interesting to note that before those two changes were made, there
> was a chance to fix the problem reported by newly added serial lint
> warnings. Unfortunately they were just silenced:
>
> http://hg.openjdk.java.net/jdk/jdk/rev/501d8479f798
>
> +@SuppressWarnings("serial") // No serialVersionUID due to usage of
> +// serial proxy pattern
>
> It is true that serialization of instances of Serializable classes is
> not broken by changes to them when they implement serial proxy pattern
> (i.e. writeReplace() method) even if they don't itself declare a private
> static final long serialVersionUID field, but this is not true of Class
> objects representing those Serializable classes. It is even more
> controversial that serialization of Class objects representing
> non-Serializable classes is never broken (which is understandable as
> they don't have a habit of declaring serialVersionUID fields).
>
> Both of the above braking changes were made post JDK 8 release, so
> deserialization of JDK 8 (and older) streams is affected in all JDK 9 +
> releases or vice versa.
>
> So, what shall be done. I suggest adding serialVersionUID field to
> EnumSet vith a value that corresponds to JDK 8 serialization format and
> later backport this change to JDK 11.
>
> What do you think?
>
>
> Regards, Peter
>
>
> PS: ImmutableCollections nested classes also implement serial proxy
> pattern and don't declare serialVersionUID fields, but they are not
> public, so it is less chance that Class objects representing them could
> be used in serial streams, although it is not impossible. For example:
>
> objectOutputStream.writeObject(Set.of().getClass());
>
>

Re: 8226242 : Diagnostic output for posix_spawn failure

[Roger Riggs]

Hi David,

The error that is being reported is the absence of an ack from 
jsspawnhelper (CHILD_IS_ALIVE)

indicating that it has started (and is about to exec the target exe).
The child process should continue to run; reporting the status of the child
will hopefully lead to identifying why the child process terminated 
without exec'ing the target.


Thanks, Roger


On 6/17/19 6:16 PM, David Holmes wrote:

Hi Roger,

On 18/06/2019 6:00 am, Roger Riggs wrote:

Hi,

Updated:
http://cr.openjdk.java.net/~rriggs/webrev-spawn-diag-8225192-3/


+ if (WIFEXITED(status)) {
+ snprintf(ebuf, sizeof ebuf,
+ "Failed to exec spawn helper: pid: %d, exit value: %d",
+ pid, WEXITSTATUS(status));

WIFEXITED returns non-zero when the process terminates normally, so 
that need not be an error AFAICS.


David



On 6/17/19 2:20 PM, Martin Buchholz wrote:



On Mon, Jun 17, 2019 at 10:47 AM Thomas Stüfe 
mailto:thomas.stu...@gmail.com>> wrote:


    Hi Roger,

    I think this is fine.

    Could you please also print out WIFEXITED and WIFSIGNALLED? Since
    I am not
    sure if the WEXITSTATUS contains valid info if WIFEXITED is 0, 
or just

    random noise. Same for WTERMSIG.

I separated the causes, both exits are abnormal since the exec 
didn't' do what was expected.
Only an exit status of 0 is 'normal' and in this case, that is not 
what expected.


Regards, Roger



    Alternatively:
    if (WIFEXITED) print("exited normaly with %d", WEXITSTATUS)
    else if (WIFSTOPPED) print("terminated with signal %d", WTERMSIG)


thanks for doing this.

I also think it's best to have different detail messages for normal 
termination and death by signal, as some shells do.


I might put the exception throwing code into a separate function.

RFR [14] 8226303: Examine the HttpRequest.BodyPublishers for exception handling

[Pavel Rappo]

Hello,

Please review the following change:

 http://cr.openjdk.java.net/~prappo/8226303/webrev.00

This change increases robustness of convenience BodyPublishers by going the
extra mile and ensuring unlikely thrown exceptions are relayed downstream.

Thanks,
-Pavel

RE: [11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

[Baesken, Matthias]

Hello Christoph,   looks good   with  the exception of

src/java.base/share/classes/jdk/internal/module/Checks.java

http://cr.openjdk.java.net/~clanger/webrevs/8215281.11u/src/java.base/share/classes/jdk/internal/module/Checks.java.frames.html


 /**
- * Returns {@code true} if the given name is a legal module name.
- */
-public static boolean isModuleName(String name) {

   

-private static boolean isJavaIdentifier(CharSequence cs) {
-if (cs.length() == 0 || RESERVED.contains(cs))
+private static boolean isJavaIdentifier(String str) {
+if (str.isEmpty() || RESERVED.contains(str))


 ... where I am a bit  unsure - should we really  remove a method in jdk11  
(and change signature of another) ?  I know ,  it is  from  jdk/internal   , 
but still  I have a bad feeling about this .
This was probably fine for  the higher release  but for jdk11  it might be 
better   to keep what we have .

Best regards, Matthias


From: Langer, Christoph
Sent: Dienstag, 18. Juni 2019 10:59
To: jdk-updates-...@openjdk.java.net
Cc: Java Core Libs ; Baesken, Matthias 

Subject: [11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

Hi,

please review the backport of the String.isEmpty() cleanups in java.base.

The main reason to bring this down to JDK11u is that it'll ease backporting of 
later changes which otherwise run into conflicts and won't resolve. 
Furthermore, the original change is a nice cleanup and improves performance.

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8215281.11u/
Bug: https://bugs.openjdk.java.net/browse/JDK-8215281
Change in jdk/jdk: http://hg.openjdk.java.net/jdk/jdk/rev/8bf9268df0e2

The original patch merely applies (with a little fuzz in some places). There 
were 4 rejects:

src/java.base/share/classes/java/lang/VersionProps.java.template
--- VersionProps.java.template
+++ VersionProps.java.template
@@ -95,7 +95,7 @@
 props.put("java.version.date", java_version_date);
 props.put("java.runtime.version", java_runtime_version);
 props.put("java.runtime.name", java_runtime_name);
-if (VENDOR_VERSION_STRING.length() > 0)
+if (!VENDOR_VERSION_STRING.isEmpty())
 props.put("java.vendor.version", VENDOR_VERSION_STRING);

 props.put("java.class.version", CLASSFILE_MAJOR_MINOR);

-> manually resolved that to the right location

src/java.base/share/classes/java/text/CompactNumberFormat.java

-> file does not exist in 11u, so dropping

src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckMethodAdapter.java
--- CheckMethodAdapter.java
+++ CheckMethodAdapter.java
@@ -1314,7 +1314,7 @@
   * @param message the message to use in case of error.
   */
 static void checkMethodIdentifier(final int version, final String name, 
final String message) {
-if (name == null || name.length() == 0) {
+if (name == null || name.isEmpty()) {
 throw new IllegalArgumentException(INVALID + message + 
MUST_NOT_BE_NULL_OR_EMPTY);
 }
 if ((version & 0x) >= Opcodes.V1_5) {
@@ -1347,7 +1347,7 @@
   * @param message the message to use in case of error.
   */
 static void checkInternalName(final int version, final String name, final 
String message) {
-if (name == null || name.length() == 0) {
+if (name == null || name.isEmpty()) {
 throw new IllegalArgumentException(INVALID + message + 
MUST_NOT_BE_NULL_OR_EMPTY);
 }
 if (name.charAt(0) == '[') {
@@ -1457,7 +1457,7 @@
   * @param descriptor the string to be checked.
   */
 static void checkMethodDescriptor(final int version, final String 
descriptor) {
-if (descriptor == null || descriptor.length() == 0) {
+if (descriptor == null || descriptor.isEmpty()) {
 throw new IllegalArgumentException("Invalid method descriptor 
(must not be null or empty)");
 }
 if (descriptor.charAt(0) != '(' || descriptor.length() < 3) {

-> file looks different in 11u due to a missing change. So, modified the right 
places in the 11u version

src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckSignatureAdapter.java
--- CheckSignatureAdapter.java
+++ CheckSignatureAdapter.java
@@ -365,7 +365,7 @@
 }

 private void checkClassName(final String name, final String message) {
-if (name == null || name.length() == 0) {
+if (name == null || name.isEmpty()) {
 throw new IllegalArgumentException(INVALID + message + " (must not 
be null or empty)");
 }
 for (int i = 0; i < name.length(); ++i) {
@@ -377,7 +377,7 @@
 }

 private void checkIdentifier(final String name, final String message) {
-if (name == null || name.length() == 0) {
+if (name == null || name.isEmpty()) {
 throw new IllegalArgumentException(INVALID + message + " (must not 
be null or empty)");
 }
 for (int i = 0; i < 

Re: Question re: Usage of JNICALL with varargs function with 32bit compilers

[Steve Groeger]

Hi Thomas, 

There are not issues caused by this, as you state, the compile forces the 
callee and caller back to the same convention.
The only issue we had was with the amount of warning that were produced 
when we (IBM) compiled the jdk.
We avoided the error by adding -Wno-ignored-attributes in the makefile, 
but that was more of a work-around than an actual fix.

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



From:   "Thomas Stüfe" 
To: Steve Groeger 
Cc: core-libs 
Date:   18/06/2019 13:01
Subject:Re: Question re: Usage of JNICALL with varargs function 
with 32bit compilers



Hi Steve,

I always saw this as a harmless warning, safe to ignore, since both caller 
and callee are forced back to the same convention so it all works out in 
the end. Or can you envision a scenario where this would be harmfull?

Thank god Microsoft abandoned this calling convention circus with 64bit.

Cheers, Thomas


On Tue, Jun 18, 2019 at 1:31 PM Steve Groeger  wrote:
Hi all, 

I had a query regarding the use of JNICALL with varargs functions, which I 

am hoping someone can comment on.

As explained on MSDN at [1], all functions declared as __stdcall that have 

variable parameters (varargs) will be forced back to __cdecl by the MSVC. 
In such case, keeping JNICALL for varargs functions/function pointers 
turns out be inappropriate.
e.g.

include/jni.h
jobject (JNICALL *NewObject)
  (JNIEnv *env, jclass clazz, jmethodID methodID, ...);

According to [2], stdcall does not support variadic calls in C on 32-bit 
x86 targets. 

When compiling a JNI native (calling NewObject) with Clang, it ends up 
with the warning as follows:
...\include\jni.h:277:14: warning: stdcall calling convention ignored on 
variadic function [-Wignored-attributes]
jobject (JNICALL *NewObject)
 ^

So basically the compiler is saying it can't do what was requested, as 
such should we stop asking for JNICALL on varargs functions. 
Doing so will only improve the portability of code to a wider set of 
compilers.

Comments?

[1] https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx
[2] https://clang.llvm.org/docs/AttributeReference.html#stdcall

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

EnumSet.class serialization broken - twice

[Peter Levart]

Hi,

I recently stumbled on an exception thrown when deserializing stream 
produced on JDK 8 and read with JDK 11. I narrowed the problem down to 
serialization/deserialization of a public EnumSet.class object. There 
were several changes made to EnumSet in the Mercurial history of jdk 
repo, but I think the following two broke the serialization:


http://hg.openjdk.java.net/jdk/jdk/rev/d0e8542ef650
http://hg.openjdk.java.net/jdk/jdk/rev/a7e13065a7a0

It is interesting to note that before those two changes were made, there 
was a chance to fix the problem reported by newly added serial lint 
warnings. Unfortunately they were just silenced:


http://hg.openjdk.java.net/jdk/jdk/rev/501d8479f798

+@SuppressWarnings("serial") // No serialVersionUID due to usage of
+    // serial proxy pattern

It is true that serialization of instances of Serializable classes is 
not broken by changes to them when they implement serial proxy pattern 
(i.e. writeReplace() method) even if they don't itself declare a private 
static final long serialVersionUID field, but this is not true of Class 
objects representing those Serializable classes. It is even more 
controversial that serialization of Class objects representing 
non-Serializable classes is never broken (which is understandable as 
they don't have a habit of declaring serialVersionUID fields).


Both of the above braking changes were made post JDK 8 release, so 
deserialization of JDK 8 (and older) streams is affected in all JDK 9 + 
releases or vice versa.


So, what shall be done. I suggest adding serialVersionUID field to 
EnumSet vith a value that corresponds to JDK 8 serialization format and 
later backport this change to JDK 11.


What do you think?


Regards, Peter


PS: ImmutableCollections nested classes also implement serial proxy 
pattern and don't declare serialVersionUID fields, but they are not 
public, so it is less chance that Class objects representing them could 
be used in serial streams, although it is not impossible. For example:


objectOutputStream.writeObject(Set.of().getClass());

Re: Question re: Usage of JNICALL with varargs function with 32bit compilers

[Thomas Stüfe]

Hi Steve,

I always saw this as a harmless warning, safe to ignore, since both caller
and callee are forced back to the same convention so it all works out in
the end. Or can you envision a scenario where this would be harmfull?

Thank god Microsoft abandoned this calling convention circus with 64bit.

Cheers, Thomas


On Tue, Jun 18, 2019 at 1:31 PM Steve Groeger  wrote:

> Hi all,
>
> I had a query regarding the use of JNICALL with varargs functions, which I
> am hoping someone can comment on.
>
> As explained on MSDN at [1], all functions declared as __stdcall that have
> variable parameters (varargs) will be forced back to __cdecl by the MSVC.
> In such case, keeping JNICALL for varargs functions/function pointers
> turns out be inappropriate.
> e.g.
>
> include/jni.h
> jobject (JNICALL *NewObject)
>   (JNIEnv *env, jclass clazz, jmethodID methodID, ...);
>
> According to [2], stdcall does not support variadic calls in C on 32-bit
> x86 targets.
>
> When compiling a JNI native (calling NewObject) with Clang, it ends up
> with the warning as follows:
> ...\include\jni.h:277:14: warning: stdcall calling convention ignored on
> variadic function [-Wignored-attributes]
> jobject (JNICALL *NewObject)
>  ^
>
> So basically the compiler is saying it can't do what was requested, as
> such should we stop asking for JNICALL on varargs functions.
> Doing so will only improve the portability of code to a wider set of
> compilers.
>
> Comments?
>
> [1] https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx
> [2] https://clang.llvm.org/docs/AttributeReference.html#stdcall
>
> Thanks
> Steve Groeger
> IBM Runtime Technologies
> Hursley, Winchester
> Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
> Fax (44) 1962 816800
> Lotus Notes: Steve Groeger/UK/IBM
> Internet: groe...@uk.ibm.com
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>
>

Re: Slightly faster java.util.Arrays.byteSort(byte[])

[Vladimir Yaroslavskiy]

Hi Rodion,

I looked at your byte sorting and compared it with the version which is under
review now. Here is the summary of my results:

1. net.coderodde.Arrays.sort(byte[]) works slower on small data (len < ~70).
The reason is that insertion sort is invoked on small arrays where counting
sort is not fast.

2. On other data net.coderodde.Arrays.sort shows the same time.

In general your code is not faster to be integrated, but many thanks
for pointing to non-perfect implementation of existing byte sorting.

I provide here the new implementation of byte sorting. Please look at it
and let me know if you have other ideas how to improve it.

/**
 * Min size of a byte array to use counting sort.
 */
private static final int MIN_BYTE_COUNTING_SORT_SIZE = 64;

/**
 * Sorts the specified range of the array using
 * counting sort or insertion sort.
 *
 * @param a the array to be sorted
 * @param low the index of the first element, inclusive, to be sorted
 * @param high the index of the last element, exclusive, to be sorted
 */
static void sort(byte[] a, int low, int high) {
if (high - low > MIN_BYTE_COUNTING_SORT_SIZE) {
countingSort(a, low, high);
} else {
insertionSort(a, low, high);
}
}

/**
 * Sorts the specified range of the array using insertion sort.
 *
 * @param a the array to be sorted
 * @param low the index of the first element, inclusive, to be sorted
 * @param high the index of the last element, exclusive, to be sorted
 */
private static void insertionSort(byte[] a, int low, int high) {
for (int i, k = low; ++k < high; ) {
byte ai = a[i = k];

if (ai < a[i - 1]) {
while (--i >= low && ai < a[i]) {
a[i + 1] = a[i];
}
a[i + 1] = ai;
}
}
}

/**
 * The number of distinct byte values.
 */
private static final int NUM_BYTE_VALUES = 1 << 8;

/**
 * Max index of byte counter.
 */
private static final int MAX_BYTE_INDEX = Byte.MAX_VALUE + NUM_BYTE_VALUES + 1;

/**
 * Sorts the specified range of the array using counting sort.
 *
 * @param a the array to be sorted
 * @param low the index of the first element, inclusive, to be sorted
 * @param high the index of the last element, exclusive, to be sorted
 */
private static void countingSort(byte[] a, int low, int high) {
int[] count = new int[NUM_BYTE_VALUES];

/*
 * Compute a histogram with the number of each values.
 */
for (int i = high; i > low; ++count[a[--i] & 0xFF]);

/*
 * Place values on their final positions.
 */
for (int k = high, i = MAX_BYTE_INDEX; --i > Byte.MAX_VALUE; ) {
int value = i & 0xFF;
for (low = k - count[value]; k > low; a[--k] = (byte) value);
}
}

Thank you,
Vladimir

>>I'm not an expert, however, while your OCA (see below) is being processed I
>>would recommend to provide more comprehensive stats. Different lengths of an
>>array, different flavours of data: sorted, sorted in the reverse order, 
>>random,
>>typical expected case(s), etc.
>>
>>It seems that this particular functionality ( sort(byte[] ) hasn't changed 
>>since
>>the JDK 8. However, you should probably add the current JDK to your 
>>comparison.
>>
>>One necessary step towards making this eligible for inclusion in the JDK would
>>be to sign the OCA
>>
>>  https://www.oracle.com/technetwork/community/oca-486395.html
>>
>>Keep in mind that it is not by any means a guarantee that your change will be
>>included. Once the OCA has been signed and processed, the code then can be
>>discussed and evaluated by experts.
>>
>>-Pavel
>>
>>> On 14 Jun 2019, at 16:34, Rodion Efremov <  codero...@gmail.com > wrote:
>>> 
>>> Good evening!
>>> 
>>> I managed to improve the JDK 8 java.util.Arrays.sort(byte[])
>>> performance-wise [1]. The (warmed up) demonstration program produces more
>>> or less optimistic results on arrays of 1e8 bytes:
>>> 
>>> seed = 1560526264738
>>> java.util.Arrays.sort(byte[]) in  87.643701 milliseconds.
>>> java.util.Arrays.parallelSort(byte[]) in 301.329701 milliseconds.
>>> net.coderodde.Arrays.sort(byte[]) in 62.0763 milliseconds.
>>> Algorithms agree: true
>>> 
>>> I would like to hear any comments on how to make it eligible for inclusion
>>> in JDK.
>>> 
>>> Best regards,
>>> Rodion E.
>>> 
>>> References:
>>> [1]  https://gist.github.com/coderodde/493407bc1c57352b53c2aa18b5c9a7a8
>Hi Rodion,
>I'm working on the new version of Arrays.sort() / Arrays.parallelSort() which 
>is under review.
>I will look on your version and provide my comments.
>Thank you,
>Vladimir

Re: RFR: 8224974: Implement JEP 352

[Andrew Dinn]

Hi Alan,

Thanks for reviewing the JEP one more time.

On 16/06/2019 09:47, Alan Bateman wrote:
> I re-read the JEP, trying to put myself in the position of someone
> reading it for the first time in 2020.
> 
> Summary section:
> 
> What would you think about replacing this with a sentence that makes it
> clear that the JEP adds new JDK-specific file mapping modes to allow the
> FileChannel API create MappedByteBuffers over non-volatile memory?

I would be happy with that way of describing the behaviour change except
that what you suggest doesn't mention actually making it work -- which
/is/ part of the JEP. How about:

"Add new JDK-specific file mapping modes, allowing the FileChannel API
to be used to create MappedByteBuffers over non-volatile memory, and
extend the underlying implementation to support this new use case"

> Goals section:
> 
> I think the first paragraph could be re-worded to make it clear that the
> goal is to use the existing MappedByteBuffer API to access NVM.

Yes indeed:

"This JEP proposes that class MappedByteBuffer be reworked to support
access to non-volatile memory (NVM). The only API change required is a
new enumeration employed by FileChannel clients to request mapping of a
file located on an NVM-backed file system rather than a conventional,
file storage system. Recent changes to the MappedByteBufer API mean that
it supports all the behaviours needed to allow direct memory updates and
provide the durability guarantees needed for higher level, Java client
libraries to implement persistent data types (e.g. block file systems,
journaled logs, persistent objects, etc.). The implementations of
FileChannel and MappedByteBuffer need revising to be aware of this new
backing type for the mapped file."

> Paragraphs 2-5 split this into two sub-goals. The first suggests that it
> extends the MBB API but this is no longer the case. The second also
> hints that it adds a new API. I think these two need to be re-worded.

Agreed. How about this:

"The primary goal of this JEP is to ensure that clients can access and
update NVM from a Java program efficiently and coherently. A key element
of this goal is to ensure that individual writes (or small groups of
contiguous writes) to a buffer region can be committed with minimal
overhead i.e. to ensure that any changes which might still be in cache
are written back to memory."

n.b. I snuck in the word coherence because I thought it clarified the
notion of committing to memory.

> Goal 3 and 4 are okay, although I think the 4th could be summarized as
> allowing mapped buffers on NVM to be tracked by the existing monitoring
> and management APIs.

Agreed. So, renumbering accordingly, how about this:

"A second, subordinate goal is to implement this commit behaviour using
a restricted, JDK-internal API defined in class Unsafe, allowing it to
be re-used by classes other than MappedByteBuffer that may need to
commit NVM.

A final, related goal is to allow buffers mapped over NVM to be tracked
by the existing monitoring and management APIs."

> Description section
> 
> It might be clearer of "Proposed Java API Changes" were renamed to
> "Proposed JDK-specific API changes".

Agreed.

> One other thing to mention is that I re-read the javadoc for the MBB
> force methods and I think we need to adjust one of the sentences in the
> existing (and new) methods to take account of implementation specific
> map modes. I've created JDK-8226203 [1] to track it. As support for
> implementation specific map modes is in new in Java SE 13 then it might
> be worth trying to get that fixed now while it is fresh in our minds.
Sure, I'll post an RFR with the javadoc patch as a separate step. Can it
go into jdk13? or is it too late for that?

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

Question re: Usage of JNICALL with varargs function with 32bit compilers

[Steve Groeger]

Hi all, 

I had a query regarding the use of JNICALL with varargs functions, which I 
am hoping someone can comment on.

As explained on MSDN at [1], all functions declared as __stdcall that have 
variable parameters (varargs) will be forced back to __cdecl by the MSVC. 
In such case, keeping JNICALL for varargs functions/function pointers 
turns out be inappropriate.
e.g.

include/jni.h
jobject (JNICALL *NewObject)
  (JNIEnv *env, jclass clazz, jmethodID methodID, ...);

According to [2], stdcall does not support variadic calls in C on 32-bit 
x86 targets. 

When compiling a JNI native (calling NewObject) with Clang, it ends up 
with the warning as follows:
...\include\jni.h:277:14: warning: stdcall calling convention ignored on 
variadic function [-Wignored-attributes]
jobject (JNICALL *NewObject)
 ^

So basically the compiler is saying it can't do what was requested, as 
such should we stop asking for JNICALL on varargs functions. 
Doing so will only improve the portability of code to a wider set of 
compilers.

Comments?

[1] https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx
[2] https://clang.llvm.org/docs/AttributeReference.html#stdcall

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

[11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

[Langer, Christoph]

Hi,

please review the backport of the String.isEmpty() cleanups in java.base.

The main reason to bring this down to JDK11u is that it'll ease backporting of 
later changes which otherwise run into conflicts and won't resolve. 
Furthermore, the original change is a nice cleanup and improves performance.

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8215281.11u/
Bug: https://bugs.openjdk.java.net/browse/JDK-8215281
Change in jdk/jdk: http://hg.openjdk.java.net/jdk/jdk/rev/8bf9268df0e2

The original patch merely applies (with a little fuzz in some places). There 
were 4 rejects:

src/java.base/share/classes/java/lang/VersionProps.java.template
--- VersionProps.java.template
+++ VersionProps.java.template
@@ -95,7 +95,7 @@
 props.put("java.version.date", java_version_date);
 props.put("java.runtime.version", java_runtime_version);
 props.put("java.runtime.name", java_runtime_name);
-if (VENDOR_VERSION_STRING.length() > 0)
+if (!VENDOR_VERSION_STRING.isEmpty())
 props.put("java.vendor.version", VENDOR_VERSION_STRING);
 props.put("java.class.version", CLASSFILE_MAJOR_MINOR);

-> manually resolved that to the right location

src/java.base/share/classes/java/text/CompactNumberFormat.java

-> file does not exist in 11u, so dropping

src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckMethodAdapter.java
--- CheckMethodAdapter.java
+++ CheckMethodAdapter.java
@@ -1314,7 +1314,7 @@
   * @param message the message to use in case of error.
   */
 static void checkMethodIdentifier(final int version, final String name, 
final String message) {
-if (name == null || name.length() == 0) {
+if (name == null || name.isEmpty()) {
 throw new IllegalArgumentException(INVALID + message + 
MUST_NOT_BE_NULL_OR_EMPTY);
 }
 if ((version & 0x) >= Opcodes.V1_5) {
@@ -1347,7 +1347,7 @@
   * @param message the message to use in case of error.
   */
 static void checkInternalName(final int version, final String name, final 
String message) {
-if (name == null || name.length() == 0) {
+if (name == null || name.isEmpty()) {
 throw new IllegalArgumentException(INVALID + message + 
MUST_NOT_BE_NULL_OR_EMPTY);
 }
 if (name.charAt(0) == '[') {
@@ -1457,7 +1457,7 @@
   * @param descriptor the string to be checked.
   */
 static void checkMethodDescriptor(final int version, final String 
descriptor) {
-if (descriptor == null || descriptor.length() == 0) {
+if (descriptor == null || descriptor.isEmpty()) {
 throw new IllegalArgumentException("Invalid method descriptor 
(must not be null or empty)");
 }
 if (descriptor.charAt(0) != '(' || descriptor.length() < 3) {

-> file looks different in 11u due to a missing change. So, modified the right 
places in the 11u version

src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckSignatureAdapter.java
--- CheckSignatureAdapter.java
+++ CheckSignatureAdapter.java
@@ -365,7 +365,7 @@
 }
 private void checkClassName(final String name, final String message) {
-if (name == null || name.length() == 0) {
+if (name == null || name.isEmpty()) {
 throw new IllegalArgumentException(INVALID + message + " (must not 
be null or empty)");
 }
 for (int i = 0; i < name.length(); ++i) {
@@ -377,7 +377,7 @@
 }
 private void checkIdentifier(final String name, final String message) {
-if (name == null || name.length() == 0) {
+if (name == null || name.isEmpty()) {
 throw new IllegalArgumentException(INVALID + message + " (must not 
be null or empty)");
 }
 for (int i = 0; i < name.length(); ++i) {

-> file looks different in 11u due to a missing change. So, modified the right 
places in the 11u version

Thanks
Christoph