Re: RFR: 8333396: Use StringBuilder internally for java.text.Format.* formatting [v19]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Use StringBuilder internally for java.text.Format.* formatting
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/5fda8747..6573e413
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=18
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=17-18
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Use StringBuilder internally for java.text.Format.* formatting [v17]
On Mon, 8 Jul 2024 17:31:53 GMT, Justin Lu wrote: > I believe the `static MessageFormat.format(...` also has similar incorrect > "this is equivalent ..." wording, if you could please include that as well. I > think we should also rename the issue (and CSR) as well since the scope has > changed. Something along the lines of "Use StringBuilder internally for > java.text.Format.* formatting". Thanks for checking carefully. Updated. - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2216286724
Re: RFR: 8333396: Use StringBuilder internally for java.text.Format.* formatting [v18]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Use StringBuilder internally for java.text.Format.* formatting
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/d675bcbf..5fda8747
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=17
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=16-17
Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v16]
On Fri, 5 Jul 2024 16:54:41 GMT, Justin Lu wrote: > Following up on my previous comment regarding the _untrue wording_. > > I no longer think we can just split the implementation and specification > portion of this PR into two issues, since if we backport the implementation > only, the specification would now be violated for the back-ported releases. > We may have to just include the specification in this issue as well, (which > will require a CSR and no longer means we can backport this change). A CSR request created: https://bugs.openjdk.org/browse/JDK-8335834 - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2212946822
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v17]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/7122c2c8..d675bcbf
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=16
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=15-16
Stats: 5 lines in 1 file changed: 0 ins; 4 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Tue, 25 Jun 2024 19:33:02 GMT, Naoto Sato wrote: >> I did not mean to introduce a public API for this change (if we do, the fix >> cannot be backported). I thought we could define a package private one, but >> based on your observation, it may get messier... So I agree that falling >> back to `StringBuf` is the way to go, IMO. > >> So, considering all the information given, is it enough to start our new >> review process? @naotoj @liach @justin-curtis-lu > > Well, I was suggesting the same buffer proxying for other Format classes than > NumberFormat subclasses, such as DateFormat so that they would have the same > performance benefit. Would you be willing to do that too? Thanks all for your valuable suggestions. All suggestions were accepted. Is it enough to start our new review process? @naotoj @justin-curtis-lu @liach @irisclark - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2207831524
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v16]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/da09d85a..7122c2c8
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=15
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=14-15
Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v15]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/b5bdc733..da09d85a
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=14
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=13-14
Stats: 9 lines in 3 files changed: 0 ins; 6 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v14]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with two additional
commits since the last revision:
- 896: Performance regression of DecimalFormat.format
- 896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/f1b88f36..b5bdc733
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=13
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=12-13
Stats: 310 lines in 6 files changed: 233 ins; 71 del; 6 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v12]
On Thu, 27 Jun 2024 05:08:06 GMT, Justin Lu wrote:
>> lingjun-cg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 896: Performance regression of DecimalFormat.format
>
> src/java.base/share/classes/java/text/StringBufFactory.java line 45:
>
>> 43: }
>> 44:
>> 45: private static class StringBufferImpl implements Format.StringBuf {
>
> The implementations may be more concise as a `record class`
I know little about `record class`, it seems `record class` is help to model
data aggregation, but here it act as a proxy class.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19513#discussion_r1656957129
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v13]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/7eb9b523..f1b88f36
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=12
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=11-12
Stats: 247 lines in 11 files changed: 78 ins; 63 del; 106 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Tue, 25 Jun 2024 19:33:02 GMT, Naoto Sato wrote: > > So, considering all the information given, is it enough to start our new > > review process? @naotoj @liach @justin-curtis-lu > > Well, I was suggesting the same buffer proxying for other Format classes than > NumberFormat subclasses, such as DateFormat so that they would have the same > performance benefit. Would you be willing to do that too? Sure. All `Format`'s subclasses has been replaced with buffer proxying. After that I run the benchmark test with averageTime mode. The result show the StringBuilder has take effect. Please review again. @naotoj @liach @justin-curtis-lu | Testcase | JDK 11 | JDK 22 | Current JDK | | - | - |- | - | | java.text.NumberFormat#format(double)| 362.221 | 636.049 | 351.913| | java.text.DateFormat#format(java.util.Date)| 362.273|944.733|317.436| |java.text.MessageFormat#format| 599.146| 937.717|499.584| |java.text.ListFormat#format| N/A | 464.123|318.978| - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2191307113
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v12]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/b6646c5d..7eb9b523
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=11
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=10-11
Stats: 242 lines in 9 files changed: 220 ins; 0 del; 22 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v11]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/67c724c7..b6646c5d
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=10
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=09-10
Stats: 305 lines in 6 files changed: 158 ins; 122 del; 25 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Thu, 20 Jun 2024 18:58:27 GMT, Naoto Sato wrote: >> lingjun-cg has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 896: Performance regression of DecimalFormat.format > > I did not mean to introduce a public API for this change (if we do, the fix > cannot be backported). I thought we could define a package private one, but > based on your observation, it may get messier... So I agree that falling back > to `StringBuf` is the way to go, IMO. So, considering all the information given, is it enough to start our new review process? @naotoj @liach @justin-curtis-lu - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2187842086
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v10]
On Fri, 21 Jun 2024 07:25:27 GMT, lingjun-cg wrote:
>> ### Performance regression of DecimalFormat.format
>> From the output of perf, we can see the hottest regions contain atomic
>> instructions. But when run with JDK 11, there is no such problem. The
>> reason is the removed biased locking.
>> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
>> contains many synchronized methods.
>> So I added support for some new methods that accept StringBuilder which is
>> lock-free.
>>
>> ### Benchmark testcase
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class JmhDecimalFormat {
>>
>> private DecimalFormat format;
>>
>> @Setup(Level.Trial)
>> public void setup() {
>> format = new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testNewAndFormat() throws InterruptedException {
>> new DecimalFormat("#0.0").format(9524234.1236457);
>> }
>>
>> @Benchmark
>> public void testNewOnly() throws InterruptedException {
>> new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testFormatOnly() throws InterruptedException {
>> format.format(9524234.1236457);
>> }
>> }
>>
>>
>> ### Test result
>> Current JDK before optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>>
>>
>>
>> Current JDK after optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>>
>>
>> ### JDK 11
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
>
> lingjun-cg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 896: Performance regression of DecimalFormat.format
Updates:
1. Add package private interface `StringBuf` into `Format` as an inner class.
Like `java.text.Format.FieldDelegate` that used by `Format` implemetations.
2. Add `boolean isInternalSubclass` into all `Format` implemetations that in
package `java.text`.
-
PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2182167568
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v10]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/9a6f646f..67c724c7
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=09
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=08-09
Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v9]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/1fde6a60..9a6f646f
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=08
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=07-08
Stats: 348 lines in 11 files changed: 198 ins; 65 del; 85 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Thu, 20 Jun 2024 21:53:25 GMT, Justin Lu wrote:
> > It requires append(int), but the Appendable has no such method.
>
> If via `Appendable` we don't have access to `append(int)`, can we simply
> `append(String.valueOf(int))`. And similarly for `append(char[], int, int)`,
> can we `append(String.valueOf(char[], int, int))`.
>
> According to the method descriptions in `AbstractStringBuilder`, this should
> behaviorally be the same. That way we don't have to add any new classes, and
> can continue with the generic implementation.
If do that ,there is some performance degradation.
Benchmark Mode Cnt
Score Error Units
AppendableTest.tesAppendableAppendCharAsciiavgt 50
33.628 ± 0.263 ns/op
AppendableTest.tesAppendableAppendCharMixNoAsciiChars avgt 50
33.522 ± 0.245 ns/op
AppendableTest.testAppendableAppendInt avgt 50
32.602 ± 0.186 ns/op
AppendableTest.testStringBufferAppendCharArrayAsciiavgt 50
31.028 ± 0.134 ns/op
AppendableTest.testStringBufferAppendCharArrayMixNoAsciiChars avgt 50
31.075 ± 0.158 ns/op
AppendableTest.testStringBufferAppendInt avgt 50
25.706 ± 0.268 ns/op
@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
@Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
@State(Scope.Thread)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class AppendableTest {
private StringBuffer sb;
private Appendable appendable;
private char[] asciiChars;
private char[] mixNoAsciiChars;
@Setup(Level.Iteration)
public void setup() {
sb = new StringBuffer();
appendable = sb;
asciiChars = "Criticism appears to Anatole France the most recent and
possibly the ultimate evolution".toCharArray();
mixNoAsciiChars = "The 测试数据 above mentioned two volumes of poetry were
followed by many works in prose, which we shall notice. France’s critical
writings".toCharArray();
}
@Benchmark
public void testStringBufferAppendInt() throws InterruptedException {
sb.append(12345890);
}
@Benchmark
public void testAppendableAppendInt() throws InterruptedException,
IOException {
appendable.append(String.valueOf(12345890));
}
@Benchmark
public void testStringBufferAppendCharArrayAscii() throws
InterruptedException {
sb.append(asciiChars, 40, 18);
}
@Benchmark
public void tesAppendableAppendCharAscii() throws InterruptedException,
IOException {
appendable.append(new String(asciiChars, 40, 18));
}
@Benchmark
public void testStringBufferAppendCharArrayMixNoAsciiChars() throws
InterruptedException {
sb.append(mixNoAsciiChars, 40, 18);
}
@Benchmark
public void tesAppendableAppendCharMixNoAsciiChars() throws
InterruptedException, IOException {
appendable.append(new String(mixNoAsciiChars, 40, 18));
}
}
-
PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2181851223
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Tue, 18 Jun 2024 10:00:33 GMT, lingjun-cg wrote:
>> ### Performance regression of DecimalFormat.format
>> From the output of perf, we can see the hottest regions contain atomic
>> instructions. But when run with JDK 11, there is no such problem. The
>> reason is the removed biased locking.
>> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
>> contains many synchronized methods.
>> So I added support for some new methods that accept StringBuilder which is
>> lock-free.
>>
>> ### Benchmark testcase
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class JmhDecimalFormat {
>>
>> private DecimalFormat format;
>>
>> @Setup(Level.Trial)
>> public void setup() {
>> format = new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testNewAndFormat() throws InterruptedException {
>> new DecimalFormat("#0.0").format(9524234.1236457);
>> }
>>
>> @Benchmark
>> public void testNewOnly() throws InterruptedException {
>> new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testFormatOnly() throws InterruptedException {
>> format.format(9524234.1236457);
>> }
>> }
>>
>>
>> ### Test result
>> Current JDK before optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>>
>>
>>
>> Current JDK after optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>>
>>
>> ### JDK 11
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
>
> lingjun-cg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 896: Performance regression of DecimalFormat.format
If change the visibility of `java.lang.AbstractStringBuilder` from
package-private to public, it seems more simple.
The `AbstractStringBuilder` is a sealed class, user code cannot extend it, and
it also used by other class like `String`.
-
PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2179655141
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Wed, 19 Jun 2024 02:12:01 GMT, lingjun-cg wrote:
>> src/java.base/share/classes/java/text/Format.java line 278:
>>
>>> 276: * {@code false} otherwise
>>> 277: */
>>> 278: boolean isInternalSubclass() {
>>
>> Since this is defined in Format, can we apply similar changes of
>> StringBuilder formatting to the other Format subclasses beyond just
>> NumberFormat.
>>
>> For example, in DateFormat, something such as,
>>
>>
>> T formatWithGeneric(Date date,
>>T toAppendTo,
>>FieldPosition pos) {
>> throw new UnsupportedOperationException("Subclasses should override
>> this method");
>> }
>
> ok. I will update it if we have a conclusion about using `StringBuf` or
> using ``.
This comment inspire me that use '' in
`SimpleDateFormat` is impossible, because the 'Appendable' lack of necessary
`append` method which SimpleDateFormat requires.
So we need discuss whether `StringBuf` is a better solution?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19513#discussion_r1645309140
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Tue, 18 Jun 2024 20:39:30 GMT, Justin Lu wrote:
>> lingjun-cg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 896: Performance regression of DecimalFormat.format
>
> src/java.base/share/classes/java/text/ChoiceFormat.java line 561:
>
>> 559: toAppendTo.append(choiceFormats[i]);
>> 560: } catch (IOException ioe) {
>> 561: throw new UncheckedIOException(ioe.getMessage(), ioe);
>
> Perhaps `AssertionError` instead of `UncheckedIOException` is better suited
> here and in the other ocurrences.
This can be removed if using `StringBuf`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19513#discussion_r1645304136
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Tue, 18 Jun 2024 20:36:52 GMT, Justin Lu wrote:
>> lingjun-cg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 896: Performance regression of DecimalFormat.format
>
> src/java.base/share/classes/java/text/Format.java line 278:
>
>> 276: * {@code false} otherwise
>> 277: */
>> 278: boolean isInternalSubclass() {
>
> Since this is defined in Format, can we apply similar changes of
> StringBuilder formatting to the other Format subclasses beyond just
> NumberFormat.
>
> For example, in DateFormat, something such as,
>
>
> T formatWithGeneric(Date date,
>T toAppendTo,
>FieldPosition pos) {
> throw new UnsupportedOperationException("Subclasses should override
> this method");
> }
ok. I will update it if we have a conclusion about using `StringBuf` or using
``.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19513#discussion_r1645300958
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Tue, 18 Jun 2024 10:00:33 GMT, lingjun-cg wrote:
>> ### Performance regression of DecimalFormat.format
>> From the output of perf, we can see the hottest regions contain atomic
>> instructions. But when run with JDK 11, there is no such problem. The
>> reason is the removed biased locking.
>> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
>> contains many synchronized methods.
>> So I added support for some new methods that accept StringBuilder which is
>> lock-free.
>>
>> ### Benchmark testcase
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class JmhDecimalFormat {
>>
>> private DecimalFormat format;
>>
>> @Setup(Level.Trial)
>> public void setup() {
>> format = new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testNewAndFormat() throws InterruptedException {
>> new DecimalFormat("#0.0").format(9524234.1236457);
>> }
>>
>> @Benchmark
>> public void testNewOnly() throws InterruptedException {
>> new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testFormatOnly() throws InterruptedException {
>> format.format(9524234.1236457);
>> }
>> }
>>
>>
>> ### Test result
>> Current JDK before optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>>
>>
>>
>> Current JDK after optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>>
>>
>> ### JDK 11
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
>
> lingjun-cg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 896: Performance regression of DecimalFormat.format
Using ` ` instead of `StringBuf` seems a
good idea, but it has a disadvantage. The `Appendable` defines only 3 `append`
methods which cannot cover some cases. For example, this method
`SimpleDateFormat#format(Date, StringBuffer, FieldDelegate)` contains the
following code:
case TAG_QUOTE_CHARS:
toAppendTo.append(compiledPattern, i, count);
i += count;
break;
It requires `append(char[], int, int)`, but the `Appendable` has no such method.
So it's difficult to use ` ` in methods
`String DateFormat.format(Date date), String ListFormat.format(List
input) and, String MessageFormat.format(Object obj)`.
The method in `java.text.MessageFormat#subformat` contains the following code:
int argumentNumber = argumentNumbers[i];
if (arguments == null || argumentNumber >= arguments.length) {
result.append('{').append(argumentNumber).append('}');
continue;
}
It requires `append(int)`, but the `Appendable` has no such method.
To sum up
1. ` ` is clear, but lack of extensibility
2. StringBuf is not so clear, but it's more extensibility. We can add
`append(char[], int, int)` to `StringBuf` to support `SimpleDateFormat`.
3. New interface StringBuf no need to handle IOException, but use ` ` need to add a lot of try...catch statements around
the append method.
4.
So it seems `StringBuf` is better if we want to fix the problem in
`DateFormat,ListFormat,MessageFormat` in unique way.
I look forward to your reply. @naotoj @liach @justin-curtis-lu
Using ` ` instead of `StringBuf` seems a
good idea, but it has a disadvantage. The `Appendable` defines only 3 `append`
methods which cannot cover some cases. For example, this method
`SimpleDateFormat#format(Date, StringBuffer, FieldDelegate)` contains the
following code:
case TAG_QUOTE_CH
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v5]
On Mon, 17 Jun 2024 18:17:06 GMT, Justin Lu wrote:
>>> I second Justin's suggestion here. The change should benefit every
>>> implementation in the JDK, not only NumberFormat. Also, I might suggest
>>> renaming the new class, as it is kind of long/redundant. How about using
>>> something as simple as "StringBuf"?
>>
>> Thanks for your comment. The long name bother me for a while. I have changed
>> it to ""StringBuf".
>
> Hi @lingjun-cg
>
> Thanks for your work here.
>
> While this would benefit the performance of NumberFormat subclasses, we are
> hoping that if we are going to make the internal switch to StringBuilder, we
> can also make similar changes to other Format subclasses as well where
> applicable. So, we would want `isInternalSubclass()` to be defined at the
> Format level. Some methods that I can immediately think of would be `String
> DateFormat.format(Date date)`, `String ListFormat.format(List input)`
> and, `String MessageFormat.format(Object obj)`.
>
> I believe @naotoj shares the same sentiment.
@justin-curtis-lu The method `isInternalSubclass` in `DecimalFormat` already
pulls up to `Format`.
@liach @naotoj Using `Appendable & CharSequence` instead of an interface with a
limited method is a great idea. Following this idea, I have updated the PR. But
there are some notes when implementing the idea.
1. Rename `format` to `formatWithGeneric` in DecimalFormat that accepts
StringBuilder. If not that, throw a StackOverflowException when run with
`DecimalFormat.format(double)`. If not rename the format, there are 2 methods
in DecimalFormat:
```
@Override
public StringBuffer format(double number, StringBuffer result,
FieldPosition fieldPosition) {
return format(number, result, fieldPosition);
}
@Override
T format(double number, T result,
FieldPosition fieldPosition)
{
}
Because the second parameter type `StringBuffer` of the 1st method is more
concrete than type `T` of the 2nd method, so the 1st method will call itself
recursively.
2. The `Appendable#append(java.lang.CharSequence)` method can throws
IOException, so there is some messy code handling IOException.
-
PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2175778275
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/e95f0928..1fde6a60
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=07
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=06-07
Stats: 186 lines in 8 files changed: 130 ins; 0 del; 56 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Withdrawn: 8333396: Performance regression of DecimalFormat.format
On Mon, 3 Jun 2024 04:18:20 GMT, lingjun-cg wrote:
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
This pull request has been closed without being integrated.
-
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v7]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request with a new target base due to a merge
or a rebase.
-
Changes: https://git.openjdk.org/jdk/pull/19513/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=19513=06
Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v5]
On Thu, 13 Jun 2024 17:53:45 GMT, Justin Lu wrote:
>> lingjun-cg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 896: Performance regression of DecimalFormat.format
>
> src/java.base/share/classes/java/text/CompactNumberFormat.java line 885:
>
>> 883: }
>> 884:
>> 885: private StringBuilderBufferProxy format(BigInteger number,
>> StringBuilderBufferProxy result,
>
> While the DecimalFormat and CompactNumberFormat BigInteger and BigDecimal
> `format` methods will always return a StringBuffer, we must change the method
> signatures to use the proxy, otherwise we would need to define alternate
> methods all the way at the `Format.FieldDelegate` level. So I think this is a
> lesser of two evils.
I'm not sure what you mean. Would you suggest add two new methods that accept
StringBuilderBufferProxy and keep the origin methods that accept StringBuffer?
Like this:
interface FieldDelegate {
public void formatted(Format.Field attr, Object value, int start,
int end, StringBuffer buffer);
public void formatted(int fieldID, Format.Field attr, Object value,
int start, int end, StringBuffer buffer);
public void formatted(Format.Field attr, Object value, int start,
int end, StringBuilderBufferProxy buffer);
public void formatted(int fieldID, Format.Field attr, Object value,
int start, int end, StringBuilderBufferProxy
buffer);
}
But DecimalFormat#subformatNumber is a common method that only accept
StringBuilderBufferProxy, and CompactNumberFormat#format(BigDecimal) call it,
so CompactNumberFormat#format(BigDecimal) need change signature.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19513#discussion_r1639197282
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v5]
On Thu, 13 Jun 2024 19:40:49 GMT, Chen Liang wrote:
>> lingjun-cg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 896: Performance regression of DecimalFormat.format
>
> src/java.base/share/classes/java/text/StringBuilderBufferProxy.java line 108:
>
>> 106: }
>> 107:
>> 108: class StringBuilderImpl implements StringBuilderBufferProxy {
>
> Avoid adding nested classes to interfaces, which are public. These classes
> are publicly accessible under reflection, and we want to avoid more of such
> occurrences. See
> https://bugs.openjdk.org/browse/JDK-8308040?focusedId=14581402=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14581402
Thanks for your suggestion. I split this class into 2 classes: one is the
interface, the other is the factory class with package-private.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19513#discussion_r1639179219
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v5]
On Thu, 13 Jun 2024 19:40:48 GMT, Naoto Sato wrote: > I second Justin's suggestion here. The change should benefit every > implementation in the JDK, not only NumberFormat. Also, I might suggest > renaming the new class, as it is kind of long/redundant. How about using > something as simple as "StringBuf"? Thanks for your comment. The long name bother me for a while. I have changed it to ""StringBuf". - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2167144044
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v6]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/1ab65e18..d590f132
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=05
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=04-05
Stats: 467 lines in 11 files changed: 222 ins; 188 del; 57 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v4]
On Tue, 4 Jun 2024 19:44:57 GMT, Naoto Sato wrote: >> lingjun-cg has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 896: Performance regression of DecimalFormat.format > > Also it would be helpful to compare the performance without biased locking in > JDK11. @naotoj The above contains the result of JDK11 with disabled and enabled biased locking. It seems the key is the biased locking. Now our application use DecimalFormat.format() seriously, the performance is declining when migrate from JDK 11 to JDK 21. Could you give some suggestion?Thanks. - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2162813093
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v5]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/6210c61e..1ab65e18
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=04
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=03-04
Stats: 45 lines in 8 files changed: 37 ins; 0 del; 8 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v4]
On Tue, 4 Jun 2024 19:44:57 GMT, Naoto Sato wrote: > Also it would be helpful to compare the performance without biased locking in > JDK11. If run in JDK11 without biased locking, the performance is as same as run in current JDK. - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2148719265
Integrated: 8333462: Performance regression of new DecimalFormat() when compare to jdk11
On Tue, 4 Jun 2024 02:32:28 GMT, lingjun-cg wrote:
> Run the below benchmark test ,it show the average time of new
> DecimalFormat() increase 18% when compare to jdk 11.
>
> the result with jdk 11:
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
>
>
> the result with current jdk:
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> @Setup(Level.Trial)
> public void setup() {
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
> }
>
>
> Compare the flame graph it shows the
> java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time.
> After replacing the lambda implementation with a simple loop , it shows
> nearly the same performance as jdk 11.
>
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> [flame-graph-jdk11-jdk21.zip](https://github.com/user-attachments/files/15541764/flame-graph-jdk11-jdk21.zip)
This pull request has now been integrated.
Changeset: d8261279
Author:lingjun.cg
Committer: Denghui Dong
URL:
https://git.openjdk.org/jdk/commit/d826127970bd2ae8bf4cacc3c55634dc5af307c4
Stats: 8 lines in 1 file changed: 3 ins; 0 del; 5 mod
8333462: Performance regression of new DecimalFormat() when compare to jdk11
Reviewed-by: liach, naoto, jlu
-
PR: https://git.openjdk.org/jdk/pull/19534
Re: RFR: 8333462: Performance regression of new DecimalFormat() when compare to jdk11 [v2]
> Run the below benchmark test ,it show the average time of new
> DecimalFormat() increase 18% when compare to jdk 11.
>
> the result with jdk 11:
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
>
>
> the result with current jdk:
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> @Setup(Level.Trial)
> public void setup() {
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
> }
>
>
> Compare the flame graph it shows the
> java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time.
> After replacing the lambda implementation with a simple loop , it shows
> nearly the same performance as jdk 11.
>
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> [flame-graph-jdk11-jdk21.zip](https://github.com/user-attachments/files/15541764/flame-graph-jdk11-jdk21.zip)
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
8333462: Performance regression of new DecimalFormat() when compare to jdk11
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19534/files
- new: https://git.openjdk.org/jdk/pull/19534/files/a230ce00..d5b9ccfb
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19534=01
- incr: https://webrevs.openjdk.org/?repo=jdk=19534=00-01
Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/19534.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19534/head:pull/19534
PR: https://git.openjdk.org/jdk/pull/19534
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v4]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/0a581428..6210c61e
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=03
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=02-03
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v3]
On Tue, 4 Jun 2024 07:54:56 GMT, kuaiwei wrote: > > > Hi, I think you can add the jmh test case. > > > > > > [#19513 > > (comment)](https://github.com/openjdk/jdk/pull/19513#issue-2330131051) > > already contains the test case. > > I mean it can be added as a test case in test/micro and be evaluated in > future build. Thanks for your remainder. There exist a JMH test case which named 'test/micro/org/openjdk/bench/java/text/DefFormatterBench.java'. This JMH test case run in throughput mode. If run without the patch, the output is: Benchmark Mode Cnt ScoreError Units DefFormatterBench.testDefNumberFormatter thrpt5 1842.666 ? 83.694 ops/ms If run with after apply the patch, the output is: Benchmark Mode Cnt Score Error Units DefFormatterBench.testDefNumberFormatter thrpt5 3988.274 ? 513.434 ops/ms The score is increased by (3988.274-1842.666)/1842.666 = 116%. - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2146983441
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v3]
On Tue, 4 Jun 2024 07:21:15 GMT, kuaiwei wrote: > Hi, I think you can add the jmh test case. https://github.com/openjdk/jdk/pull/19513#issue-2330131051 already contains the test case. - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2146844849
RFR: 8333462: Performance regression of new DecimalFormat() when compare to jdk11
Run the below benchmark test ,it show the average time of new DecimalFormat()
increase 18% when compare to jdk 11.
the result with jdk 11:
Benchmark Mode CntScore Error Units
JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
the result with current jdk:
Benchmark Mode CntScore Error Units
JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
@Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
@State(Scope.Thread)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class JmhDecimalFormat {
@Setup(Level.Trial)
public void setup() {
}
@Benchmark
public void testNewOnly() throws InterruptedException {
new DecimalFormat("#0.0");
}
}
Compare the flame graph it shows the
java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time.
After replacing the lambda implementation with a simple loop , it shows nearly
the same performance as jdk 11.
Benchmark Mode CntScore Error Units
JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
[flame-graph-jdk11-jdk21.zip](https://github.com/user-attachments/files/15541764/flame-graph-jdk11-jdk21.zip)
-
Commit messages:
- 8333462: Performance regression of new DecimalFormat() when compare to jdk11
Changes: https://git.openjdk.org/jdk/pull/19534/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=19534=00
Issue: https://bugs.openjdk.org/browse/JDK-8333462
Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod
Patch: https://git.openjdk.org/jdk/pull/19534.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19534/head:pull/19534
PR: https://git.openjdk.org/jdk/pull/19534
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v3]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Test result
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/b48962b5..0a581428
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=02
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=01-02
Stats: 6 lines in 1 file changed: 0 ins; 2 del; 4 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of new DecimalFormat and DecimalFormat.format [v2]
On Mon, 3 Jun 2024 06:13:35 GMT, lingjun-cg wrote:
>> ### Performance regression of DecimalFormat.format
>> From the output of perf, we can see the hottest regions contain atomic
>> instructions. But when run with JDK 11, there is no such problem. The
>> reason is the removed biased locking.
>> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
>> contains many synchronized methods.
>> So I added support for some new methods that accept StringBuilder which is
>> lock-free.
>>
>> ### Performance regression of new DecimalFormat
>> After comparing the flame graph between current jdk and jdk 11, the method
>> java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time.
>> The performance becomes as good as jdk11 after replacing it with a simple
>> loop implementation.
>>
>>
>>
>> ### Test result
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class JmhDecimalFormat {
>>
>> private DecimalFormat format;
>>
>> @Setup(Level.Trial)
>> public void setup() {
>> format = new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testNewAndFormat() throws InterruptedException {
>> new DecimalFormat("#0.0").format(9524234.1236457);
>> }
>>
>> @Benchmark
>> public void testNewOnly() throws InterruptedException {
>> new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testFormatOnly() throws InterruptedException {
>> format.format(9524234.1236457);
>> }
>> }
>>
>> Current JDK before optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>>
>>
>>
>> Current JDK after optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnly avgt 50 351.499 ? 0.761 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>>
>>
>> ### JDK 11
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
>> JmhDecimalFormat.testNewAndForma...
>
> lingjun-cg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 896: Performance regression of new DecimalFormat and
> DecimalFormat.format
Hi naotaoj,
What I mean "performance regression" is compare to JDK 11. We have an
server side application that use DecimalFormat.format API seriously. When
migrate it from JDK 11 to JDK 21, we found a performance degradation.
So I write the JMH test case "JmhDecimalFormat". It show that there a
performance regression since JDK 21.
These are the perfasm output for running JMH on both JDK 11 and JDK21. There
are some hot regions around the atomic instructions in JDK 21, but no such
problem in JDK 11.
[jdk11.txt](https://github.com/user-attachments/files/15541278/jdk11.txt)
[jdk21.txt](https://github.com/user-attachments/files/15541279/jdk21.txt)
Maybe the [JEP 374: Deprecate and Disable Biased
Locking](https://openjdk.org/jeps/374) is the reason?
So I run the benchmark on JDK 11 again but with option '-XX:-UseBiasedLocking',
there only a minor gap between jdk11 and jdk 21.
OK, return to my patch. java.text use StringBuffer internally, but nearly all
methods in StringBuffer are synchronized:
@Override
public synchronized StringBuffer append(Object obj) {
}
@Override
@IntrinsicCandidate
public synchronized StringBuffer append(String str) {
...
}
>From the above analysis, the atomic instructions slow down
>DecimalFormat.format, and StringBuffer's synchronized methods generate there
>atomic instructions. So If remove these synchronized methods, it will get a
>better performance.
So I replace StringBuffer with StringBuilder in java.text.NumberFormat.
public final String format(double number) {
// Use fast-path for double result if t
Re: RFR: 8333396: Performance regression of new DecimalFormat and DecimalFormat.format [v2]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Performance regression of new DecimalFormat
> After comparing the flame graph between current jdk and jdk 11, the method
> java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time.
> The performance becomes as good as jdk11 after replacing it with a simple
> loop implementation.
>
>
>
> ### Test result
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode Cnt Score Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of new DecimalFormat and DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/a6755f8f..b48962b5
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=01
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=00-01
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
RFR: 8333396: Performance regression of new DecimalFormat and DecimalFormat.format
### Performance regression of DecimalFormat.format >From the output of perf, we can see the hottest regions contain atomic >instructions. But when run with JDK 11, there is no such problem. The reason >is the removed biased locking. The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself contains many synchronized methods. So I added support for some new methods that accept StringBuilder which is lock-free. ### Performance regression of new DecimalFormat After comparing the flame graph between current jdk and jdk 11, the method java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time. The performance becomes as good as jdk11 after replacing it with a simple loop implementation. - Commit messages: - 896: Performance regression of new DecimalFormat and DecimalFormat.format Changes: https://git.openjdk.org/jdk/pull/19513/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19513=00 Issue: https://bugs.openjdk.org/browse/JDK-896 Stats: 318 lines in 11 files changed: 268 ins; 0 del; 50 mod Patch: https://git.openjdk.org/jdk/pull/19513.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513 PR: https://git.openjdk.org/jdk/pull/19513
