Re: trivial correction to javadoc of j.l.i.MethodHandles.arrayElementVarHandle()

[Mandy Chung]

Pushed: https://hg.openjdk.java.net/jdk/jdk/rev/3a3c806aff1f

Mandy

On 4/24/20 9:17 AM, Mandy Chung wrote:

Yes any change requires a JBS issue.

I can sponsor this trivial fix.

Mandy

On 4/24/20 5:15 AM, Raffaello Giulietti wrote:

Not sure if this requires an entry in the JBS...


Greetings
Raffaello




diff -r 70c2239696ae 
src/java.base/share/classes/java/lang/invoke/MethodHandles.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 16:12:53 2020 +0530
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 14:08:46 2020 +0200

@@ -3960,7 +3960,7 @@
  * {@code short}, {@code char}, {@code int}, {@code long},
  * {@code float}, or {@code double} then numeric atomic 
update access

  * modes are unsupported.
- * if the field type is anything other than {@code boolean},
+ * if the component type is anything other than {@code 
boolean},

  * {@code byte}, {@code short}, {@code char}, {@code int} or
  * {@code long} then bitwise atomic update access modes are
  * unsupported.

Re: RFR: 8243469: Lazily encode name in ZipFile.getEntryPos

[Claes Redestad]

On 2020-04-24 20:07, Lance Andersen wrote:

Hi Claes,

Thank you for continuing to improve the performance in this part of 
java.util.zip.  The changes I think look pretty good.


Best
Lance



Thanks for the review, Lance!

Hopefully a few more improvements to come..

/Claes

Re: RFR: 8243469: Lazily encode name in ZipFile.getEntryPos

[Claes Redestad]

Hi Volker,

On 2020-04-24 20:37, Volker Simonis wrote:

On Thu, Apr 23, 2020 at 2:35 PM Claes Redestad
 wrote:


Hi,

current implementation of ZipFile.getEntryPos takes the encoded byte[]
of the String we're looking up. Which means when looking up entries
across multiple jar files, we allocate the byte[] over and over for each
jar file searched.

If we instead refactor the internal hash table to use a normalized
String-based hash value, we can almost always avoid both the repeated
encoding and (most of) the hash calculation when the entry is not found
in the jar/zip.

This realizes a significant startup improvement on applications with
several or many jar files. A rather typical case. For example I see a
25% speedup of ZipEnty.getEntry calls on a Spring PetClinic, which
reduces total startup time by ~120ms, or ~2.5% of total, on my setup. At
the same time remaining neutral on single-jar apps.

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

Testing: tier1-2


Hi Claes,

that's a little puzzling change but also a nice enhancement and cleanup.


first: thanks for reviewing!

Yes, I've tried to simplify and explain everything clearly, since it's a
rather delicate area and we're already stretching the complexity budget
thin.



I think it looks good. I have only two minor comments:

There's no check for "end > 0" here:
   93 @Override
   94 boolean hasTrailingSlash(byte[] a, int end) {
   95 return a[end - 1] == '/';
   96 }
I think that's currently no real problem, but maybe put in a check for any case?


Will do.



And while you're on it, I think the following comment should be updated from:

  641 /* Checks ensureOpen() before invoke this method */

to something like:

  641 /* Check ensureOpen() before invoking this method */


Will do.




I've also had a quick look at the microbenchmark which you've
apparently only added today :)

It seems that 'getEntryHitUncached' is getting slightly slower with
your change while all the other variants get significantly faster. I
don't think that's a problem, but do you have an explanation why
that's the case?


I've noticed it swing a bit either way, and have been asking myself the
same thing. After a little analysis I think it's actually a bug in my
microbenchmark: I'm always looking up the same entry, and thus hitting
the same bucket in the hash table. If that one has a collision, we'll do
a few extra passes. If not, we won't. This might be reflected as a
significant swing in either direction.

I'm going to try rewriting it to consider more (if not all) entries in
the zip file. That should mean the cost averages out a bit.

/Claes



Thanks for this nice improvement,
Volker



Thanks!

/Claes

