Re: RFR: 8333396: Performance regression of DecimalFormat.format [v16]

2024-07-07 Thread lingjun-cg 
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]

2024-07-07 Thread Chen Liang 
On Mon, 8 Jul 2024 02:32:17 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

Quick question about the violation of the "This is equivalent to" spec: Does 
our new implementation lead to any observable side effects that make the 
returned results or thrown exceptions different from that of `format(obj, new 
StringBuffer(), new FieldPosition(0)).toString()`?

In the ClassFile API, there are similar "behave as if" notes such as 
https://github.com/openjdk/jdk/blob/3f37c5718d676b7001e6a084aed3ba645745a144/src/java.base/share/classes/java/lang/classfile/ClassFile.java#L433-L438,
 yet we don't restrict implementations as the API surface, including the return 
an exception result and side effects are the same.

-

PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2212893099

Re: RFR: 8333396: Performance regression of DecimalFormat.format [v17]

2024-07-07 Thread 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: 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&pr=19513&range=16
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19513&range=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

RFR: 8335637: Add explicit well-behaved expectations to Object.{toString, hashCode}

2024-07-07 Thread Joe Darcy 
Make well-behaved implementation expectations of Object.{toString, hashCode} 
explicit.

-

Commit messages:
 - JDK-8335637: Add explicit well-behaved expectations to Object.{toString, 
hashCode}

Changes: https://git.openjdk.org/jdk/pull/20063/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20063&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8335637
  Stats: 14 lines in 1 file changed: 14 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/20063.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20063/head:pull/20063

PR: https://git.openjdk.org/jdk/pull/20063