Re: RFR: 8244154: Update SunPKCS11 provider with PKCS11 v3.0 header files

2020-10-30 Thread Weijun Wang
On Fri, 30 Oct 2020 21:55:10 GMT, Valerie Peng  wrote:

> > The constants in PKCS11Exception are duplicated in PKCS11Constants.
> > ```
> > 0x,
> > ```
> > 
> > 
> > vs
> > ```
> > public static final long  CKR_OK = 0xL;
> > ```
> > 
> > 
> > Is there any way to simplify it?
> 
> One defines the value, the other defines the displayed String. I agree that 
> the way it's currently done is a bit outdated and may be error prone. We can 
> re-visit this after finishing other pending PKCS11 RFEs..

Since the value is already defined PKCS11Constants, we can use it in 
PKCSException and does not need to write 0x anymore.

-

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


Integrated: 8255494: PKCS7 should use digest algorithm to verify the signature

2020-10-30 Thread Weijun Wang
On Wed, 28 Oct 2020 21:01:44 GMT, Weijun Wang  wrote:

> This is a regression made by 
> [JDK-8242068](https://bugs.openjdk.java.net/browse/JDK-8242068). When the 
> digest algorithm is not the same as the hash part of the signature algorithm, 
> we used to combine the digest algorithm with the key part of the signature 
> algorithm into a new signature algorithm and use it when generating a 
> signature. The previous code change uses the signature algorithm in the 
> SignerInfo directly. This bugfix will revert to the old behavior.

This pull request has now been integrated.

Changeset: 80380d51
Author:Weijun Wang 
URL:   https://git.openjdk.java.net/jdk/commit/80380d51
Stats: 129 lines in 3 files changed: 116 ins; 5 del; 8 mod

8255494: PKCS7 should use digest algorithm to verify the signature

Reviewed-by: valeriep

-

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


Re: RFR: 8244154: Update SunPKCS11 provider with PKCS11 v3.0 header files

2020-10-30 Thread Hai-May Chao
On Fri, 30 Oct 2020 21:44:00 GMT, Valerie Peng  wrote:

>> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11Constants.java
>>  line 987:
>> 
>>> 985: public static final long  CKM_SP800_108_FEEDBACK_KDF = 
>>> 0x03adL;
>>> 986: public static final long  CKM_SP800_108_DOUBLE_PIPELINE_KDF = 
>>> 0x03aeL;
>>> 987: 
>> 
>> Same comment.
>
> These three are just by themselves, so unless you feel strongly about, I 
> prefer just leave them here which matches the ordering of pkcs11t.h, i.e. 
> right before the CKM_VENDOR_DEFINED line.

Just thought they could be moved like CKM_ECDSA_SHA3_224 to CKM_EDDSA (which 
are not matching to the ordering of pkcs11t.h), so we keep the ordering in 
PKCS11Constants. It's fine if you'd prefer as is.

-

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


Re: RFR: 8244154: Update SunPKCS11 provider with PKCS11 v3.0 header files

2020-10-30 Thread Hai-May Chao
On Fri, 30 Oct 2020 21:39:42 GMT, Valerie Peng  wrote:

>> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/Functions.java
>>  line 1095:
>> 
>>> 1093: addMech(CKM_SP800_108_FEEDBACK_KDF, 
>>> "CKM_SP800_108_FEEDBACK_KDF");
>>> 1094: addMech(CKM_SP800_108_DOUBLE_PIPELINE_KDF,
>>> 1095:  
>>> "CKM_SP800_108_DOUBLE_PIPELINE_KDF");
>> 
>> same comment as above.
>
> Well, per the ordering in PKCS11Constants, these three lines are at the right 
> place. Note that the ordering of CKM_ECDSA_SHA3_224 to CKM_EDDSA in pkcs11t.h 
> is different from PKCS11Constants class, so I will add the comment about the 
> general ordering following PKCS11Constants class and keep them here.

Sounds good that a comment will be added.

-

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


Re: RFR: 8255494: PKCS7 should use digest algorithm to verify the signature [v2]

2020-10-30 Thread Valerie Peng
On Thu, 29 Oct 2020 18:37:06 GMT, Weijun Wang  wrote:

>> This is a regression made by 
>> [JDK-8242068](https://bugs.openjdk.java.net/browse/JDK-8242068). When the 
>> digest algorithm is not the same as the hash part of the signature 
>> algorithm, we used to combine the digest algorithm with the key part of the 
>> signature algorithm into a new signature algorithm and use it when 
>> generating a signature. The previous code change uses the signature 
>> algorithm in the SignerInfo directly. This bugfix will revert to the old 
>> behavior.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   more comment to the test, and full DER encoding

Marked as reviewed by valeriep (Reviewer).

-

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


Re: RFR: 8255494: PKCS7 should use digest algorithm to verify the signature

2020-10-30 Thread Valerie Peng
On Thu, 29 Oct 2020 18:57:45 GMT, Hai-May Chao  wrote:

>> This is a regression made by 
>> [JDK-8242068](https://bugs.openjdk.java.net/browse/JDK-8242068). When the 
>> digest algorithm is not the same as the hash part of the signature 
>> algorithm, we used to combine the digest algorithm with the key part of the 
>> signature algorithm into a new signature algorithm and use it when 
>> generating a signature. The previous code change uses the signature 
>> algorithm in the SignerInfo directly. This bugfix will revert to the old 
>> behavior.
>
> Looks good!

Looks good to me.

-

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


Re: RFR: 8244154: Update SunPKCS11 provider with PKCS11 v3.0 header files

2020-10-30 Thread Valerie Peng
On Fri, 30 Oct 2020 14:43:20 GMT, Weijun Wang  wrote:

> 
> 
> The constants in PKCS11Exception are duplicated in PKCS11Constants.
> 
> ```
> 0x,
> ```
> 
> vs
> 
> ```
> public static final long  CKR_OK = 0xL;
> ```
> 
> Is there any way to simplify it?

One defines the value, the other defines the displayed String. I agree that the 
way it's currently done is a bit outdated and may be error prone. We can 
re-visit this after finishing other pending PKCS11 RFEs..

-

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


Re: RFR: 8244154: Update SunPKCS11 provider with PKCS11 v3.0 header files

2020-10-30 Thread Valerie Peng
On Thu, 29 Oct 2020 03:18:33 GMT, Weijun Wang  wrote:

> 
> 
> Just curious, can the Java files be generated during the build process?

Hmm, maybe, by the java files, do you just mean PKCS11Constants class or more? 
I am not familiar with how to generate Java files during the build process, can 
you share some pointers? I can look into them for possible future enhancement.

-

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


Re: RFR: 8244154: Update SunPKCS11 provider with PKCS11 v3.0 header files

2020-10-30 Thread Valerie Peng
On Thu, 29 Oct 2020 02:16:15 GMT, Hai-May Chao  wrote:

>> Could someone please help review this PKCS#11 v3.0 header files update?
>> 
>> Changes are straight-forward as below:
>> 1) Updated pkcs11.h, pkcs11f.h, pkcs11t.h to v3.0
>> 2) Updated java side w/ the new constants definitions and name/error code 
>> mappings.
>> 
>> For the native headers, it's a direct copy of the official v3.0 headers 
>> except that I have to remove the tab space, and trailing white spaces due to 
>> JDK code requirement. I verified the result using 'diff -w'. As for the java 
>> side, the edit is based on the diff of native headers. I also commented out 
>> some of the unused native identifiers at java side.
>> 
>> I am adding the SHA-3 digests, signatures, and macs in a separate RFE and 
>> would need this one to be reviewed/integrated first.
>> 
>> Thanks,
>> Valerie
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11Constants.java
>  line 987:
> 
>> 985: public static final long  CKM_SP800_108_FEEDBACK_KDF = 
>> 0x03adL;
>> 986: public static final long  CKM_SP800_108_DOUBLE_PIPELINE_KDF = 
>> 0x03aeL;
>> 987: 
> 
> Same comment.

These three are just by themselves, so unless you feel strongly about, I prefer 
just leave them here which matches the ordering of pkcs11t.h, i.e. right before 
the CKM_VENDOR_DEFINED line.

-

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


Re: RFR: 8244154: Update SunPKCS11 provider with PKCS11 v3.0 header files

2020-10-30 Thread Valerie Peng
On Thu, 29 Oct 2020 02:07:39 GMT, Hai-May Chao  wrote:

>> Could someone please help review this PKCS#11 v3.0 header files update?
>> 
>> Changes are straight-forward as below:
>> 1) Updated pkcs11.h, pkcs11f.h, pkcs11t.h to v3.0
>> 2) Updated java side w/ the new constants definitions and name/error code 
>> mappings.
>> 
>> For the native headers, it's a direct copy of the official v3.0 headers 
>> except that I have to remove the tab space, and trailing white spaces due to 
>> JDK code requirement. I verified the result using 'diff -w'. As for the java 
>> side, the edit is based on the diff of native headers. I also commented out 
>> some of the unused native identifiers at java side.
>> 
>> I am adding the SHA-3 digests, signatures, and macs in a separate RFE and 
>> would need this one to be reviewed/integrated first.
>> 
>> Thanks,
>> Valerie
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/Functions.java
>  line 1095:
> 
>> 1093: addMech(CKM_SP800_108_FEEDBACK_KDF, 
>> "CKM_SP800_108_FEEDBACK_KDF");
>> 1094: addMech(CKM_SP800_108_DOUBLE_PIPELINE_KDF,
>> 1095:  
>> "CKM_SP800_108_DOUBLE_PIPELINE_KDF");
> 
> same comment as above.