Re: RFR: 8243469: Lazily encode name in ZipFile.getEntryPos

[Volker Simonis]

On Thu, Apr 23, 2020 at 2:35 PM Claes Redestad
 wrote:
>
> Hi,
>
> current implementation of ZipFile.getEntryPos takes the encoded byte[]
> of the String we're looking up. Which means when looking up entries
> across multiple jar files, we allocate the byte[] over and over for each
> jar file searched.
>
> If we instead refactor the internal hash table to use a normalized
> String-based hash value, we can almost always avoid both the repeated
> encoding and (most of) the hash calculation when the entry is not found
> in the jar/zip.
>
> This realizes a significant startup improvement on applications with
> several or many jar files. A rather typical case. For example I see a
> 25% speedup of ZipEnty.getEntry calls on a Spring PetClinic, which
> reduces total startup time by ~120ms, or ~2.5% of total, on my setup. At
> the same time remaining neutral on single-jar apps.
>
> Webrev: http://cr.openjdk.java.net/~redestad/8243469/open.00/
> Bug:https://bugs.openjdk.java.net/browse/JDK-8243469
>
> Testing: tier1-2

Hi Claes,

that's a little puzzling change but also a nice enhancement and cleanup.

I think it looks good. I have only two minor comments:

There's no check for "end > 0" here:
  93 @Override
  94 boolean hasTrailingSlash(byte[] a, int end) {
  95 return a[end - 1] == '/';
  96 }
I think that's currently no real problem, but maybe put in a check for any case?

And while you're on it, I think the following comment should be updated from:

 641 /* Checks ensureOpen() before invoke this method */

to something like:

 641 /* Check ensureOpen() before invoking this method */


I've also had a quick look at the microbenchmark which you've
apparently only added today :)

It seems that 'getEntryHitUncached' is getting slightly slower with
your change while all the other variants get significantly faster. I
don't think that's a problem, but do you have an explanation why
that's the case?

Thanks for this nice improvement,
Volker

>
> Thanks!
>
> /Claes

Re: RFR: 8243469: Lazily encode name in ZipFile.getEntryPos

[Lance Andersen]

Hi Claes,

Thank you for continuing to improve the performance in this part of 
java.util.zip.  The changes I think look pretty good. 

Best
Lance

> On Apr 23, 2020, at 8:34 AM, Claes Redestad  wrote:
> 
> Hi,
> 
> current implementation of ZipFile.getEntryPos takes the encoded byte[]
> of the String we're looking up. Which means when looking up entries
> across multiple jar files, we allocate the byte[] over and over for each
> jar file searched.
> 
> If we instead refactor the internal hash table to use a normalized
> String-based hash value, we can almost always avoid both the repeated
> encoding and (most of) the hash calculation when the entry is not found
> in the jar/zip.
> 
> This realizes a significant startup improvement on applications with
> several or many jar files. A rather typical case. For example I see a
> 25% speedup of ZipEnty.getEntry calls on a Spring PetClinic, which
> reduces total startup time by ~120ms, or ~2.5% of total, on my setup. At
> the same time remaining neutral on single-jar apps.
> 
> Webrev: http://cr.openjdk.java.net/~redestad/8243469/open.00/
> Bug:https://bugs.openjdk.java.net/browse/JDK-8243469
> 
> Testing: tier1-2
> 
> Thanks!
> 
> /Claes

 
  

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

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

[Maurizio Cimadamore]

On 24/04/2020 17:38, Paul Sandoz wrote:



On Apr 23, 2020, at 5:45 PM, Maurizio Cimadamore 
 wrote:


On 24/04/2020 01:35, Paul Sandoz wrote:

Hi,

Looks good. I have seen almost all of this in reviews on panama-dev hence the 
lack of substantial comments here.

I suspect we are not gonna need the drop argument VH combinator, dropping 
coordinates feels a little suspicious to me, but I can see why it's there for 
completeness.

Thanks Paul.

Re. drop coordinates, note that we're actually using it in 
MemoryHandles.withStride, so that we can insert a dummy coordinate that is 
always discarded in case the stride is zero. We could do without it as well, of 
course, but sometimes it's hard to have a sense of how these pieces might be 
joined in practice.


