Re: Please add HMAC keygen to SunPKCS11

[Bernd Eckenfels]

Hello,

I would agree with this request, my usecase would be to use a HSM, where I 
typically don’t want to import keys but generate them safely on the HSM so not 
even admins have access to the key  material ever (besides maybe having a key 
handle to wrap it). Isn’t that what the KeyGen interface is all about?

Such cases are not tha easy to model with the current abstract PKCS11 Support 
it seems.

Gruss
Bernd
--
http://bernd.eckenfels.net

Von: security-dev  im Auftrag von Valerie 
Peng 
Gesendet: Saturday, October 24, 2020 3:18:56 AM
An: security-dev@openjdk.java.net 
Betreff: Re: Please add HMAC keygen to SunPKCS11


Hi, Justin,

Most callers just wrap the HMAC key bytes into a java SecretKey object, e.g. 
new SecretKeySpec(keyBytes, "HmacSHA256"), pass that into the HMAC impl from 
SunPKCS11 provider which will then convert it into a CKK_GENERIC_SECRET key and 
passing that to underlying PKCS11 library.

Maybe for some very specific cases, support CKM_GENERIC_SECRET_KEY_GEN is 
necessary and I can look into that. For determining the priority on this, would 
the java SecretKey object address your need? Or is there other reason requiring 
3rd party utility?

Thanks,
Valerie


On 10/21/2020 8:44 PM, Justin Cranford wrote:

Compare SunPKCS11 support for AES vs HMAC

  *   AES => keygen is supported, and AES key can be used for encrypt and 
decrypt.
  *   HMAC => keygen is not supported, but HMAC key can be used for MAC.



This does not make sense. A third-party utility is required for HMAC keygen, 
but not for AES keygen.



Use case:

  *   PKCS#11 driver is v2.20.
  *   This means AES-256-GCM is not available for confidentiality and 
integrity, because GCM supported was only added in PKCS#11 v2.40.
  *   Fallback to AES-256-CBC and HmacSha256 is required for confidentiality 
and integrity, respectively.
  *   Java can trigger AES keygen, but not HMAC keygen. A third-party utility 
is required to trigger HMAC keygen before running Java.



Would it be possible to add the missing GENERIC-SECRET-KEY-GEN mechanism to 
SunPKCS11? Notice how that mechanism is missing from the documented SunPKCS11 
algorithms and mechanisms. It is the same in Java 8 all the way up to 15.

  *   
https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html#ALG







To reproduce and demonstrate the missing HMAC keygen issue, here is a small 
Java Maven project.

  *   https://github.com/justincranford/pkcs11



The readme shows the commands to initialize the SoftHSM2 token, and use a 
third-party OpenSC utility to trigger HMAC keygen. It also shows how to set the 
required SoftHSM2 env variable and run the Maven build.



The Maven build will execute the ITPkcs11.java integration test class. The 
tests demonstrate:

  *   Successful SunPKCS11 login to SoftHSM2 and list any existing keys
  *   Successful AES keygen, encrypt, decrypt
  *   Successful HMAC mac
  *   Failed HMAC keygen (because SunPKCS11 does not support 
GENERIC-SECRET-KEY-GEN mechanism yet)







Thank you,

Justin Cranford

Re: Please add HMAC keygen to SunPKCS11

[Valerie Peng]

Hi, Justin,

Most callers just wrap the HMAC key bytes into a java SecretKey object, 
e.g. new SecretKeySpec(keyBytes, "HmacSHA256"), pass that into the HMAC 
impl from SunPKCS11 provider which will then convert it into a 
CKK_GENERIC_SECRET key and passing that to underlying PKCS11 library.


Maybe for some very specific cases, support CKM_GENERIC_SECRET_KEY_GEN 
is necessary and I can look into that. For determining the priority on 
this, would the java SecretKey object address your need? Or is there 
other reason requiring 3rd party utility?


Thanks,
Valerie


On 10/21/2020 8:44 PM, Justin Cranford wrote:


Compare SunPKCS11 support for AES vs HMAC

  * AES => keygen is supported, and AES key can be used for encrypt
and decrypt.
  * HMAC => keygen is not supported, but HMAC key can be used for MAC.