Well, per the ordering in PKCS11Constants, these three lines are at the right 
place. Note that the ordering of CKM_ECDSA_SHA3_224 to CKM_EDDSA in pkcs11t.h 
is different from PKCS11Constants class, so I will add the comment about the 
general ordering following PKCS11Constants class and keep them here.

-

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


Re: RFR: 8244154: Update SunPKCS11 provider with PKCS11 v3.0 header files

2020-10-30 Thread Valerie Peng
On Thu, 29 Oct 2020 02:06:06 GMT, Hai-May Chao  wrote:

>> Could someone please help review this PKCS#11 v3.0 header files update?
>> 
>> Changes are straight-forward as below:
>> 1) Updated pkcs11.h, pkcs11f.h, pkcs11t.h to v3.0
>> 2) Updated java side w/ the new constants definitions and name/error code 
>> mappings.
>> 
>> For the native headers, it's a direct copy of the official v3.0 headers 
>> except that I have to remove the tab space, and trailing white spaces due to 
>> JDK code requirement. I verified the result using 'diff -w'. As for the java 
>> side, the edit is based on the diff of native headers. I also commented out 
>> some of the unused native identifiers at java side.
>> 
>> I am adding the SHA-3 digests, signatures, and macs in a separate RFE and 
>> would need this one to be reviewed/integrated first.
>> 
>> Thanks,
>> Valerie
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/Functions.java
>  line 793:
> 
>> 791: addMech(CKM_SHA3_512_RSA_PKCS_PSS,  
>> "CKM_SHA3_512_RSA_PKCS_PSS");
>> 792: addMech(CKM_SHA3_224_RSA_PKCS,  
>> "CKM_SHA3_224_RSA_PKCS");
>> 793: addMech(CKM_SHA3_224_RSA_PKCS_PSS,  
>> "CKM_SHA3_224_RSA_PKCS_PSS");
> 
> It appears that you're arranging the addMech(with CKM_xxx) based on the 
> mechanism values. How about the code from #773 to #793, move it up?