Ah, ok I can see its value now when dropping a coordinate that would otherwise 
result in some redundant calculation.  However, looking at this more closely:

* @param bytesStride the stride, in bytes, by which to multiply the coordinate 
value. Must be greater than zero.

It implies that a zero value should be disallowed contrary to the 
implementation.  I am wondering if your intent was support a signed stride 
value?

In this case it could be argued that a zero stride is misleading, if supported, 
since any value passed for the coordinate X has no effect.  But I can also see 
the other side from a position uniformity, and then why not support negative 
strides, which I think the implementation does support.


That comment is bogus. In the previous iteration the stride was positive 
- now we removed all restrictions so that you can go forwards and 
backwards; we added zero for completeness but the javadoc does not 
reflect that (I mean, the @throws does, but not the main method 
description).


I'll rectify that

Thanks
Maurizio



Paul.


Maurizio


Paul.


On Apr 23, 2020, at 1:33 PM, 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 the program is executed using the 
-Dforeign.unsafe=permit flag.
   - the MemorySegment::mapFromPath now returns a MappedMemorySegment
* MappedMemorySegment
   - small sub-interface which provides extra capabilities for mapped segments 
(load(), unload() and force())
* MemoryAddress
   - added distinction between *checked* and *unchecked* addresses; *unchecked* 