This does not make sense. A third-party utility is required for HMAC 
keygen, but not for AES keygen.


Use case:

  * PKCS#11 driver is v2.20.
  * This means AES-256-GCM is not available for confidentiality and
integrity, because GCM supported was only added in PKCS#11 v2.40.
  * Fallback to AES-256-CBC and HmacSha256 is required for
confidentiality and integrity, respectively.
  * Java can trigger AES keygen, but not HMAC keygen. A third-party
utility is required to trigger HMAC keygen before running Java.

Would it be possible to add the missing GENERIC-SECRET-KEY-GEN 
mechanism to SunPKCS11? Notice how that mechanism is missing from the 
documented SunPKCS11 algorithms and mechanisms. It is the same in Java 
8 all the way up to 15.


  * 
https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html#ALG



To reproduce and demonstrate the missing HMAC keygen issue, here is a 
small Java Maven project.


  * https://github.com/justincranford/pkcs11


The readme shows the commands to initialize the SoftHSM2 token, and 
use a third-party OpenSC utility to trigger HMAC keygen. It also shows 
how to set the required SoftHSM2 env variable and run the Maven build.


The Maven build will execute the ITPkcs11.java integration test class. 
The tests demonstrate:


  * Successful SunPKCS11 login to SoftHSM2 and list any existing keys
  * Successful AES keygen, encrypt, decrypt
  * Successful HMAC mac
  * Failed HMAC keygen (because SunPKCS11 does not support
GENERIC-SECRET-KEY-GEN mechanism yet)

Thank you,

Justin Cranford

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Daniel Fuchs]

On Fri, 23 Oct 2020 09:14:48 GMT, Daniel Fuchs  wrote:

>> The changes in src/java.desktop looks fine.
>
> Changes to `java.logging` and `java.net.http` also look good to me.

Hi Sergey,

I'll give it some testing and sponsor it next week unless someone else steps up.

best regards,
-- daniel

-

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

Re: RFR: 8253821: Improve ByteBuffer performance with GCM [v3]

[Anthony Scarpino]

On Wed, 7 Oct 2020 19:16:12 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with six 
>> additional commits since the last revision:
>> 
>>  - style
>>  - style & comments
>>  - full update
>>  - remove old
>>  - update
>>  - outputsize
>
> I will take a look as well.

This is an update for all of Valerie's comments and adding a test to verify 
different buffer types and configurations

-

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

Re: RFR: 8253821: Improve ByteBuffer performance with GCM [v3]

[Anthony Scarpino]

> 8253821: Improve ByteBuffer performance with GCM

Anthony Scarpino has updated the pull request incrementally with six additional 
commits since the last revision:

 - style
 - style & comments
 - full update
 - remove old
 - update
 - outputsize

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/411/files
  - new: https://git.openjdk.java.net/jdk/pull/411/files/b29c1ed5..7c54017f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=411&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=411&range=01-02

  Stats: 857 lines in 6 files changed: 635 ins; 133 del; 89 mod
  Patch: https://git.openjdk.java.net/jdk/pull/411.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/411/head:pull/411

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

Re: Request for comment, a new idea about distributed TLS sessions

[Xuelei Fan]

Hi,

The JEP was updated so that it has a better presentation.

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

The goals now is described in a higher level, and some of the details 
are moved to the Description section.  Any comments are welcome.  Please 
let me know by end of this month, October 31, 2020.


BTW, I will post a new thread about the algorithm used for the session 
ticket protection and synchronization in the cluster.


Thanks,
Xuelei


On 9/29/2020 9:25 PM, Xuelei Fan wrote:

Hi,

I was wondering to improve the scalability of the TLS implementation in 
JDK.  TLS session resumption is much faster than full handshaking.  It 
may be a good to support efficiently distributing and resuming TLS 
sessions across clusters of computers, by using stateless TLS session 
tickets.


The following is a list of the goals:
1. Use session tickets to distribute and resume sessions.

2. Implement a protection scheme for session tickets.

3. Deprecate or modify Java SE APIs that negatively impact distributed 
session resumption.


4. Ensure that the session tickets generated and protected in one server 
node can be used for session resumption in other nodes in the 
distributed system.


