Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects [v2]
On Wed, 28 Oct 2020 08:56:05 GMT, Сергей Цыпанов wrote: >> FYI it is better to use merge, instead of rebase+force push. Rebase breaks >> history and all existed code comments. > > @mrserb thanks for pointing this out! Thanks for updating with latest master changes Sergey! My tests were all green. - PR: https://git.openjdk.java.net/jdk/pull/818
Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects [v2]
On Wed, 28 Oct 2020 08:49:38 GMT, Sergey Bylokhov wrote: >> Rebased onto master to have the fix introduced in >> https://github.com/openjdk/jdk/pull/778 > > FYI it is better to use merge, instead of rebase+force push. Rebase breaks > history and all existed code comments. @mrserb thanks for pointing this out! - PR: https://git.openjdk.java.net/jdk/pull/818
Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects [v2]
On Wed, 28 Oct 2020 08:40:02 GMT, Сергей Цыпанов wrote: >> client changes are fine > > Rebased onto master to have the fix introduced in > https://github.com/openjdk/jdk/pull/778 FYI it is better to use merge, instead of rebase+force push. Rebase breaks history and all existed code comments. - PR: https://git.openjdk.java.net/jdk/pull/818
Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects [v2]
> 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?
Сергей Цыпанов 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 one additional commit
since the last revision:
8255299: Drop explicit zeroing at instantiation of Atomic* objects
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/818/files
- new: https://git.openjdk.java.net/jdk/pull/818/files/c1fb362f..7dc646d0
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=818&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=818&range=00-01
Stats: 4576 lines in 201 files changed: 2659 ins; 1135 del; 782 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
Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects [v2]
On Sat, 24 Oct 2020 23:12:09 GMT, Phil Race wrote: >> Сергей Цыпанов 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 one additional >> commit since the last revision: >> >> 8255299: Drop explicit zeroing at instantiation of Atomic* objects > > client changes are fine Rebased onto master to have the fix introduced in https://github.com/openjdk/jdk/pull/778 - PR: https://git.openjdk.java.net/jdk/pull/818
Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects
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?
client changes are fine
-
Marked as reviewed by prr (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/818
Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects
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: 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
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
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
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
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
Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects
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