addresses do not have a segment, so they cannot be dereferenced
   - added NULL memory address (it's an unchecked address)
   - added factory to construct MemoryAddress from long value (result is also 
an unchecked address)
   - added API point to get raw

Re: RFR: JDK-8236129: Exe installers have wrong properties

[Andy Herrick]

looks good

/Andy

On 4/23/2020 5:34 PM, Alexey Semenyuk wrote:

Please review fix [2] for jpackage bug [1].

Implement rebranding of exe installers produced by jpackage.

- Alexey

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

[2] http://cr.openjdk.java.net/~asemenyuk/8236129/webrev.00

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

[Paul Sandoz]

> On Apr 23, 2020, at 5:45 PM, Maurizio Cimadamore 
>  wrote:
> 
> 
> On 24/04/2020 01:35, Paul Sandoz wrote:
>> Hi,
>> 
>> Looks good. I have seen almost all of this in reviews on panama-dev hence 
>> the lack of substantial comments here.
>> 
>> I suspect we are not gonna need the drop argument VH combinator, dropping 
>> coordinates feels a little suspicious to me, but I can see why it's there 
>> for completeness.
> 
> Thanks Paul.
> 
> Re. drop coordinates, note that we're actually using it in 
> MemoryHandles.withStride, so that we can insert a dummy coordinate that is 
> always discarded in case the stride is zero. We could do without it as well, 
> of course, but sometimes it's hard to have a sense of how these pieces might 
> be joined in practice.
> 

Ah, ok I can see its value now when dropping a coordinate that would otherwise 
result in some redundant calculation.  However, looking at this more closely:

* @param bytesStride the stride, in bytes, by which to multiply the coordinate 
value. Must be greater than zero.

It implies that a zero value should be disallowed contrary to the 
implementation.  I am wondering if your intent was support a signed stride 
value?

In this case it could be argued that a zero stride is misleading, if supported, 
since any value passed for the coordinate X has no effect.  But I can also see 
the other side from a position uniformity, and then why not support negative 
strides, which I think the implementation does support.

Paul.

> Maurizio
> 
>> Paul.
>> 
>>> On Apr 23, 2020, at 1:33 PM, 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 the program is 
>>> executed using the -Dforeign.unsafe=permit flag.
>>>   - the MemorySegment::mapFromPath now returns a MappedMemorySegment
>>> * MappedMemorySegment
>>>   - small sub-interface which provides extra capabilities for mapped 
>>> segments (load(), unload() and force())
>>> * MemoryAddress
>>>   - added distinction between *checked* and *unchecked* addresses; 
>>> *unchecked* addresses do not have a segment, so they cannot be dereferenced
>>>   - added NULL memory address (it's an unchecked address)
>>>   - added factory to construct MemoryAddress from long value (result is 
>>> also an unchecked address)
>>>   - added API point to get raw address value (where poss

Re: trivial correction to javadoc of j.l.i.MethodHandles.arrayElementVarHandle()

[Raffaello Giulietti]

Hi Mandy,

OK, I'll file a record in the JBS. Thanks for sponsoring

Raffaello


On 2020-04-24 18:17, Mandy Chung wrote:

Yes any change requires a JBS issue.

I can sponsor this trivial fix.

Mandy

On 4/24/20 5:15 AM, Raffaello Giulietti wrote:

Not sure if this requires an entry in the JBS...


Greetings
Raffaello




diff -r 70c2239696ae 
src/java.base/share/classes/java/lang/invoke/MethodHandles.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 16:12:53 2020 +0530
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 14:08:46 2020 +0200

@@ -3960,7 +3960,7 @@
  * {@code short}, {@code char}, {@code int}, {@code long},
  * {@code float}, or {@code double} then numeric atomic 
update access

  * modes are unsupported.
- * if the field type is anything other than {@code boolean},
+ * if the component type is anything other than {@code boolean},
  * {@code byte}, {@code short}, {@code char}, {@code int} or
  * {@code long} then bitwise atomic update access modes are
  * unsupported.

Re: trivial correction to javadoc of j.l.i.MethodHandles.arrayElementVarHandle()

[Mandy Chung]

Yes any change requires a JBS issue.

I can sponsor this trivial fix.

Mandy

On 4/24/20 5:15 AM, Raffaello Giulietti wrote:

Not sure if this requires an entry in the JBS...


Greetings
Raffaello




diff -r 70c2239696ae 
src/java.base/share/classes/java/lang/invoke/MethodHandles.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 16:12:53 2020 +0530
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 14:08:46 2020 +0200

@@ -3960,7 +3960,7 @@
  * {@code short}, {@code char}, {@code int}, {@code long},
  * {@code float}, or {@code double} then numeric atomic 
update access

  * modes are unsupported.
- * if the field type is anything other than {@code boolean},
+ * if the component type is anything other than {@code boolean},
  * {@code byte}, {@code short}, {@code char}, {@code int} or
  * {@code long} then bitwise atomic update access modes are
  * unsupported.

Re: RFR 15: 8243010: Test support: Customizable Hex Printer

[Daniel Fuchs]

Hi Roger,

On 23/04/2020 19:46, Roger Riggs wrote:

A review would be appreciated.


Ship it!

cheers,

-- daniel

trivial correction to javadoc of j.l.i.MethodHandles.arrayElementVarHandle()

[Raffaello Giulietti]

Not sure if this requires an entry in the JBS...


Greetings
Raffaello




diff -r 70c2239696ae 
src/java.base/share/classes/java/lang/invoke/MethodHandles.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 16:12:53 2020 +0530
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 14:08:46 2020 +0200

@@ -3960,7 +3960,7 @@
  * {@code short}, {@code char}, {@code int}, {@code long},
  * {@code float}, or {@code double} then numeric atomic update 
access

  * modes are unsupported.
- * if the field type is anything other than {@code boolean},
+ * if the component type is anything other than {@code boolean},
  * {@code byte}, {@code short}, {@code char}, {@code int} or
  * {@code long} then bitwise atomic update access modes are
  * unsupported.

Re: RFR(S): 8241638: launcher time metrics alway report 1 on Linux when _JAVA_LAUNCHER_DEBUG set(Internet mail)

[廖彬]

Dear all,

As Lin mentioned, I found the copyright info is not updated, so I prepared a 
patch to fix this. Lin helps me to build an issue in the jdk bug system. Would 
you help me to review this patch? Thank you very much.
Patch link: https://cr.openjdk.java.net/~lzang/BuddyLiao/launcher/webrev/
Issue link: https://bugs.openjdk.java.net/browse/JDK-8243539

BRs,
Bin


在 2020/4/24 上午10:38，“linzang(臧琳)” 写入:

Hi Henry, Alan, David
My colleague Bin(Buddy) just remind me that the copyright info of 
the touched files should be updated, sorry that I  forgot to add it in my 
original patch , do you think there should be a patch for fixing that? 

BRs,
Lin

On 2020/4/7, 4:56 PM, "Alan Bateman"  wrote:

On 06/04/2020 18:36, Henry Jen wrote:
> Here is the combined webrev[1] which I think is what we should have. 
By using __solaris__, we make sure no extra define from build and assuming that 
all solaris build will have gethrtime() and use that.
>
> The current build only use that for static build launcher, not custom 
launcher use libjli.
>
> Cheers,
> Henry
>
> [1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.2/webrev/
>
I think it would be simpler to drop the macros and have function 
prototypes in java_md_sollinux.h. That would avoid preprocessor code in 
the header file. The implementation in java_md_sollinux.c would then be 
simple #ifdef __solaris ... #else ... Minor nit is that the #include 
 should probably be moved up to the top with the other 
includes.

-Alan.

Re: RFR: 8243453: Option --describe-module failed with non-ASCII module name under non-UTF8 environment

[Alan Bateman]

On 24/04/2020 09:33, Toshio 5 Nakamura wrote:

Hi all,

Please review this fix.
Also, I'd like to ask a sponsor of the fix, since I'm not a committer.

Issue: https://bugs.openjdk.java.net/browse/JDK-8243453
Webrev: http://cr.openjdk.java.net/~tnakamura/8243453/webrev.00/

Under Windows Japanese or Chinese, encoding of command line option
cannot be assumed as UTF-8.
This fix changes from "(*env)->NewStringUTF()" to "NewPlatformString()"
to call Helper method.

This looks correct to me (and apologies for this bug, I don't know why I 
used NewStringUTF when adding this option in JDK 9).


-Alan.

RFR: 8243453: Option --describe-module failed with non-ASCII module name under non-UTF8 environment

[Toshio 5 Nakamura]

Hi all,

Please review this fix.
Also, I'd like to ask a sponsor of the fix, since I'm not a committer.

Issue: https://bugs.openjdk.java.net/browse/JDK-8243453
Webrev: http://cr.openjdk.java.net/~tnakamura/8243453/webrev.00/

Under Windows Japanese or Chinese, encoding of command line option
cannot be assumed as UTF-8.
This fix changes from "(*env)->NewStringUTF()" to "NewPlatformString()"
to call Helper method.

Tier 1/2 tests on Linux and Windows passed.

Best Regards,
Toshio Nakamura

Re: RFR 15 8243099: Adding ADQ support to JDK

[Vyom Tiwari]

Hi Vladimir,

In LinuxSocketOptions.c we have lot's of duplicate code, can you please
reuse "socketOptionSupported" & "handleError" as follows, this we remove
lot's of duplicate code.
Thanks,
Vyom

diff -r b6b4506a7827 src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c
--- a/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c Fri Apr 24
15:28:57 2020 +0800
+++ b/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c Fri Apr 24
13:37:36 2020 +0530
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights
reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,10 @@
 #include "jni_util.h"
 #include "jdk_net_LinuxSocketOptions.h"

+#ifndef SO_INCOMING_NAPI_ID
+#define SO_INCOMING_NAPI_ID56
+#endif
+
 /*
  * Class: jdk_net_LinuxSocketOptions
  * Method:setQuickAck
@@ -44,15 +48,7 @@
 int rv;
 optval = (on ? 1 : 0);
 rv = setsockopt(fd, SOL_SOCKET, TCP_QUICKACK, &optval, sizeof
(optval));
-if (rv < 0) {
-if (errno == ENOPROTOOPT) {
-JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
-"unsupported socket option");
-} else {
-JNU_ThrowByNameWithLastError(env, "java/net/SocketException",
-"set option TCP_QUICKACK failed");
-}
-}
+handleError(env, rv, "set option TCP_QUICKACK failed");
 }

 /*
@@ -65,15 +61,7 @@
 int on;
 socklen_t sz = sizeof (on);
 int rv = getsockopt(fd, SOL_SOCKET, TCP_QUICKACK, &on, &sz);
-if (rv < 0) {
-if (errno == ENOPROTOOPT) {
-JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
-"unsupported socket option");
-} else {
-JNU_ThrowByNameWithLastError(env, "java/net/SocketException",
-"get option TCP_QUICKACK failed");
-}
-}
+handleError(env, rv, "get option TCP_QUICKACK failed");
 return on != 0;
 }

@@ -83,31 +71,18 @@
  * Signature: ()Z
  */
 JNIEXPORT jboolean JNICALL
Java_jdk_net_LinuxSocketOptions_quickAckSupported0
-(JNIEnv *env, jobject unused) {
-int one = 1;
-int rv, s;
-s = socket(PF_INET, SOCK_STREAM, 0);
-if (s < 0) {
-return JNI_FALSE;
-}
-rv = setsockopt(s, SOL_SOCKET, TCP_QUICKACK, (void *) &one, sizeof
(one));
-if (rv != 0 && errno == ENOPROTOOPT) {
-rv = JNI_FALSE;
-} else {
-rv = JNI_TRUE;
-}
-close(s);
-return rv;
+(JNIEnv *env, jobject unused) {
+return socketOptionSupported(SOL_SOCKET, TCP_QUICKACK);
 }

-static jint socketOptionSupported(jint sockopt) {
+static jint socketOptionSupported(jint level, jint optname) {
 jint one = 1;
 jint rv, s;
 s = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
 if (s < 0) {
 return 0;
 }
-rv = setsockopt(s, SOL_TCP, sockopt, (void *) &one, sizeof (one));
+rv = setsockopt(s, level, optname, (void *) &one, sizeof (one));
 if (rv != 0 && errno == ENOPROTOOPT) {
 rv = 0;
 } else {
@@ -135,8 +110,8 @@
  */
 JNIEXPORT jboolean JNICALL
Java_jdk_net_LinuxSocketOptions_keepAliveOptionsSupported0
 (JNIEnv *env, jobject unused) {
-return socketOptionSupported(TCP_KEEPIDLE) &&
socketOptionSupported(TCP_KEEPCNT)
-&& socketOptionSupported(TCP_KEEPINTVL);
+return socketOptionSupported(SOL_TCP, TCP_KEEPIDLE) &&
socketOptionSupported(SOL_TCP, TCP_KEEPCNT)
+&& socketOptionSupported(SOL_TCP, TCP_KEEPINTVL);
 }

 /*
@@ -213,3 +188,27 @@
 handleError(env, rv, "get option TCP_KEEPINTVL failed");
 return optval;
 }
+
+/*
+ * Class: jdk_net_LinuxSocketOptions
+ * Method:IncomingNapiIdSupported0
+ * Signature: (I)I;
+ */
+JNIEXPORT jboolean JNICALL
Java_jdk_net_LinuxSocketOptions_IncomingNapiIdSupported0
+(JNIEnv *env, jobject unused) {
+return socketOptionSupported(SOL_SOCKET, SO_INCOMING_NAPI_ID);
+}
+
+/*
+ * Class: jdk_net_LinuxSocketOptions
+ * Method:getIncomingNapiId0
+ * Signature: (I)I;
+ */
+JNIEXPORT jint JNICALL Java_jdk_net_LinuxSocketOptions_getIncomingNapiId0
+(JNIEnv *env, jobject unused, jint fd) {
+jint optval, rv;
+socklen_t sz = sizeof (optval);
+rv = getsockopt(fd, SOL_SOCKET, SO_INCOMING_NAPI_ID, &optval, &sz);
+handleError(env, rv, "get option SO_INCOMING_NAPI_ID failed");
+return optval;
+}

On Fri, Apr 24, 2020 at 12:43 AM Ivanov, Vladimir A <
vladimir.a.iva...@intel.com> wrote:

> Thanks a lot to Chris and Alan for detailed comments.
> The updated version of patch available at
> http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.01/
>
> Changes:
> 1. in native code the common pattern was used for the 'getsockopt' call.
> 2. condition to define SO_INCOMING_NAPI_ID was added.
> 3. t