Yes, I will move it up and add a comment about overall ordering is based on 
PKCS11Constants class.

-

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


Re: RFR: 8244154: Update SunPKCS11 provider with PKCS11 v3.0 header files

2020-10-30 Thread Weijun Wang
On Thu, 29 Oct 2020 03:18:33 GMT, Weijun Wang  wrote:

>> Changes look good. Only minor comments.
>
> Just curious, can the Java files be generated during the build process?

The constants in PKCS11Exception are duplicated in PKCS11Constants.
0x,
vs
public static final long  CKR_OK = 0xL;
Is there any way to simplify it?

-

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


Integrated: 8255536: Remove the directsign property and option

2020-10-30 Thread Weijun Wang
On Wed, 28 Oct 2020 20:56:31 GMT, Weijun Wang  wrote:

> I regret adding the `directsign` property/option to JarSigner/jarsigner. See 
> the bug for details.

This pull request has now been integrated.

Changeset: a7563207
Author:Weijun Wang 
URL:   https://git.openjdk.java.net/jdk/commit/a7563207
Stats: 184 lines in 7 files changed: 14 ins; 162 del; 8 mod

8255536: Remove the directsign property and option

Reviewed-by: mullan

-

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


Re: RFR: 8153005: Upgrade the default PKCS12 encryption/MAC algorithms [v4]

