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]
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 [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
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? 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
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