5. Ensure that the secret keys used to protect the session ticket can be 
rotated and synchronized effectively.


6. Ensure that a new server node inserted into the distributed system 
can be automatically synchronized, thus making it possible to plugin new 
server nodes as needed.


For more details, please refer to the draft JEP.

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

Does it sound like a good idea?  Did you run into scalability problems 
for TLS/HTTPS connections?  Any suggestions?  Any comments are welcome.


Thanks & Regards,
Xuelei

Re: NPE in PKIXCertPathValidator

[Sean Mullan]

Yes, that is a bug. Do you want to file a bug report or would you like 
us to file on one your behalf?


Thanks,
Sean

On 10/23/20 10:56 AM, Kai wrote:

Hi,

I ran into a NPE while validating a certificate chain with the latest 
JDK 11 using a TrustAnchor that has been created using the 
TrustAnchor(caName, publicKey, nameConstraints) constructor.


I suspect the PKIXCertPathValidator.validate(TrustAnchor, 
ValidatorParams) method to cause the NPE 
(http://hg.openjdk.java.net/jdk/jdk/file/ee1d592a9f53/src/java.base/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java):


X509ValidationEvent xve = new X509ValidationEvent();
if (xve.shouldCommit() || EventHelper.isLoggingSecurity()) {
int[] certIds = params.certificates().stream()
.mapToInt(x -> x.hashCode())
.toArray();
int anchorCertId =  anchor.getTrustedCert().hashCode();
if (xve.shouldCommit()) {
xve.certificateId = anchorCertId;
int certificatePos = 1; //anchor cert
xve.certificatePosition = certificatePos;
xve.validationCounter = validationCounter.incrementAndGet();
xve.commit();
// now, iterate through remaining
for (int id : certIds) {
xve.certificateId = id;
xve.certificatePosition = ++certificatePos;
xve.commit();
}
}
if (EventHelper.isLoggingSecurity()) {
EventHelper.logX509ValidationEvent(anchorCertId, certIds);
}
}

IMHO line
int anchorCertId =  anchor.getTrustedCert().hashCode();
will throw the NPE if the trust anchor has not been created with a 
certificate as in my case.
The code should do a null check here and fall back to using the 
hashCode of the PublicKey.

WDYT?

Kai

NPE in PKIXCertPathValidator

[Kai]

Hi,

I ran into a NPE while validating a certificate chain with the latest JDK
11 using a TrustAnchor that has been created using the TrustAnchor(caName,
publicKey, nameConstraints) constructor.

I suspect the PKIXCertPathValidator.validate(TrustAnchor, ValidatorParams)
method to cause the NPE (
http://hg.openjdk.java.net/jdk/jdk/file/ee1d592a9f53/src/java.base/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java
):

X509ValidationEvent xve = new X509ValidationEvent();if
(xve.shouldCommit() || EventHelper.isLoggingSecurity()) {  int[]
certIds = params.certificates().stream()  .mapToInt(x ->
x.hashCode())  .toArray();  int anchorCertId =
anchor.getTrustedCert().hashCode();  if (xve.shouldCommit()) {
xve.certificateId = anchorCertId;  int certificatePos = 1;
//anchor cert  xve.certificatePosition = certificatePos;
xve.validationCounter = validationCounter.incrementAndGet();
xve.commit();  // now, iterate through remaining  for (int id
: certIds) {  xve.certificateId = id;
xve.certificatePosition = ++certificatePos;  xve.commit();
 }   }   if (EventHelper.isLoggingSecurity()) {
EventHelper.logX509ValidationEvent(anchorCertId, certIds);   }
}

IMHO line

int anchorCertId = anchor.getTrustedCert().hashCode();

will throw the NPE if the trust anchor has not been created with a
certificate as in my case.

The code should do a null check here and fall back to using the
hashCode of the PublicKey.

WDYT?

Kai

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

[Maurizio Cimadamore]

On Fri, 23 Oct 2020 11:02:11 GMT, Magnus Ihse Bursie  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix whitespaces
>
> Changes requested by ihse (Reviewer).