2020-10-30 Thread Weijun Wang
> Default algorithms are bumped to be based on PBES2 with AES-256 and SHA-256. 
> Please also review the CSR at 
> https://bugs.openjdk.java.net/browse/JDK-8228481.

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

 - simplify test
 - merge
 - update README and exclude README
 - change ic to 1
 - 8153005: Upgrade the default PKCS12 encryption/MAC algorithms

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/473/files
  - new: https://git.openjdk.java.net/jdk/pull/473/files/41be78aa..31a22fd4

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

  Stats: 401475 lines in 1918 files changed: 365145 ins; 23887 del; 12443 mod
  Patch: https://git.openjdk.java.net/jdk/pull/473.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/473/head:pull/473

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


Integrated: 8153005: Upgrade the default PKCS12 encryption/MAC algorithms

2020-10-30 Thread Weijun Wang
On Thu, 1 Oct 2020 20:02:34 GMT, Weijun Wang  wrote:

> Default algorithms are bumped to be based on PBES2 with AES-256 and SHA-256. 
> Please also review the CSR at 
> https://bugs.openjdk.java.net/browse/JDK-8228481.

This pull request has now been integrated.

Changeset: f77a6585
Author:Weijun Wang 
URL:   https://git.openjdk.java.net/jdk/commit/f77a6585
Stats: 511 lines in 8 files changed: 172 ins; 113 del; 226 mod

8153005: Upgrade the default PKCS12 encryption/MAC algorithms

Reviewed-by: mullan

-

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


Re: RFR: 8153005: Upgrade the default PKCS12 encryption/MAC algorithms [v3]

2020-10-30 Thread Sean Mullan
On Fri, 9 Oct 2020 01:33:38 GMT, Weijun Wang  wrote:

>> Default algorithms are bumped to be based on PBES2 with AES-256 and SHA-256. 
>> Please also review the CSR at 
>> https://bugs.openjdk.java.net/browse/JDK-8228481.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update README and exclude README

Marked as reviewed by mullan (Reviewer).

-

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


Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v14]

