Re: RFR: 8333396: Use StringBuilder internally for java.text.Format.* formatting [v18]

[Justin Lu]

On Tue, 9 Jul 2024 02:06:34 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: Use StringBuilder internally for java.text.Format.* formatting

Thanks for the changes. Let's work to get the CSR in good shape now.

(I have left comments directly on the CSR).

-

Marked as reviewed by jlu (Committer).

PR Review: https://git.openjdk.org/jdk/pull/19513#pullrequestreview-2167093930

Re: RFR: 8333396: Use StringBuilder internally for java.text.Format.* formatting [v18]

[lingjun-cg]

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