@magicus the files you commented on are not part of this PR, but they are 
introduced as part of:
https://git.openjdk.java.net/jdk/pull/548
(you seemed to have approved the changes there - but it's also likely that this 
PR doesn't include the latest changes in that PR). Sorry for the confusion - 
but please do report any comment you have on the build changes on that PR!

-

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

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

[Magnus Ihse Bursie]

On Thu, 22 Oct 2020 17:04:34 GMT, Maurizio Cimadamore  
wrote:

>> 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 

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Сергей Цыпанов]

On Fri, 23 Oct 2020 09:12:15 GMT, Daniel Fuchs  wrote:

>> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
>> reason to explicitly instantiate any instance of `Atomic*` class with its 
>> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
>> AtomicInteger()` which is faster:
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @BenchmarkMode(value = Mode.AverageTime)
>> public class AtomicBenchmark {
>>   @Benchmark
>>   public Object defaultValue() {
>> return new AtomicInteger();
>>   }
>>   @Benchmark
>>   public Object explicitValue() {
>> return new AtomicInteger(0);
>>   }
>> }
>> THis benchmark demonstrates that `explicitValue()` is much slower:
>> Benchmark  Mode  Cnt   Score   Error  Units
>> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
>> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
>> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
>> progress we could trivially replace explicit zeroing with default 
>> constructors gaining some performance benefit with no risk.
>> 
>> I've tested the changes locally, both tier1 and tier 2 are ok. 
>> 
>> Could one create an issue for tracking this?
>
> src/java.base/share/classes/sun/net/ResourceManager.java line 65:
> 
>> 63: } catch (NumberFormatException e) {}
>> 64: maxSockets = defmax;
>> 65: numSockets = new AtomicInteger();
> 
> Changes in sun/net look good to me.

@dfuch Could you then sponsor this PR?

-

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

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Daniel Fuchs]

On Fri, 23 Oct 2020 08:15:00 GMT, Sergey Bylokhov  wrote:

>> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
>> reason to explicitly instantiate any instance of `Atomic*` class with its 
>> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
>> AtomicInteger()` which is faster:
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @BenchmarkMode(value = Mode.AverageTime)
>> public class AtomicBenchmark {
>>   @Benchmark
>>   public Object defaultValue() {
>> return new AtomicInteger();
>>   }
>>   @Benchmark
>>   public Object explicitValue() {
>> return new AtomicInteger(0);
>>   }
>> }
>> THis benchmark demonstrates that `explicitValue()` is much slower:
>> Benchmark  Mode  Cnt   Score   Error  Units
>> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
>> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
>> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
>> progress we could trivially replace explicit zeroing with default 
>> constructors gaining some performance benefit with no risk.
>> 
>> I've tested the changes locally, both tier1 and tier 2 are ok. 
>> 
>> Could one create an issue for tracking this?
>
> The changes in src/java.desktop looks fine.

Changes to `java.logging` and `java.net.http` also look good to me.

-

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

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Daniel Fuchs]

On Thu, 22 Oct 2020 20:46:15 GMT, Сергей Цыпанов 
 wrote:

> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
> reason to explicitly instantiate any instance of `Atomic*` class with its 
> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
> AtomicInteger()` which is faster:
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @BenchmarkMode(value = Mode.AverageTime)
> public class AtomicBenchmark {
>   @Benchmark
>   public Object defaultValue() {
> return new AtomicInteger();
>   }
>   @Benchmark
>   public Object explicitValue() {
> return new AtomicInteger(0);
>   }
> }
> THis benchmark demonstrates that `explicitValue()` is much slower:
> Benchmark  Mode  Cnt   Score   Error  Units
> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
> progress we could trivially replace explicit zeroing with default 
> constructors gaining some performance benefit with no risk.
> 
> I've tested the changes locally, both tier1 and tier 2 are ok. 
> 
> Could one create an issue for tracking this?

src/java.base/share/classes/sun/net/ResourceManager.java line 65:

> 63: } catch (NumberFormatException e) {}
> 64: maxSockets = defmax;
> 65: numSockets = new AtomicInteger();

Changes in sun/net look good to me.

-

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

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Sergey Bylokhov]

On Thu, 22 Oct 2020 20:46:15 GMT, Сергей Цыпанов 
 wrote:

> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
> reason to explicitly instantiate any instance of `Atomic*` class with its 
> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
> AtomicInteger()` which is faster:
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @BenchmarkMode(value = Mode.AverageTime)
> public class AtomicBenchmark {
>   @Benchmark
>   public Object defaultValue() {
> return new AtomicInteger();
>   }
>   @Benchmark
>   public Object explicitValue() {
> return new AtomicInteger(0);
>   }
> }
> THis benchmark demonstrates that `explicitValue()` is much slower:
> Benchmark  Mode  Cnt   Score   Error  Units
> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
> progress we could trivially replace explicit zeroing with default 
> constructors gaining some performance benefit with no risk.
> 
> I've tested the changes locally, both tier1 and tier 2 are ok. 
> 
> Could one create an issue for tracking this?