2020-10-30 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of 
> the foreign linker access API incubation
> (see JEP 389 [1]). This work is meant to sit on top of the foreign memory 
> access support (see JEP 393 [2] and associated pull request [3]).
> 
> The main goal of this API is to provide a way to call native functions from 
> Java code without the need of intermediate JNI glue code. In order to do 
> this, native calls are modeled through the MethodHandle API. I suggest 
> reading the writeup [4] I put together few weeks ago, which illustrates what 
> the foreign linker support is, and how it should be used by clients.
> 
> Disclaimer: the pull request mechanism isn't great at managing *dependent* 
> reviews. For this reasons, I'm attaching a webrev which contains only the 
> differences between this PR and the memory access PR. I will be periodically 
> uploading new webrevs, as new iterations come out, to try and make the life 
> of reviewers as simple as possible.
> 
> A big thank to Jorn Vernee and Vladimir Ivanov - they are the main architects 
> of all the hotspot changes you see here, and without their help, the foreign 
> linker support wouldn't be what it is today. As usual, a big thank to Paul 
> Sandoz, who provided many insights (often by trying the bits first hand).
> 
> Thanks
> Maurizio
> 
> Webrev:
> http://cr.openjdk.java.net/~mcimadamore/8254231_v1/webrev
> 
> Javadoc:
> 
> http://cr.openjdk.java.net/~mcimadamore/8254231_v1/javadoc/jdk/incubator/foreign/package-summary.html
> 
> Specdiff (relative to [3]):
> 
> http://cr.openjdk.java.net/~mcimadamore/8254231_v1/specdiff_delta/overview-summary.html
> 
> CSR:
> 
> https://bugs.openjdk.java.net/browse/JDK-8254232
> 
> 
> 
> ### API Changes
> 
> The API changes are actually rather slim:
> 
> * `LibraryLookup`
>   * This class allows clients to lookup symbols in native libraries; the 
> interface is fairly simple; you can load a library by name, or absolute path, 
> and then lookup symbols on that library.
> * `FunctionDescriptor`
>   * This is an abstraction that is very similar, in spirit, to `MethodType`; 
> it is, at its core, an aggregate of memory layouts for the function 
> arguments/return type. A function descriptor is used to describe the 
> signature of a native function.
> * `CLinker`
>   * This is the real star of the show. A `CLinker` has two main methods: 
> `downcallHandle` and `upcallStub`; the first takes a native symbol (as 
> obtained from `LibraryLookup`), a `MethodType` and a `FunctionDescriptor` and 
> returns a `MethodHandle` instance which can be used to call the target native 
> symbol. The second takes an existing method handle, and a 
> `FunctionDescriptor` and returns a new `MemorySegment` corresponding to a 
> code stub allocated by the VM which acts as a trampoline from native code to 
> the user-provided method handle. This is very useful for implementing upcalls.
>* This class also contains the various layout constants that should be 
> used by clients when describing native signatures (e.g. `C_LONG` and 
> friends); these layouts contain additional ABI classfication information (in 
> the form of layout attributes) which is used by the runtime to *infer* how 
> Java arguments should be shuffled for the native call to take place.
>   * Finally, this class provides some helper functions e.g. so that clients 
> can convert Java strings into C strings and back.
> * `NativeScope`
>   * This is an helper class which allows clients to group together logically 
> related allocations; that is, rather than allocating separate memory segments 
> using separate *try-with-resource* constructs, a `NativeScope` allows clients 
> to use a _single_ block, and allocate all the required segments there. This 
> is not only an usability boost, but also a performance boost, since not all 
> allocation requests will be turned into `malloc` calls.
> * `MemorySegment`
>   * Only one method added here - namely `handoff(NativeScope)` which allows a 
> segment to be transferred onto an existing native scope.
> 
> ### Safety
> 
> The foreign linker API is intrinsically unsafe; many things can go wrong when 
> requesting a native method handle. For instance, the description of the 
> native signature might be wrong (e.g. have too many arguments) - and the 
> runtime has, in the general case, no way to detect such mismatches. For these 
> reasons, obtaining a `CLinker` instance is a *restricted* operation, which 
> can be enabled by specifying the usual JDK property 
> `-Dforeign.restricted=permit` (as it's the case for other restricted method 
> in the foreign memory API).
> 
> ### Implementation changes
> 
> The Java changes associated with `LibraryLookup` are relative 
> straightforward; the only interesting thing to note here is that library 
> loading does _not_ depend on class loaders, so `LibraryLookup` is not subject 
> to the same restrictions which apply to JNI library loading (e.g. same 
> library 

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v13]