The changes in src/java.desktop looks fine.

-

Marked as reviewed by serb (Reviewer).

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

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Claes Redestad]

On Thu, 22 Oct 2020 20:46:15 GMT, Сергей Цыпанов 
 wrote:

> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
> reason to explicitly instantiate any instance of `Atomic*` class with its 
> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
> AtomicInteger()` which is faster:
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @BenchmarkMode(value = Mode.AverageTime)
> public class AtomicBenchmark {
>   @Benchmark
>   public Object defaultValue() {
> return new AtomicInteger();
>   }
>   @Benchmark
>   public Object explicitValue() {
> return new AtomicInteger(0);
>   }
> }
> THis benchmark demonstrates that `explicitValue()` is much slower:
> Benchmark  Mode  Cnt   Score   Error  Units
> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
> progress we could trivially replace explicit zeroing with default 
> constructors gaining some performance benefit with no risk.
> 
> I've tested the changes locally, both tier1 and tier 2 are ok. 
> 
> Could one create an issue for tracking this?

Filed [8255299](https://bugs.openjdk.java.net/browse/JDK-8255299) for this. 
Prefix the name of the PR with "8255299: " and it should pass checks.

-

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

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Claes Redestad]

On Thu, 22 Oct 2020 20:46:15 GMT, Сергей Цыпанов 
 wrote:

> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
> reason to explicitly instantiate any instance of `Atomic*` class with its 
> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
> AtomicInteger()` which is faster:
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @BenchmarkMode(value = Mode.AverageTime)
> public class AtomicBenchmark {
>   @Benchmark
>   public Object defaultValue() {
> return new AtomicInteger();
>   }
>   @Benchmark
>   public Object explicitValue() {
> return new AtomicInteger(0);
>   }
> }
> THis benchmark demonstrates that `explicitValue()` is much slower:
> Benchmark  Mode  Cnt   Score   Error  Units
> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
> progress we could trivially replace explicit zeroing with default 
> constructors gaining some performance benefit with no risk.
> 
> I've tested the changes locally, both tier1 and tier 2 are ok. 
> 
> Could one create an issue for tracking this?

Marked as reviewed by redestad (Reviewer).

-

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

RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Сергей Цыпанов]

As discussed in https://github.com/openjdk/jdk/pull/510 there is never a reason 
to explicitly instantiate any instance of `Atomic*` class with its default 
value, i.e. `new AtomicInteger(0)` could be replaced with `new AtomicInteger()` 
which is faster:
@State(Scope.Thread)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@BenchmarkMode(value = Mode.AverageTime)
public class AtomicBenchmark {
  @Benchmark
  public Object defaultValue() {
return new AtomicInteger();
  }
  @Benchmark
  public Object explicitValue() {
return new AtomicInteger(0);
  }
}
THis benchmark demonstrates that `explicitValue()` is much slower:
Benchmark  Mode  Cnt   Score   Error  Units
AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
progress we could trivially replace explicit zeroing with default constructors 
gaining some performance benefit with no risk.

I've tested the changes locally, both tier1 and tier 2 are ok. 

Could one create an issue for tracking this?

-

Commit messages:
 - 8255299: Drop explicit zeroing at instantiation of Atomic* objects

Changes: https://git.openjdk.java.net/jdk/pull/818/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=818&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8255299
  Stats: 19 lines in 17 files changed: 0 ins; 3 del; 16 mod
  Patch: https://git.openjdk.java.net/jdk/pull/818.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/818/head:pull/818

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