2020-10-30 Thread Maurizio Cimadamore
> This patch contains the changes associated with the first incubation round of 
> the foreign linker access API incubation
> (see JEP 389 [1]). This work is meant to sit on top of the foreign memory 
> access support (see JEP 393 [2] and associated pull request [3]).
> 
> The main goal of this API is to provide a way to call native functions from 
> Java code without the need of intermediate JNI glue code. In order to do 
> this, native calls are modeled through the MethodHandle API. I suggest 
> reading the writeup [4] I put together few weeks ago, which illustrates what 
> the foreign linker support is, and how it should be used by clients.
> 
> Disclaimer: the pull request mechanism isn't great at managing *dependent* 
> reviews. For this reasons, I'm attaching a webrev which contains only the 
> differences between this PR and the memory access PR. I will be periodically 
> uploading new webrevs, as new iterations come out, to try and make the life 
> of reviewers as simple as possible.
> 
> A big thank to Jorn Vernee and Vladimir Ivanov - they are the main architects 
> of all the hotspot changes you see here, and without their help, the foreign 
> linker support wouldn't be what it is today. As usual, a big thank to Paul 
> Sandoz, who provided many insights (often by trying the bits first hand).
> 
> Thanks
> Maurizio
> 
> Webrev:
> http://cr.openjdk.java.net/~mcimadamore/8254231_v1/webrev
> 
> Javadoc:
> 
> http://cr.openjdk.java.net/~mcimadamore/8254231_v1/javadoc/jdk/incubator/foreign/package-summary.html
> 
> Specdiff (relative to [3]):
> 
> http://cr.openjdk.java.net/~mcimadamore/8254231_v1/specdiff_delta/overview-summary.html
> 
> CSR:
> 
> https://bugs.openjdk.java.net/browse/JDK-8254232
> 
> 
> 
> ### API Changes
> 
> The API changes are actually rather slim:
> 
> * `LibraryLookup`
>   * This class allows clients to lookup symbols in native libraries; the 
> interface is fairly simple; you can load a library by name, or absolute path, 
> and then lookup symbols on that library.
> * `FunctionDescriptor`
>   * This is an abstraction that is very similar, in spirit, to `MethodType`; 
> it is, at its core, an aggregate of memory layouts for the function 
> arguments/return type. A function descriptor is used to describe the 
> signature of a native function.
> * `CLinker`
>   * This is the real star of the show. A `CLinker` has two main methods: 
> `downcallHandle` and `upcallStub`; the first takes a native symbol (as 
> obtained from `LibraryLookup`), a `MethodType` and a `FunctionDescriptor` and 
> returns a `MethodHandle` instance which can be used to call the target native 
> symbol. The second takes an existing method handle, and a 
> `FunctionDescriptor` and returns a new `MemorySegment` corresponding to a 
> code stub allocated by the VM which acts as a trampoline from native code to 
> the user-provided method handle. This is very useful for implementing upcalls.
>* This class also contains the various layout constants that should be 
> used by clients when describing native signatures (e.g. `C_LONG` and 
> friends); these layouts contain additional ABI classfication information (in 
> the form of layout attributes) which is used by the runtime to *infer* how 
> Java arguments should be shuffled for the native call to take place.
>   * Finally, this class provides some helper functions e.g. so that clients 
> can convert Java strings into C strings and back.
> * `NativeScope`
>   * This is an helper class which allows clients to group together logically 
> related allocations; that is, rather than allocating separate memory segments 
> using separate *try-with-resource* constructs, a `NativeScope` allows clients 
> to use a _single_ block, and allocate all the required segments there. This 
> is not only an usability boost, but also a performance boost, since not all 
> allocation requests will be turned into `malloc` calls.
> * `MemorySegment`
>   * Only one method added here - namely `handoff(NativeScope)` which allows a 
> segment to be transferred onto an existing native scope.
> 
> ### Safety
> 
> The foreign linker API is intrinsically unsafe; many things can go wrong when 
> requesting a native method handle. For instance, the description of the 
> native signature might be wrong (e.g. have too many arguments) - and the 
> runtime has, in the general case, no way to detect such mismatches. For these 
> reasons, obtaining a `CLinker` instance is a *restricted* operation, which 
> can be enabled by specifying the usual JDK property 
> `-Dforeign.restricted=permit` (as it's the case for other restricted method 
> in the foreign memory API).
> 
> ### Implementation changes
> 
> The Java changes associated with `LibraryLookup` are relative 
> straightforward; the only interesting thing to note here is that library 
> loading does _not_ depend on class loaders, so `LibraryLookup` is not subject 
> to the same restrictions which apply to JNI library loading (e.g. same 
> library 

Re: RFR: 8254162: Implementation of Foreign-Memory Access API (Third Incubator) [v19]

2020-10-30 Thread Maurizio Cimadamore
> This patch contains the changes associated with the third incubation round of 
> the foreign memory access API incubation  (see JEP 393 [1]). This iteration 
> focus on improving the usability of the API in 3 main ways:
> 
> * first, by providing a way to obtain truly *shared* segments, which can be 
> accessed and closed concurrently from multiple threads
> * second, by providing a way to register a memory segment against a 
> `Cleaner`, so as to have some (optional) guarantee that the memory will be 
> deallocated, eventually
> * third, by not requiring users to dive deep into var handles when they first 
> pick up the API; a new `MemoryAccess` class has been added, which defines 
> several useful dereference routines; these are really just thin wrappers 
> around memory access var handles, but they make the barrier of entry for 
> using this API somewhat lower.
> 
> A big conceptual shift that comes with this API refresh is that the role of 
> `MemorySegment` and `MemoryAddress` is not the same as it used to be; it used 
> to be the case that a memory address could (sometimes, not always) have a 
> back link to the memory segment which originated it; additionally, memory 
> access var handles used `MemoryAddress` as a basic unit of dereference.
> 
> This has all changed as per this API refresh;  now a `MemoryAddress` is just 
> a dumb carrier which wraps a pair of object/long addressing coordinates; 
> `MemorySegment` has become the star of the show, as far as dereferencing 
> memory is concerned. You cannot dereference memory if you don't have a 
> segment. This improves usability in a number of ways - first, it is a lot 
> easier to wrap native addresses (`long`, essentially) into a `MemoryAddress`; 
> secondly, it is crystal clear what a client has to do in order to dereference 
> memory: if a client has a segment, it can use that; otherwise, if the client 
> only has an address, it will have to create a segment *unsafely* (this can be 
> done by calling `MemoryAddress::asSegmentRestricted`).
> 
> 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. 
> 
> A big thank to Erik Osterlund, Vladimir Ivanov and David Holmes, without whom 
> the work on shared memory segment would not have been possible; also I'd like 
> to thank Paul Sandoz, whose insights on API design have been very helpful in 
> this journey.
> 
> Thanks 
> Maurizio 
> 
> Javadoc: 
> 
> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/javadoc/jdk/incubator/foreign/package-summary.html
> 
> Specdiff: 
> 
> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/specdiff/jdk/incubator/foreign/package-summary.html
> 
> CSR: 
> 
> https://bugs.openjdk.java.net/browse/JDK-8254163
> 
> 
> 
> ### API Changes
> 
> * `MemorySegment`
>   * drop factory for restricted segment (this has been moved to 
> `MemoryAddress`, see below)
>   * added a no-arg factory for a native restricted segment representing 
> entire native heap
>   * rename `withOwnerThread` to `handoff`
>   * add new `share` method, to create shared segments
>   * add new `registerCleaner` method, to register a segment against a cleaner
>   * add more helpers to create arrays from a segment e.g. `toIntArray`
>   * add some `asSlice` overloads (to make up for the fact that now segments 
> are more frequently used as cursors)
>   * rename `baseAddress` to `address` (so that `MemorySegment` can implement 
> `Addressable`)
> * `MemoryAddress`
>   * drop `segment` accessor
>   * drop `rebase` method and replace it with `segmentOffset` which returns 
> the offset (a `long`) of this address relative to a given segment
> * `MemoryAccess`
>   * New class supporting several static dereference helpers; the helpers are 
> organized by carrier and access mode, where a carrier is one of the usual 
> suspect (a Java primitive, minus `boolean`); the access mode can be simple 
> (e.g. access base address of given segment), or indexed, in which case the 
> accessor takes a segment and either a low-level byte offset,or a high level 
> logical index. The classification is reflected in the naming scheme (e.g. 
> `getByte` vs. `getByteAtOffset` vs `getByteAtIndex`).
> * `MemoryHandles`
>   * drop `withOffset` combinator
>   * drop `withStride` combinator
>   * the basic memory access handle factory now returns a var handle which 
> takes a `MemorySegment` and a `long` - from which it is easy to derive all 
> the other handles using plain var handle combinators.
> * `Addressable`
>   * This is a new interface which is attached to entities which can be 
> projected to a `MemoryAddress`. For now, both `MemoryAddress` and 
> `MemorySegment` implement it; we have plans, with JEP 389 [2] to add more 
> implementations. Clients can largely ignore this interface, which comes in 
> really handy when 

Re: RFR: JDK-8255603: Memory/Performance regression after JDK-8210985

2020-10-30 Thread Andrew Haley
On Thu, 29 Oct 2020 15:11:09 GMT, Christoph Langer  wrote:

> It seems that there exists a memory/performance regression that was 
> introduced with JDK-8210985: Update the default SSL session cache size to 
> 20480.
> 
> The idea to limit the maixmum SSL session cache size by itself is good. 
> Unfortunately, as per the current implementation of 
> sun.security.util.MemoryCache, it also modifies the initial size of the 
> LinkedHashMap that is backing the cache to initialize with more than the 
> maximum size.
> 
> I suggest to restore the original behavior that initializes with an 
> initialCapacity of 1 in most cases. That was true when before JDK-8210985, 
> the property javax.net.ssl.sessionCacheSize was not set at all.
> In case it was set, the initial size would have been like now, 
> (javax.net.ssl.sessionCacheSize / 0.75f) + 1, which still seems strange.

Marked as reviewed by aph (Reviewer).

src/java.base/share/classes/sun/security/util/Cache.java line 268:

> 266: this.queue = null;
> 267: 
> 268: cacheMap = new LinkedHashMap<>(1, LOAD_FACTOR, true);

This looks right. The idea of scaling the initial cache size to the maximum 
size seems obviously to be wrong.

-

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